Skip to content

Read a cell id on the argument side as the return side already does - #358

Merged
estebanzimanyi merged 1 commit into
MobilityDB:mainfrom
estebanzimanyi:feat/duck-cell-id-arg
Aug 30, 2026
Merged

Read a cell id on the argument side as the return side already does#358
estebanzimanyi merged 1 commit into
MobilityDB:mainfrom
estebanzimanyi:feat/duck-cell-id-arg

Conversation

@estebanzimanyi

@estebanzimanyi estebanzimanyi commented Aug 30, 2026

Copy link
Copy Markdown
Member

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.

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.
@estebanzimanyi
estebanzimanyi merged commit 84d9ac0 into MobilityDB:main Aug 30, 2026
10 checks passed
@estebanzimanyi
estebanzimanyi deleted the feat/duck-cell-id-arg branch August 30, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant