Read a cell id on the argument side as the return side already does - #358
Merged
estebanzimanyi merged 1 commit intoAug 30, 2026
Merged
Conversation
A by-value cell id is uint64_t for h3index and quadbin and unsigned long for s2cell, and both spellings are equally an ordinary hash seed, so the type alone cannot say which one a parameter is. ret_type settles it for a bare cell-id return by reading the function's own scope; arg_type took only the type, which left every cell-id argument unmappable. arg_type now takes the function name and resolves the cell id through the same reg_scope and CELL_BASEVAL the return side uses. shape_binary and shape_scalar_first read a by-value argument through scalar_arg_for, which applies that same resolution to the DuckDB type alone: a cell type is a BIGINT alias, so the C local stays int64_t and the value casts to uint64_t at the MEOS call, the cast the return side already writes back with. A hash_extended seed has no cell scope to resolve and keeps its native UBIGINT. The generic binary shape therefore reaches the cell comparisons for every grid, and shape_h3_prefilter no longer claims them. It keeps the geometry prefilter, which the generic paths do not marshal. Two runs over one catalog, with and without the change, add 24 registrations and remove none. quadbin gains the cell-vs-temporal comparisons it declared and did not publish, h3index and quadbin gain tEq and tNe, and the sixteen h3index comparisons the retired pass emitted are unchanged. tquadbin.test and h3_prefilter.test state the surface in both operand orders, each with the cell the value never takes so the answer can be false. The static accessors stay out. Their reg_scope is None, so a static function states no grid to resolve a cell type from, and reading one is its own change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A by-value cell id is uint64_t for h3index and quadbin and unsigned long for
s2cell, and both spellings are equally an ordinary hash seed, so the type alone
cannot say which one a parameter is. ret_type settles it for a bare cell-id
return by reading the function's own scope; arg_type took only the type, which
left every cell-id argument unmappable.
arg_type now takes the function name and resolves the cell id through the same
reg_scope and CELL_BASEVAL the return side uses. shape_binary and
shape_scalar_first read a by-value argument through scalar_arg_for, which
applies that same resolution to the DuckDB type alone: a cell type is a BIGINT
alias, so the C local stays int64_t and the value casts to uint64_t at the MEOS
call, the cast the return side already writes back with. A hash_extended seed
has no cell scope to resolve and keeps its native UBIGINT.
The generic binary shape therefore reaches the cell comparisons for every grid,
and shape_h3_prefilter no longer claims them. It keeps the geometry prefilter,
which the generic paths do not marshal.
Two runs over one catalog, with and without the change, add 24 registrations
and remove none. quadbin gains the cell-vs-temporal comparisons it declared and
did not publish, h3index and quadbin gain tEq and tNe, and the sixteen h3index
comparisons the retired pass emitted are unchanged. tquadbin.test and
h3_prefilter.test state the surface in both operand orders, each with the cell
the value never takes so the answer can be false.
The static accessors stay out. Their reg_scope is None, so a static function
states no grid to resolve a cell type from, and reading one is its own change.