feat(function): add S2 and H3 geospatial cell functions

[fengttt]

Implements S2_* and H3_* SQL functions for geospatial cell indexing.

Functions

S2 (via github.com/golang/geo/s2, already a dependency):
s2_cellid(POINT), s2_cellid_level, s2_cellid_center, s2_cellid_area, s2_cellid_parent, s2_cellid_edgeneighbours, s2_cellid_allneighbours, s2_cellid_areneighbours

H3 (via github.com/uber/h3-go/v4, newly added — cgo-backed, bundles the H3 C library):
h3_h3index(POINT[, res]), h3_h3index_resolution, h3_h3index_center, h3_h3index_boundary, h3_h3index_parent[, res], h3_h3index_neighbours, h3_h3index_areneighbours

A CellId / H3Index is BIGINT UNSIGNED (uint64). A POINT argument carries (longitude, latitude). Neighbour lists return a JSON array of uint64 encoded with bytejson's native uint64 type code so the full 64-bit id is preserved (above 2^53 a JSON float would lose precision). Both British and American spellings of "neighbour(s)" are accepted.

Behavioural notes

• s2_cellid_parent returns a CellId (BIGINT UNSIGNED), mirroring h3_h3index_parent.
• h3_h3index(POINT) defaults to resolution 15 and has an optional explicit-resolution overload — H3 needs a resolution to pick a cell, unlike S2 where a point maps to a unique leaf cell.
• s2_cellid_area returns square metres rather than S2's native unit-sphere steradians, so it is directly usable and comparable to H3.
• Invalid CellId/H3Index (e.g. 0) or a non-POINT geometry argument raises an invalid input error; NULL inputs yield NULL.

Files

• pkg/sql/plan/function/func_s2h3.go (new — implementations + helpers)
• pkg/sql/plan/function/function_id.go, function_id_test.go, list_builtIn.go (ids + registration)
• go.mod / go.sum (h3-go/v4)
• docs/design/s2h3_funcs.md (design note + implementation details)
• test/distributed/cases/geo/geo_s2h3.{sql,result} (BVT)

Testing

• Full mo-service build + link clean.
• Test_funids ID-registration unit test passes.
• BVT geo_s2h3.sql: 34/34 statements pass (100%), covering every function, the stored BIGINT UNSIGNED column path, and invalid-input error cases.

Note for reviewers

h3-go/v4 is cgo-backed and bundles the H3 C library — please confirm it builds in CI / musl-static configurations.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

I re-checked the latest head and still see two substantive correctness issues.

1. h3_h3index_areneighbours does not match the documented mixed-resolution semantics.
   The docs now explicitly say mixed-level comparisons should return false, not error. But the implementation still forwards directly to ca.IsNeighbor(cb), and with the H3 v4.5.0 library used by this PR that call returns ErrResolutionMismatch when the two cells are valid but at different resolutions.

   I verified this against the dependency version in the PR: a cell and its parent do not produce (false, nil); they produce a resolution-mismatch error. So the current implementation contradicts the documented/user-facing semantics.

   Suggestion: detect differing H3 resolutions before calling IsNeighbor and return false; add an H3 ancestor/descendant SQL regression alongside the existing S2 cross-level case.

2. The INSERT binder fix still misses wrapped st_point(...) expressions.
   pkg/sql/plan/bind_insert.go now switches to the function-call binder only when the top-level VALUES AST node is exactly *tree.FuncExpr. Parenthesized or cast-wrapped constructors like (st_point(...)) or cast(st_point(...) as point) still go through the destination-column binder, and base_binder.go recursively binds their inner expression with that same binder. So the original overload-resolution failure can still survive under transparent wrappers.

   Suggestion: unwrap at least ParenExpr and CastExpr before deciding whether to use the function-call binder, and add regression cases for wrapped constructors in INSERT ... VALUES into point / point32 columns.

Non-blocking follow-up suggestions:

• add alias-call coverage for the US spelling variants (...neighbors),
• add an H3 pentagon neighbor/boundary edge-case test, since that is a real H3 shape difference even though the safe GridDisk path itself did not reproduce a zero-cell bug in my local probe.

[fengttt]

Thanks @XuPeng-SH — both addressed in 11a7e32fb7.

1. h3_h3index_areneighbours mixed-resolution. You're right: h3-go v4.5.0's IsNeighbor returns a resolution-mismatch error for two valid cells at different resolutions, which contradicted the documented "cross-resolution → false" semantics. Now I check ca.Resolution() != cb.Resolution() and return (false, nil) before calling IsNeighbor, matching the S2 cross-level behavior. Verified a cell vs its coarser ancestor now returns 0, not an error, and added an H3 ancestor/descendant regression alongside the S2 cross-level case in geo_s2h3.sql.

2. Wrapped st_point(...) in INSERT ... VALUES. Correct — the *tree.FuncExpr-only check missed transparent wrappers. I added valuesExprIsFuncCall, which unwraps *tree.ParenExpr and *tree.CastExpr before deciding whether to use the function-call binder. So (st_point(...)) and cast(st_point(...) as point) now bind their arguments by their own types and insert correctly into point/point32; a cast wrapping a bare literal (123.45::int) still uses the column binder, so dtype/decimal and pg_cast remain green. Added regression cases for both wrapper forms into point/point32.

Non-blocking:

• Added American-spelling alias coverage (s2_cellid_allneighbors, s2_cellid_edgeneighbors, h3_h3index_neighbors, and the *_areneighbors predicates).
• H3 pentagon boundary/neighbour edge case: deferred — h3_h3index_boundary already handles whatever vertex count the library returns (5 for a pentagon, 6 for a hexagon) and the GridDisk path didn't reproduce a zero-cell issue, so I left it as a possible follow-up rather than pinning a specific pentagon cell. Happy to add it if you'd like.

Verified locally: geo, dtype/decimal and pg_cast all 100%.

[aunjgr]

LGTM