fix(#319): strip H3 index/partition columns from value_columns (#320)

register_hex_tiles listed every non-leading SELECT column as a
value_column, including H3 join/partition keys (h0, h3, …) the caller
carries through. Since downstream defaults value_column to
value_columns[0], a `SELECT h8, h0, <value>` that omits value_column
would color the map by meaningless H3 integers instead of the metric,
and h0 also polluted suggested_scale + cost a wasted value_stats scan.

Filter sql_value_columns through a `^h\d+$` match in prepare_hex_tiles,
before the empty-value guard so an H3-only SELECT still raises. The
build is unaffected: Phase 1 derives h0 itself, so partitioning never
depended on the caller carrying it.

Author: cboettig

tests/test_tile_pyramid.py

@@ -197,7 +197,12 @@ def test_non_count_requires_value_columns(self):
 import os
 import duckdb
 from pathlib import Path
-from tiles.pyramid import register_hex_tiles, _inspect_user_sql
+from tiles.pyramid import (
+    register_hex_tiles,
+    prepare_hex_tiles,
+    _inspect_user_sql,
+    _strip_h3_columns,
+)
 
 
 @pytest.fixture
@@ -238,6 +243,54 @@ def test_still_extracts_value_cols_when_present(self, h3_conn):
         assert value_cols == ["v1", "v2"]
 
 
+class TestStripH3Columns:
+    # #319: carried H3 index/partition columns (h0, h3, …) are not values and
+    # must never reach value_columns — downstream defaults value_column to
+    # value_columns[0] and would color by meaningless H3 integers.
+    def test_drops_h0_partition_key(self):
+        assert _strip_h3_columns(["h0", "hw_frac"]) == ["hw_frac"]
+
+    def test_drops_any_h_res_index(self):
+        assert _strip_h3_columns(["h3", "h8", "val", "h0"]) == ["val"]
+
+    def test_keeps_non_h3_columns(self):
+        assert _strip_h3_columns(["val", "mean_carbon", "frac"]) == [
+            "val", "mean_carbon", "frac"]
+
+    def test_does_not_drop_hprefixed_word_columns(self):
+        # Only bare `h<digits>` are H3 columns; `height`, `hw_frac`, `h_index`
+        # are genuine values and must survive.
+        assert _strip_h3_columns(["height", "hw_frac", "h_index"]) == [
+            "height", "hw_frac", "h_index"]
+
+
+class TestPrepareValueColumns:
+    def test_carried_h0_excluded_from_value_columns(self, local_bucket, h3_conn):
+        # #319: SELECT that carries the h0 partition key through for a join must
+        # not advertise h0 as a value column.
+        user_sql = (
+            "SELECT h3_latlng_to_cell(37.8, -122.3, 5) AS h5, "
+            "h3_cell_to_parent(h3_latlng_to_cell(37.8, -122.3, 5), 0) AS h0, "
+            "0.5 AS hw_frac"
+        )
+        plan = prepare_hex_tiles(
+            con=h3_conn, sql=user_sql, finest_res=5, min_res=2, agg="SUM",
+        )
+        assert plan["value_columns"] == ["hw_frac"]
+
+    def test_only_h3_columns_raises(self, h3_conn):
+        # SELECT h8, h0 with no real value: after stripping h0, nothing is left,
+        # so the existing empty-value guard must still fire (agg != COUNT).
+        user_sql = (
+            "SELECT h3_latlng_to_cell(37.8, -122.3, 8) AS h8, "
+            "h3_cell_to_parent(h3_latlng_to_cell(37.8, -122.3, 8), 0) AS h0"
+        )
+        with pytest.raises(ValueError, match="at least one value column"):
+            prepare_hex_tiles(
+                con=h3_conn, sql=user_sql, finest_res=8, min_res=2, agg="SUM",
+            )
+
+
 class TestRegisterHexTiles:
     def test_finest_h0_partitioned_coarse_levels_single_file(self, local_bucket, h3_conn):
         # #189: only the FINEST level is partitioned by h0 (file-level pruning

tiles/pyramid.py

@@ -5,6 +5,7 @@
 """
 import json
 import os
+import re
 import sys
 import time
 from decimal import Decimal
@@ -267,6 +268,22 @@ def _json_dumps_escaped(obj) -> str:
     return json.dumps(obj).replace("'", "''")
 
 
+# H3 cell/index/partition columns follow the `h<res>` convention (h0..h15) —
+# `h0` is the hive partition key the pyramid build derives itself, and callers
+# routinely carry an `h<res>` index column through the SELECT for joins. None of
+# these are values, so they must not appear in value_columns (#319): they'd
+# corrupt suggested_scale, waste a value_stats scan, and — since downstream
+# defaults value_column to value_columns[0] — color the map by meaningless H3
+# integers instead of the real metric.
+_H3_COLUMN_RE = re.compile(r"^h\d+$")
+
+
+def _strip_h3_columns(columns: List[str]) -> List[str]:
+    """Drop H3 cell/index/partition columns (names matching `^h\\d+$`) from a
+    list of candidate value columns. See #319."""
+    return [c for c in columns if not _H3_COLUMN_RE.match(c)]
+
+
 def _inspect_user_sql(con: duckdb.DuckDBPyConnection, user_sql: str):
     """Run user SQL with LIMIT 0 to extract column names without materializing data.
 
@@ -453,12 +470,15 @@ def prepare_hex_tiles(
     if agg.upper() == "COUNT":
         value_columns = ["count"]
     else:
-        if not sql_value_columns:
+        # Drop carried H3 index/partition columns (h0, h3, …) so a partition key
+        # is never advertised as a value (#319). Runs before the empty-check so
+        # `SELECT h8, h0` (no real value) still raises below.
+        value_columns = _strip_h3_columns(sql_value_columns)
+        if not value_columns:
             raise ValueError(
                 "user SQL must return at least one value column after the H3 index "
                 "(or use agg='COUNT')"
             )
-        value_columns = sql_value_columns
 
     h = content_hash(sql=sql, finest_res=finest_res, min_res=min_res, agg=agg, zoom_offset=zoom_offset)
     paths = tile_paths_for_hash(h)
@@ -647,7 +667,9 @@ def register_hex_tiles(
       Any extra columns in the user SQL are ignored.
     - Other aggs: user SQL must return at least one value column after the H3
       index. Each is aggregated via `agg` at parent resolutions and passed
-      through raw at the finest level.
+      through raw at the finest level. Carried H3 index/partition columns
+      (names matching `h<res>`, e.g. h0, h3) are dropped from value_columns —
+      they're join/partition keys, not values (#319).
     """
     plan = prepare_hex_tiles(
         con=con, sql=sql, finest_res=finest_res,