Skip to content

feat(vortex-spatial): add make-line scalar function - #9201

Open
HarukiMoriarty wants to merge 1 commit into
nemo/interleave-primitive-executionfrom
nemo/geo-native-bbox-precheck
Open

feat(vortex-spatial): add make-line scalar function#9201
HarukiMoriarty wants to merge 1 commit into
nemo/interleave-primitive-executionfrom
nemo/geo-native-bbox-precheck

Conversation

@HarukiMoriarty

@HarukiMoriarty HarukiMoriarty commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Rationale

Vortex native geometry arrays should support constructing two-point LineStrings directly from paired Point columns without an Arrow or WKB round trip.

What changes are included?

  • Add vortex.st.make_line for paired native Point arrays.
  • Promote mixed XY, XYZ, XYM, and XYZM inputs, filling absent ordinates with zero.
  • Propagate CRS metadata, constants, and endpoint nulls.
  • Build native LineString storage directly and add focused tests and CodSpeed benchmarks.

Stack

Depends on #9291 for primitive Interleave execution. That general vortex-array capability is reviewed separately from the spatial function.

ST_Length was split into #9290 so each scalar function can also be reviewed independently.

@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 259 untouched benchmarks
⏩ 1763 skipped benchmarks1


Comparing nemo/geo-native-bbox-precheck (1698f91) with nemo/interleave-primitive-execution (0b5c2ac)2

Open in CodSpeed

Footnotes

  1. 1763 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.

  2. No successful run was found on nemo/interleave-primitive-execution (6ad96f8) during the generation of this report, so 9f4aa0e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from a9ca3bf to 7a91f2d Compare August 5, 2026 18:56
@HarukiMoriarty
HarukiMoriarty changed the base branch from develop to nemo/geo-scalar-execution August 5, 2026 19:13
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 7a91f2d to 351f3be Compare August 5, 2026 19:17
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 351f3be to fe549e1 Compare August 5, 2026 19:33
@connortsui20

Copy link
Copy Markdown
Member

@claude review this PR, and look for potential regressions as well as inconsistencies with other code that is not in this PR

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 72a4ae1 to 4abcde8 Compare August 6, 2026 14:41
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 4abcde8 to 98e2031 Compare August 6, 2026 17:58
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from fbd0853 to 184beec Compare August 7, 2026 13:52
Base automatically changed from nemo/geo-scalar-execution to develop August 7, 2026 14:44
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 184beec to b9875c7 Compare August 7, 2026 14:44
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch 3 times, most recently from 9548a2d to f83d073 Compare August 7, 2026 19:17

@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.

discussed offline, should pull out the interleave perf optimizations out of this PR

@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from f83d073 to 1e028c4 Compare August 7, 2026 19:52
Ok(ConstantArray::new(0.0f64, points.len()).into_array())
}
}

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.

in what cases do you ever run into an "absent ordinate", and is returning 0.0 really ok here? instead of null, for example?

its a bit hard for me to understand why we want this behavior in the create linestring functionality

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

“absent ordinate” only occurs with mixed-dimensional inputs. We promote the output dimension to the union of both inputs and fill missing Z/M values with zero, matching DuckDB’s ST_MakeLine behavior.

Null would represent a null coordinate inside an otherwise valid geometry and is not supported by the native LineString coordinate storage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@HarukiMoriarty HarukiMoriarty changed the title feat(vortex-geo): add make-line and length scalar functions feat(vortex-spatial): add make-line scalar function Aug 7, 2026

@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.

Ok LGTM, but please pull out the interleave optimizations before you merge

@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 1e028c4 to 317771d Compare August 7, 2026 20:04
@HarukiMoriarty
HarukiMoriarty removed the request for review from joseph-isaacs August 7, 2026 20:04
@HarukiMoriarty
HarukiMoriarty changed the base branch from develop to nemo/interleave-primitive-execution August 7, 2026 20:05
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@HarukiMoriarty
HarukiMoriarty force-pushed the nemo/geo-native-bbox-precheck branch from 317771d to 1698f91 Compare August 7, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants