fix: a cidr or varchar column, or a domain over either, prunes chunk groups (#1433)

CHANGELOG.md

@@ -430,6 +430,14 @@ true until the next version shipped.
   itself. The runtime filter now leaves such a join to the ordinary plan. Found by
   OffgridwithJD (#1437). `test/native_join_runtime_filter.sh` and
   `test_join_runtime_filter.py`.
+- **A cidr or varchar column, or a domain over either, prunes chunk groups.** A
+  comparison on such a column resolves to inet's or text's operator, and the reader
+  called the key cross-type and dropped it, because the opfamily orders only
+  (inet, inet) and (text, text). The column was read whole, while EXPLAIN still
+  counted the filter as pushed down. Such a key now uses the column's own ordering
+  and hash, which the writer used to build the zone maps and bloom filters. Found
+  by OffgridwithJD reviewing #1426 (#1433). `test/native_skip.sh` and
+  `test_native_skip.py`.
 
 - **A concurrent `UPDATE` of the same row now fails with 40001 on a table with a
   unique index, as documented, not 23505 (#1421).** Two sessions updating one row of

src/columnar_reader.c

@@ -29,6 +29,7 @@
 #include "catalog/pg_am.h"
 #include "commands/defrem.h"
 #include "miscadmin.h"
+#include "parser/parse_coerce.h"
 #include "utils/lsyscache.h"
 #include "utils/rangetypes.h"
 #include "port/atomics.h"
@@ -884,6 +885,37 @@ pgcolumnar_make_predicates(SkipPredicate *out, int nkeys, ScanKey keys,
 
 		crossType = (OidIsValid(argType) && argType != colType);
 
+		/*
+		 * Nor is a pair of types that are both binary-coercible to the type the
+		 * column's default btree opclass orders (#1433). A comparison on a cidr
+		 * column resolves to inet's operator and a varchar column's to text's,
+		 * so the key arrives with a column of cidr and an argument of inet. The
+		 * two share one representation and the opclass's ordering, but
+		 * network_ops registers an ordering proc only for (inet, inet), so the
+		 * opfamily lookup below found nothing and the key was dropped: the
+		 * column pruned nothing, as a domain did before #483.
+		 *
+		 * The column's own cmp and hash procs, from lookup_type_cache below,
+		 * are the opclass's (network_cmp and hashinet for cidr, bttextcmp and
+		 * hashtext for varchar). The writer builds the zone maps and bloom
+		 * filters with those same procs, so treating the key as same-type keeps
+		 * reader and writer in agreement, the bloom probe included.
+		 *
+		 * Not when the opclass orders a polymorphic type: two different enums
+		 * are each binary-coercible to anyenum without sharing an ordering.
+		 */
+		if (crossType)
+		{
+			Oid			opclass = GetDefaultOpClass(colType, BTREE_AM_OID);
+			Oid			inType = OidIsValid(opclass)
+				? get_opclass_input_type(opclass) : InvalidOid;
+
+			if (OidIsValid(inType) && !IsPolymorphicType(inType) &&
+				(colType == inType || IsBinaryCoercible(colType, inType)) &&
+				(argType == inType || IsBinaryCoercible(argType, inType)))
+				crossType = false;
+		}
+
 		tce = lookup_type_cache(att->atttypid,
 								TYPECACHE_CMP_PROC_FINFO |
 								TYPECACHE_HASH_PROC_FINFO);

test/check_ledger.tsv

@@ -3335,7 +3335,7 @@ type_matrix	type_matrix	a range on bt holds rows on heap, so its comparison read
 type_matrix	type_matrix	a range on bt lets the zone maps skip chunk groups	15;16;17;18;19	2026-10-15	the scan never skips a row group by zone map (pgcolumnar_native_group_can_match ignored)
 type_matrix	type_matrix	a range on cd finds heap's rows	15;16;17;18;19	never	-
 type_matrix	type_matrix	a range on cd holds rows on heap, so its comparison reads something	15;16;17;18;19	never	-
-type_matrix	type_matrix	a range on cd skips no chunk groups: a cidr key is treated as cross-type against inet and dropped (#1433)	15;16;17;18;19	never	-
+type_matrix	type_matrix	a range on cd lets the zone maps skip chunk groups	15;16;17;18;19	2026-10-15	a cidr key was treated as cross-type against inet and dropped, so the range removed no chunk groups (the tree before #1433)
 type_matrix	type_matrix	a range on ch finds heap's rows	15;16;17;18;19	never	-
 type_matrix	type_matrix	a range on ch holds rows on heap, so its comparison reads something	15;16;17;18;19	never	-
 type_matrix	type_matrix	a range on ch skips no chunk groups: every row group holds every value, since the column cycles through 26 letters	15;16;17;18;19	never	-

test/check_ledger_budget.txt

@@ -2803,4 +2803,16 @@ suites_not_covered 224
 #     never         2598 + 4 = 2602
 #     dated          868 + 4 = 872
 #     2602 + 872 == 3474
-checks_never_observed_red 2602
+#
+# #1433, a cidr or varchar key was dropped as cross-type, so the column pruned nothing
+# (jdatcmd): 2602 -> 2601. type_matrix's cd range arm, pinned at 0 removed groups with
+# #1433 as its reason, becomes 'a range on cd lets the zone maps skip chunk groups'.
+# The pinned row leaves and the new one arrives dated, on PG17 by the tree before the
+# fix, where the range removed 0 of 12. Five-major runs, 57/57 on each of 15-19.
+# native_skip gains sixteen checks and is not a covered suite.
+#
+#     ledger rows   3474 - 1 + 1 = 3474
+#     never         2602 - 1     = 2601
+#     dated          872 + 1     = 873
+#     2601 + 873 == 3474
+checks_never_observed_red 2601

test/native_skip.sh

@@ -428,4 +428,48 @@ check "a partition takes the columnar custom scan (#436)" \
 check "and it prunes row groups (#436)" \
 	"$(gt0 "$(skipped 'SELECT id FROM nskp WHERE id BETWEEN 5000 AND 5100')")" "yes"
 
+
+# ---- a type binary-coercible to its opclass's type must prune (#1433) --------
+#
+# A comparison on a cidr column resolves to inet's operator and a varchar
+# column's to text's, with the column relabelled. The key reached the reader
+# with a column of cidr (or varchar) and an argument of inet (or text), so it
+# was cross-type, and the opfamily registers an ordering proc only for
+# (inet, inet) and (text, text). The key was dropped, and the column pruned
+# nothing while EXPLAIN still counted the filter as pushed down. inet and text
+# hold the same values and are the controls: the fixture prunes when the key is
+# kept. Parity first in every arm, as above.
+psql_run "CREATE DOMAIN nsk_cidr AS cidr;"
+psql_run "CREATE DOMAIN nsk_vc AS varchar(20);"
+psql_run "CREATE TABLE hb (id int, ip inet, cd cidr, dcd nsk_cidr, t text, vc varchar(20), dvc nsk_vc);"
+psql_run "CREATE TABLE nb (id int, ip inet, cd cidr, dcd nsk_cidr, t text, vc varchar(20), dvc nsk_vc) USING pgcolumnar;"
+psql_run "SELECT pgcolumnar.set_options('nb', stripe_row_limit => 2048, chunk_group_row_limit => 1024);"
+NB_GEN="SELECT g, a::inet, a::cidr, a::cidr, s, s, s
+  FROM (SELECT g, '10.0.' || (g / 256) || '.' || (g % 256) AS a,
+               lpad(g::text, 6, '0') AS s
+        FROM generate_series(1, 20480) g) x"
+psql_run "INSERT INTO hb $NB_GEN;"
+psql_run "INSERT INTO nb $NB_GEN;"
+NB_IP="BETWEEN '10.0.20.0' AND '10.0.20.100'"
+NB_S="BETWEEN '005000' AND '005100'"
+for arm in "ip|inet|$NB_IP" "cd|cidr|$NB_IP" "dcd|a domain over cidr|$NB_IP" \
+	"t|text|$NB_S" "vc|varchar|$NB_S" "dvc|a domain over varchar|$NB_S"; do
+	col=${arm%%|*}; rest=${arm#*|}; what=${rest%%|*}; pred=${rest#*|}
+	check "a range on $what returns the same rows as heap (#1433)" \
+		"$(pgc_set_hash "SELECT id FROM nb WHERE $col $pred")" \
+		"$(pgc_set_hash "SELECT id FROM hb WHERE $col $pred")"
+	check "and a range on $what skips groups (#1433)" \
+		"$(gt0 "$(skipped "SELECT id FROM nb WHERE $col $pred")")" "yes"
+done
+# equality also takes the bloom probe, which hashes the constant with the
+# column's hash proc: the same proc for both types of each pair
+for arm in "cd|cidr|'10.0.40.7'" "vc|varchar|'012345'"; do
+	col=${arm%%|*}; rest=${arm#*|}; what=${rest%%|*}; val=${rest#*|}
+	check "equality on $what returns the same rows as heap (#1433)" \
+		"$(q "SELECT string_agg(id::text, ',' ORDER BY id) FROM nb WHERE $col = $val")" \
+		"$(q "SELECT string_agg(id::text, ',' ORDER BY id) FROM hb WHERE $col = $val")"
+	check "and equality on $what skips groups (#1433)" \
+		"$(gt0 "$(skipped "SELECT id FROM nb WHERE $col = $val")")" "yes"
+done
+
 pgc_summary

test/pytest/TESTS.md

@@ -178,6 +178,7 @@ behaviour, the source of that number is named.
 - [130. test_rls_query_paths.py: row-level security holds on every read path](#130-test_rls_query_pathspy-row-level-security-holds-on-every-read-path)
 - [131. test_type_matrix.py: types the differential matrix never stored answer as heap](#131-test_type_matrixpy-types-the-differential-matrix-never-stored-answer-as-heap)
 - [132. test_iceberg_data_files.py: a delete entry in a data manifest is refused](#132-test_iceberg_data_filespy-a-delete-entry-in-a-data-manifest-is-refused)
+- [133. test_native_skip.py: a type binary-coercible to its opclass's type prunes](#133-test_native_skippy-a-type-binary-coercible-to-its-opclasss-type-prunes)
 
 ## 1. How to read a test in here
 
@@ -7209,3 +7210,14 @@ A partial pytest twin of `test/iceberg_data_files.sh` (#1341 item 49). It ports
 | `test_the_control_entry_is_listed` | a data manifest with one added data entry lists that one file |
 | `test_an_existing_entry_is_listed` | an EXISTING (status 0) entry is listed too, so a lister that keeps only ADDED entries fails |
 | `test_a_delete_entry_is_refused` | a position-delete entry, an equality-delete entry and a DELETED entry are each refused with 0A000, and the backend stays up |
+
+## 133. test_native_skip.py: a type binary-coercible to its opclass's type prunes
+
+A partial pytest twin of `test/native_skip.sh` (#1433). It ports only the arms that issue added, by the shell's names; the rest of the shell suite is declared INCOMPLETE in `test_compare_to_bash.py`. A comparison on a cidr column resolves to inet's operator, and a varchar column's to text's. The reader called such a key cross-type and asked the opfamily for an ordering proc over the pair, which `network_ops` and `text_ops` do not register, so the key was dropped and the column pruned nothing. inet and text hold the same values and are the controls. Each arm asserts heap's answer, then that chunk groups were removed. A premise checks that heap's answer holds the rows the arm expects.
+
+### Every test
+
+| test | what it holds |
+| --- | --- |
+| `test_a_range_on_a_coercible_type_prunes` | a range on inet, cidr, a domain over cidr, text, varchar and a domain over varchar returns heap's rows and removes chunk groups |
+| `test_equality_on_a_coercible_type_prunes` | equality on cidr and on varchar, which also takes the bloom probe, returns heap's row and removes chunk groups |

test/pytest/expected_tests.txt

@@ -731,4 +731,6 @@ guard_tests 438
 # test_join_runtime_filter.py, by collection over the cluster files on this tree.
 # cluster_tests 604 -> 609 for #1341's test_iceberg_data_files.py, five tests (two
 # listed controls and three parametrized refusals), by collection over the cluster files.
-cluster_tests 609
+# cluster_tests 609 -> 617 for #1433's test_native_skip.py, eight tests (six ranges and
+# two equalities, parametrized), by collection over the cluster files on this tree.
+cluster_tests 617

test/pytest/test_compare_to_bash.py

@@ -174,6 +174,13 @@
 # about the PORT rather than a licence: the standing arm does not grade it, so the
 # reason is the only thing standing between a declared gap and a forgotten one.
 INCOMPLETE = {
+    # #1433 ported the arms it added: cidr, varchar and a domain over each prune
+    # as inet and text do, with heap's answer first. The rest of the shell suite
+    # (range, equality, bigint, the #477 and #483 arms, the partitioned table) is
+    # not.
+    "native_skip": "the #1433 binary-coercible arms and their inet and text "
+                   "controls are ported; the range, equality, bigint, #477, #483 "
+                   "and partitioned-table arms are not",
     # #1341 item 49 ported the arms it added: a delete or removed entry inside a
     # data manifest is refused, with a control that is listed. The rest of the
     # shell suite (the pyiceberg oracle, path rebasing and its escapes, the

test/pytest/test_native_skip.py

@@ -0,0 +1,88 @@
+"""A type binary-coercible to its opclass's type prunes row groups (#1433).
+
+A comparison on a cidr column resolves to inet's operator, and a varchar column's to
+text's, with the column relabelled. The reader received a key with a column of cidr
+(or varchar) and an argument of inet (or text), called it cross-type, and asked the
+opfamily for an ordering proc over the pair. network_ops and text_ops register one
+only for (inet, inet) and (text, text), so the key was dropped and the column pruned
+nothing, while EXPLAIN still counted the filter as pushed down.
+
+inet and text hold the same values and are the controls: the fixture prunes when the
+key is kept. Each arm asserts heap's answer before it asserts pruning, because using
+the wrong comparison would give a wrong answer, not only a slow one.
+
+This ports only the #1433 arms of `test/native_skip.sh`; the rest of that suite is
+declared INCOMPLETE in `test_compare_to_bash.py`. Independent of the shell arms:
+different names, values and row count.
+"""
+import re
+
+import pytest
+
+REMOVED = re.compile(r"Columnar Chunk Groups Removed by Filter: (\d+)")
+
+
+@pytest.fixture
+def tables(pgc_conn):
+    pgc_conn.execute("CREATE DOMAIN skp_cidr AS cidr")
+    pgc_conn.execute("CREATE DOMAIN skp_vc AS varchar(16)")
+    cols = "id int, ip inet, cd cidr, dcd skp_cidr, t text, vc varchar(16), dvc skp_vc"
+    pgc_conn.execute(f"CREATE TABLE skh ({cols})")
+    pgc_conn.execute(f"CREATE TABLE skn ({cols}) USING pgcolumnar")
+    pgc_conn.execute("SELECT pgcolumnar.set_options('skn', stripe_row_limit => 2000, "
+                     "chunk_group_row_limit => 1000)")
+    gen = ("SELECT g, a::inet, a::cidr, a::cidr, s, s, s FROM "
+           "(SELECT g, '172.16.' || (g / 200) || '.' || (g % 200) AS a, "
+           "'k' || lpad(g::text, 5, '0') AS s FROM generate_series(1, 18000) g) x")
+    pgc_conn.execute(f"INSERT INTO skh {gen}")
+    pgc_conn.execute(f"INSERT INTO skn {gen}")
+    return pgc_conn
+
+
+def _removed(conn, sql):
+    plan = "\n".join(r[0] for r in conn.execute(
+        "EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) " + sql).fetchall())
+    m = REMOVED.search(plan)
+    return int(m.group(1)) if m else 0
+
+
+def _ids(conn, table, where):
+    return conn.execute(f"SELECT string_agg(id::text, ',' ORDER BY id) FROM {table} "
+                        f"WHERE {where}").fetchone()[0]
+
+
+IP_RANGE = "BETWEEN '172.16.30.0' AND '172.16.30.120'"
+S_RANGE = "BETWEEN 'k06000' AND 'k06120'"
+
+
+@pytest.mark.parametrize("col, what, pred", [
+    ("ip", "inet", IP_RANGE),
+    ("cd", "cidr", IP_RANGE),
+    ("dcd", "a domain over cidr", IP_RANGE),
+    ("t", "text", S_RANGE),
+    ("vc", "varchar", S_RANGE),
+    ("dvc", "a domain over varchar", S_RANGE),
+])
+def test_a_range_on_a_coercible_type_prunes(tables, expect, col, what, pred):
+    heap = _ids(tables, "skh", f"{col} {pred}")
+    expect.num(len(heap.split(",")) if heap else 0, 121,
+               f"premise: heap's range on {what} holds 121 rows")
+    expect.text(_ids(tables, "skn", f"{col} {pred}"), heap,
+                f"a range on {what} returns the same rows as heap (#1433)")
+    expect.num(min(_removed(tables, f"SELECT id FROM skn WHERE {col} {pred}"), 1), 1,
+               f"and a range on {what} skips groups (#1433)")
+
+
+@pytest.mark.parametrize("col, what, val", [
+    ("cd", "cidr", "'172.16.55.9'"),
+    ("vc", "varchar", "'k11009'"),
+])
+def test_equality_on_a_coercible_type_prunes(tables, expect, col, what, val):
+    """Equality also takes the bloom probe, which hashes the constant with the
+    column's hash proc: the same proc for both types of each pair."""
+    expect.text(_ids(tables, "skn", f"{col} = {val}"), "11009",
+                f"equality on {what} returns the same rows as heap (#1433)")
+    expect.text(_ids(tables, "skh", f"{col} = {val}"), "11009",
+                f"premise: heap's equality on {what} finds row 11009")
+    expect.num(min(_removed(tables, f"SELECT id FROM skn WHERE {col} = {val}"), 1), 1,
+               f"and equality on {what} skips groups (#1433)")

test/pytest/test_type_matrix.py

@@ -69,7 +69,7 @@ def _both(conn, sql):
     "ch": "every row group holds every value, since the column cycles through 26 letters",
     "e": "every row group holds every value, since the column cycles through the 4 labels",
     "n": "every row group holds NaN and both infinities, so each zone map spans every value",
-    "cd": "a cidr key is treated as cross-type against inet and dropped (#1433)",
+    # cd was pinned here until #1433, when a cidr key was dropped as cross-type
 }
 
 

test/type_matrix.sh

@@ -88,13 +88,13 @@ diff_query "each column's count and NULL count is heap's" "SELECT $COUNTS FROM %
 # review of #1426). Neither id is NULL in any column.
 #
 # Each arm also says whether the zone maps skipped chunk groups, from EXPLAIN ANALYZE,
-# so an arm that should prune cannot pass on a scan that read everything. Four measure
-# 0 of 12 on PG17 and PG18, and are pinned at 0 with the reason:
+# so an arm that should prune cannot pass on a scan that read everything. Three measure
+# 0 of 12 on PG17 and PG18, and are pinned at 0 with the reason. cd was a fourth until
+# #1433, when a cidr key was treated as cross-type against inet and dropped:
 declare -A NO_PRUNE=(
 	[ch]="every row group holds every value, since the column cycles through 26 letters"
 	[e]="every row group holds every value, since the column cycles through the 4 labels"
-	[n]="every row group holds NaN and both infinities, so each zone map spans every value"
-	[cd]="a cidr key is treated as cross-type against inet and dropped (#1433)")
+	[n]="every row group holds NaN and both infinities, so each zone map spans every value")
 removed() {	# removed SQL -> the columnar scan's "Chunk Groups Removed by Filter"
 	q "EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) ${1//%T/t_col}" |
 		sed -n 's/.*Columnar Chunk Groups Removed by Filter: \([0-9]*\).*/\1/p' | head -1