Skip to content

refactor(vortex-geo): generalize scalar function execution - #9215

Merged
HarukiMoriarty merged 2 commits into
developfrom
nemo/geo-scalar-execution
Aug 7, 2026
Merged

refactor(vortex-geo): generalize scalar function execution#9215
HarukiMoriarty merged 2 commits into
developfrom
nemo/geo-scalar-execution

Conversation

@HarukiMoriarty

@HarukiMoriarty HarukiMoriarty commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Native geometry scalar functions need consistent handling for constant operands, pairwise columns, null propagation, and all-null batches. Keeping those responsibilities in each function makes new unary and binary kernels harder to implement correctly.

What changes are included in this PR?

  • Split shared geometry execution into unary, binary, and geo-types adapter modules.
  • Represent constant and column operands explicitly in shared execution state.
  • Centralize strict null propagation and all-null short-circuiting.
  • Preserve constant outputs when all operands are constant.
  • Migrate Envelope, Contains, Distance, and Intersects to the shared execution layer.
  • Keep bounding-box prechecks scoped to one-constant binary predicate paths.
  • Move native Rect array assembly into the Rect extension module.

What APIs are changed? Are there any user-facing changes?

This is an internal vortex-geo refactor. Existing scalar function semantics remain unchanged.

@connortsui20 connortsui20 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@gatesn the RowFn work that I am working on (#9128) will supersede this, but I think this is fine to merge?

@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 1934 untouched benchmarks
⏩ 51 skipped benchmarks1


Comparing nemo/geo-scalar-execution (faf33c2) with develop (d443eab)

Open in CodSpeed

Footnotes

  1. 51 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-scalar-execution branch 3 times, most recently from 09f16ff to bdcdde1 Compare August 6, 2026 14:41
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-scalar-execution branch from 105be3e to 70ff267 Compare August 6, 2026 17:58
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-scalar-execution branch from 70ff267 to faf33c2 Compare August 7, 2026 13:52
@HarukiMoriarty
HarukiMoriarty merged commit 74a2b86 into develop Aug 7, 2026
74 checks passed
@HarukiMoriarty
HarukiMoriarty deleted the nemo/geo-scalar-execution branch August 7, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/skip Do not list PR in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants