feat(vortex-spatial): add make-line scalar function - #9201
Conversation
Merging this PR will regress 1 benchmark
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
a9ca3bf to
7a91f2d
Compare
7a91f2d to
351f3be
Compare
351f3be to
fe549e1
Compare
|
@claude review this PR, and look for potential regressions as well as inconsistencies with other code that is not in this PR |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
72a4ae1 to
4abcde8
Compare
4abcde8 to
98e2031
Compare
fbd0853 to
184beec
Compare
184beec to
b9875c7
Compare
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
9548a2d to
f83d073
Compare
connortsui20
left a comment
connortsui20
left a comment
There was a problem hiding this comment.
discussed offline, should pull out the interleave perf optimizations out of this PR
f83d073 to
1e028c4
Compare
| Ok(ConstantArray::new(0.0f64, points.len()).into_array()) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
“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.
There was a problem hiding this comment.
e.g. XYZ + XYM → LINESTRING ZM (..., 2 2 0 3) https://github.com/duckdb/duckdb-spatial/blob/c27bc2b0091f75e97c189d345f37a3771dfef89c/test/sql/geometry/st_makeline.test
HarukiMoriarty
changed the title
connortsui20
left a comment
connortsui20
left a comment
There was a problem hiding this comment.
Ok LGTM, but please pull out the interleave optimizations before you merge
1e028c4 to
317771d
Compare
317771d to
1698f91
Compare
## Summary - execute Interleave arrays whose values use primitive dtypes - gather from canonical primitive buffers while preserving constant value sources - share selector length and bounds validation with the Boolean kernel - cover nullable primitive columns, constants, null constants, and invalid selectors ## Motivation Interleave previously only had an execution kernel for Boolean values. This adds the corresponding primitive kernel as a general vortex-array capability. Spatial MakeLine uses this support in the separate stacked PR #9201. --------- Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1698f91 to
b9562b7
Compare
## Rationale Vortex native LineString arrays should support planar length without an Arrow, WKB, or row-oriented geo_types conversion. ## What changes are included? - Add vortex.st.length for native LineString inputs. - Compute every row directly from flattened coordinate buffers and row offsets. - Sum all adjacent segments, supporting empty, one-point, and arbitrary-length LineStrings. - Preserve constant results and strict null propagation. - Add focused correctness tests and CodSpeed benchmarks. ## Split This was split from #9201. It is standalone on develop; #9201 now contains only ST_MakeLine and primitive Interleave support. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
2 participants
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?
Stack
#9291 (primitive Interleave execution) has merged into develop, so this PR is now standalone: rebased onto develop, it contains only the spatial function.
ST_Length was split into #9290 so each scalar function can also be reviewed independently.