Skip to content

feat: support geospatial results on the kernel path - #965

Closed
cathleeny wants to merge 10 commits into
databricks:mainfrom
cathleeny:feat/geospatial-results
Closed

cathleeny wants to merge 10 commits into
databricks:mainfrom
cathleeny:feat/geospatial-results

Conversation

@cathleeny

@cathleeny cathleeny commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • add the kernel-only, default-enabled enable_geospatial_support connection option
  • return canonical {"srid": int, "wkb": bytes} values when enabled and WKT / EWKT strings when disabled
  • preserve logical GEOMETRY / GEOGRAPHY Arrow metadata
  • pass the value through the typed PyO3 argument only; it is never forwarded to SEA

Dependency

Depends on databricks/databricks-sql-kernel#361

Validation

  • unit suite: 1,023 passed, 4 skipped, 2 deselected
  • real-kernel signature/routing tests against the exact kernel extension: 2 passed
  • live Reyden E2E against the exact extension: default-enabled, explicit-enabled, and disabled-string modes passed through both direct Arrow-scalar and pandas row conversion paths, including Arrow metadata, WKB equality, and nulls

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
@cathleeny
cathleeny marked this pull request as ready for review October 6, 2026 23:48
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Comment thread src/databricks/sql/backend/kernel/client.py
Comment thread src/databricks/sql/backend/thrift_backend.py
Comment thread tests/unit/test_kernel_result_set.py
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

@aakash-saravanan-db aakash-saravanan-db left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python-side implementation looks good. Please resolve the existing minimum-supported Kernel wheel/dependency-floor comment before merging. The additional durable E2E coverage suggestion is nonblocking.

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
@cathleeny
cathleeny enabled auto-merge October 19, 2026 18:07

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 1 Low

Looks good — well-tested feature addition with one low-severity compatibility note. The _col_to_description regex extraction, the positional (vs name-based) arrow-field indexing in _hive_schema_to_description, and the kernel kwarg wiring are all correct and carry matching unit coverage. The only thing to confirm is release-coupling of the paired kernel wheel, since the option is default-enabled and hard-fails on wheels that don't declare it.

Comment thread src/databricks/sql/backend/kernel/client.py
@cathleeny cathleeny added the kernel-e2e Trigger preview run of the Kernel E2E workflow on this PR label Oct 19, 2026
@cathleeny

Copy link
Copy Markdown
Collaborator Author

Superseded by #990, which uses the same commit from a direct upstream branch so trusted Kernel E2E can run.

@cathleeny cathleeny closed this Oct 19, 2026
auto-merge was automatically disabled October 19, 2026 20:16

Pull request was closed

This branch was successfully deployed

1 active deployment
azure-prod — 54fbd76c Deployed Oct 19, 2026 by peco-review-bot[bot] via followup #999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel-e2e Trigger preview run of the Kernel E2E workflow on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants