Skip to content

[SEA-NodeJS] (2/8) napi-rs binding consumer — TS loader + build script#380

Draft
msrathore-db wants to merge 6 commits into
msrathore/sea-abstractionfrom
msrathore/sea-napi-binding
Draft

[SEA-NodeJS] (2/8) napi-rs binding consumer — TS loader + build script#380
msrathore-db wants to merge 6 commits into
msrathore/sea-abstractionfrom
msrathore/sea-napi-binding

Conversation

@msrathore-db
Copy link
Copy Markdown
Contributor

@msrathore-db msrathore-db commented May 16, 2026

Stack

Linear stack of 8 PRs landing the M0 + M1 Phase 1 SEA NodeJS work. Merge in order from base ↑ to tip. The tip branch (msrathore/sea-auth-u2m, PR #383) is the single snapshot containing everything in flight — point your test or benchmark harness at it for an end-to-end check.

Pos PR Branch Scope
1/8 #378 sea-abstraction IBackend / ISessionBackend / IOperationBackend interfaces
2/8 #380 sea-napi-binding TS loader + build script for the kernel-provided .node artifact
3/8 #377 sea-errors-logging Kernel ErrorCode → JS error-class mapping (M0 minimum)
4/8 #379 sea-auth PAT auth via useSEA: true
5/8 #382 sea-execution executeStatement + openSession (sessionConfig, initialCatalog/Schema)
6/8 #381 sea-results CloudFetch + Inline Arrow result fetching
7/8 #384 sea-operation cancel / close / finished lifecycle + INTERVAL parity + napi-relocation acceptance (absorbed sea-integration content)
8/8 #383 sea-auth-u2m ← TIP M1 Phase 1 OAuth M2M + U2M (5 review rounds, ZERO HIGH at close)

Companion kernel stack (databricks/databricks-sql-kernel): 8 PRs — root #26 (async-public-api) → #27#25#29#28#30#24#23 (tip).

Policy: new PRs always stack on the current tip. No sibling/parallel topology. No force-pushes on existing PRs unless absolutely necessary; if a PR's content is wrong, add a fix-up commit on top of the stack tip rather than rewriting history.


This PR is position 2/8.

Summary

Thin TypeScript loader for the napi-rs binding (SeaNativeLoader), generated native/sea/index.d.ts, and the build:native npm script that compiles against the kernel workspace's napi/ sub-crate.

Companion kernel PR

The Rust napi crate itself lives in the kernel repo — see databricks/databricks-sql-kernel#25 (Database / Connection / Statement / ResultStream classes) and its skeleton predecessor #27.

Test plan

  • npm run build:native produces native/sea/index.linux-x64-gnu.node
  • ✅ Loader correctly require()s the platform-specific binary

Draft until you give the go for review.

Tracking

Co-authored-by: Isaac

@github-actions
Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@msrathore-db msrathore-db force-pushed the msrathore/sea-napi-binding branch from 84d0860 to 20231c8 Compare May 17, 2026 13:22
@msrathore-db msrathore-db changed the base branch from main to msrathore/sea-abstraction May 17, 2026 13:22
@github-actions
Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@github-actions
Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

msrathore-db added a commit that referenced this pull request May 24, 2026
Addresses PR #380 review findings C1, C2 (partial), C3, H1, H2, H3,
H4, H5, H6, H7, H8 (runtime guard), H9, M1, M2, M3, M4 (linguist),
M5, M6, M7, M8, L1, L2, L4.

- C1 lint: drop `.js` ext from native/sea require so eslint
  import/extensions passes; gitignore the auto-generated index.js
  and *.node artifacts; prettier-ignore the napi-rs auto-generated
  index.d.ts / index.js.
- C2 publish: whitelist native/sea/index.{js,d.ts} in .npmignore;
  declare @databricks/sea-native-linux-x64-gnu in optionalDependencies.
  The kernel-side package-name rename (drop the linux-x64-gnu
  prefix) is tracked separately as a cross-PR ask.
- C3 test wiring: move tests/native/version.test.ts ->
  tests/unit/sea/, tests/native/e2e-smoke.test.ts -> tests/e2e/sea/;
  both now picked up by the existing mocharc globs and run via the
  existing `npm test` / `npm run e2e` jobs.
- H1 noise drop: version.test.ts now asserts one meaningful semver
  check; three tautological assertions removed.
- H2 + H3 + H7 lazy load: rewrite SeaNativeLoader.ts on the
  lib/utils/lz4.ts pattern. Lazy require behind getSeaNative();
  capability-detection helper tryGetSeaNative(); structured error
  messages that classify MODULE_NOT_FOUND vs ERR_DLOPEN_FAILED and
  include platform/arch/Node-version + install hint.
- H4 supply chain: pin @napi-rs/cli to 2.18.4 in devDependencies;
  build:native switches to `npx --no-install` so the pinned local
  install is used (no per-build network fetch).
- H5 path: switch build:native default kernel path to the canonical
  `../../databricks-sql-kernel/napi-binding` (the worktree-specific
  `-sea-WT` suffix is gone).
- H6 CI safety: e2e suite hard-fails when CI=true and any required
  env var is missing; dev machines still skip.
- H8 runtime guard: loader throws a structured error on Node <18
  instead of attempting a dlopen that would fail mysteriously.
  Driver's engines.node stays >=14 — Thrift consumers on older
  Node continue to work.
- H9 + M1 + M2 types: tsconfig adds a `@sea-native` path alias to
  native/sea/index.d.ts; loader imports the real Connection /
  Statement / ConnectionOptions / ExecuteOptions / ArrowBatch /
  ArrowSchema types and re-exports them. Tests use the re-exported
  types — the inline-shape duplication across three files collapses.
- M3 README: rewrite native/sea/README.md to match kernel reality.
  The napi crate is a standalone Cargo workspace (not a pyo3
  sibling); explain the tls-rustls choice that the standalone
  workspace exists to enable.
- M4 drift detection: mark native/sea/index.d.ts as
  linguist-generated in .gitattributes so GitHub collapses it in
  diffs and excludes from blame/language stats.
- M5 artifacts: gitignore native/sea/index.js, index.node,
  index.*.node.
- M6 + M7 e2e coverage: decode the IPC payload via apache-arrow's
  tableFromIPC and assert numRows + cell value (not just shape);
  add drain-past-null idempotence and schema-before-fetch
  coverage.
- L1 build scripts: collapse build:native + build:native:debug
  into one script taking BUILD_PROFILE (defaults to --release).
- L2 / L4 / M8: drop the version() alias and the "Round 2+ will…"
  comment debt; the loader now actually delivers the value the
  prior JSDoc was only promising.

Verified on this branch: npm run lint clean (0 errors); npm run
prettier clean for PR-owned files (3 unrelated pre-existing
warnings on PR #378 territory); tsc --project tsconfig.build.json
clean; mocha on tests/unit/sea passes (4/4); mocha on
tests/e2e/sea passes against a live pecotesting warehouse (2/2,
IPC decode confirms SELECT 1 returns 1).
@github-actions
Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Creates the napi-rs binding skeleton: Cargo.toml + lib.rs + module
stubs for database/connection/statement/result/error/logger. Captures
napi-rs tokio Handle via OnceCell in runtime.rs. Single working
#[napi] fn version() proves the binding loads + executes end-to-end
in Node.

Depends on krn-async-public-api branch (path dep on kernel).

Round 2 will add open/execute/fetch methods.

Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…wired

Adds real async methods on the opaque wrappers backing M0:
- openSession (free function) with PAT → kernel Session
- Connection::execute_statement → kernel ExecutedStatement
- Statement::fetch_next_batch / schema / cancel / close → kernel ResultStream
- Arrow batches returned as IPC bytes (per Layer 2 design)
- Error mapping preserves kernel ErrorCode + SQLSTATE for TS layer
- All entry points wrapped in catch_unwind

End-to-end smoke test against pecotesting passes.
No new dependencies beyond arrow-{ipc,array,schema} + futures.
Uses kernel async public API (no block_on).

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…indings

Round 1 scaffold declared tracing + tracing-subscriber as deps but
never used them. Removed. Logger bridge will re-add in round 3.

Other findings from 6b3affd-2026-05-15.md reviewed:
- Finding 2 (Database::Drop unreachable in Round 1b) — obsoleted by
  Round 2 (40d0b57): database.rs no longer declares a Database struct
  or Drop impl; it is now an `open_session` free function.
- Finding 3 (empty Connection::Drop) — obsoleted by Round 2: the Drop
  impl now spawns a real fire-and-forget close on the captured tokio
  handle.

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Per D-006 architectural decision (Python team's workspace pattern):
all language bindings (PyO3, napi-rs) now live as workspace siblings
in the kernel repo at databricks-sql-kernel/{pyo3,napi}/.

What this commit removes from the nodejs repo:
- native/sea/Cargo.toml (path dep relocated; package now at
  databricks-sql-kernel/napi/Cargo.toml with path = "..")
- native/sea/build.rs
- native/sea/src/* (lib, runtime, database, connection, statement,
  result, error, logger, util — all 9 files)
- native/sea/package.json (the @databricks/sea-native-linux-x64-gnu
  sub-package moves to the kernel workspace too)
- native/sea/index.js (regenerated artifact)

What stays in nodejs:
- native/sea/index.d.ts — TS declarations consumed by lib/sea/ adapter
- native/sea/README.md (new) — explains the move; points readers at
  databricks-sql-kernel/napi/

What's updated:
- package.json: `build:native` and `build:native:debug` scripts now
  delegate to the kernel workspace via $DATABRICKS_SQL_KERNEL_REPO
  (defaults to ../../databricks-sql-kernel-sea-WT/napi-binding for the
  local dev worktree layout). Build copies index.node + index.d.ts
  back into native/sea/ for the loader to find.

Why workspace co-location:
- Arrow version pinning lockstep — no silent IPC version drift
- path = ".." (clean) vs ../../../../databricks-sql-kernel-sea-WT/...
- Single CI: cargo build --workspace covers kernel + pyo3 + napi
- Kernel API changes that break either binding caught at PR-review time
- Future cgo binding for Go SEA slots in as another workspace member

This branch (sea-napi-binding) is now a thin consumer of the kernel
napi crate. The actual Rust code lives at krn-napi-binding HEAD on
the kernel repo (commit debe3d7).

Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…generated

Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Addresses PR #380 review findings C1, C2 (partial), C3, H1, H2, H3,
H4, H5, H6, H7, H8 (runtime guard), H9, M1, M2, M3, M4 (linguist),
M5, M6, M7, M8, L1, L2, L4.

- C1 lint: drop `.js` ext from native/sea require so eslint
  import/extensions passes; gitignore the auto-generated index.js
  and *.node artifacts; prettier-ignore the napi-rs auto-generated
  index.d.ts / index.js.
- C2 publish: whitelist native/sea/index.{js,d.ts} in .npmignore;
  declare @databricks/sea-native-linux-x64-gnu in optionalDependencies.
  The kernel-side package-name rename (drop the linux-x64-gnu
  prefix) is tracked separately as a cross-PR ask.
- C3 test wiring: move tests/native/version.test.ts ->
  tests/unit/sea/, tests/native/e2e-smoke.test.ts -> tests/e2e/sea/;
  both now picked up by the existing mocharc globs and run via the
  existing `npm test` / `npm run e2e` jobs.
- H1 noise drop: version.test.ts now asserts one meaningful semver
  check; three tautological assertions removed.
- H2 + H3 + H7 lazy load: rewrite SeaNativeLoader.ts on the
  lib/utils/lz4.ts pattern. Lazy require behind getSeaNative();
  capability-detection helper tryGetSeaNative(); structured error
  messages that classify MODULE_NOT_FOUND vs ERR_DLOPEN_FAILED and
  include platform/arch/Node-version + install hint.
- H4 supply chain: pin @napi-rs/cli to 2.18.4 in devDependencies;
  build:native switches to `npx --no-install` so the pinned local
  install is used (no per-build network fetch).
- H5 path: switch build:native default kernel path to the canonical
  `../../databricks-sql-kernel/napi-binding` (the worktree-specific
  `-sea-WT` suffix is gone).
- H6 CI safety: e2e suite hard-fails when CI=true and any required
  env var is missing; dev machines still skip.
- H8 runtime guard: loader throws a structured error on Node <18
  instead of attempting a dlopen that would fail mysteriously.
  Driver's engines.node stays >=14 — Thrift consumers on older
  Node continue to work.
- H9 + M1 + M2 types: tsconfig adds a `@sea-native` path alias to
  native/sea/index.d.ts; loader imports the real Connection /
  Statement / ConnectionOptions / ExecuteOptions / ArrowBatch /
  ArrowSchema types and re-exports them. Tests use the re-exported
  types — the inline-shape duplication across three files collapses.
- M3 README: rewrite native/sea/README.md to match kernel reality.
  The napi crate is a standalone Cargo workspace (not a pyo3
  sibling); explain the tls-rustls choice that the standalone
  workspace exists to enable.
- M4 drift detection: mark native/sea/index.d.ts as
  linguist-generated in .gitattributes so GitHub collapses it in
  diffs and excludes from blame/language stats.
- M5 artifacts: gitignore native/sea/index.js, index.node,
  index.*.node.
- M6 + M7 e2e coverage: decode the IPC payload via apache-arrow's
  tableFromIPC and assert numRows + cell value (not just shape);
  add drain-past-null idempotence and schema-before-fetch
  coverage.
- L1 build scripts: collapse build:native + build:native:debug
  into one script taking BUILD_PROFILE (defaults to --release).
- L2 / L4 / M8: drop the version() alias and the "Round 2+ will…"
  comment debt; the loader now actually delivers the value the
  prior JSDoc was only promising.

Verified on this branch: npm run lint clean (0 errors); npm run
prettier clean for PR-owned files (3 unrelated pre-existing
warnings on PR #378 territory); tsc --project tsconfig.build.json
clean; mocha on tests/unit/sea passes (4/4); mocha on
tests/e2e/sea passes against a live pecotesting warehouse (2/2,
IPC decode confirms SELECT 1 returns 1).

Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
@msrathore-db msrathore-db force-pushed the msrathore/sea-napi-binding branch from 6b70ce4 to 548a14b Compare May 24, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant