fix: refuse a uint32-truncated column chunk on index fetch (#1063, rebased + ledger)

CHANGELOG.md

@@ -615,6 +615,18 @@ true until the next version shipped.
   This is the third arm in this file to be repaired for counting a string across a
   whole file. The `deltuples` comment 15 lines above records the first, fixed by
   scoping; these two were left as whole-file counts and did the same thing again.
+- An index fetch silently returned a row when `page_length` was 2^32 too large.
+
+  `NativeColumnChunkMetadata.pageLength` is `uint64`. Both decode entry points
+  cast `(pageLength - validityBytes)` to `uint32`. Adding 2^32 to the catalog
+  value leaves the low 32 bits unchanged, so a btree fetch reconstructed the
+  original stream and returned the row. A sequential scan already refused: the
+  chunk no longer fitted its row group, so containment raised XX001. The fetch
+  path never had that check.
+
+  The value-stream length is now required to fit in `uint32` before either path
+  decodes. Adding 2^32 is refused with XX001 on the fetch and on the scan. New
+  twins `native_chunk_length_bound` and `test_native_chunk_length_bound.py`.
 
 - `compare_to_bash.py`'s corpus arm called a WRAPPED name fabricated. A name too long
   for one line is written as adjacent literals, and Python joins them at parse time,

src/columnar_reader.c

@@ -1027,6 +1027,40 @@ pgcolumnar_read_start(PgColumnarReadState *readState)
 	}
 }
 
+
+/*
+ * pgcolumnar_chunk_value_bytes
+ *		The value stream that follows a chunk's validity bitmap, as a uint32.
+ *
+ *		page_length is uint64 in the catalog. Both decode entry points used to
+ *		cast (page_length - validityBytes) to uint32. Adding 2^32 to page_length
+ *		leaves the low 32 bits unchanged, so an index fetch silently read the
+ *		original stream and returned the row. A sequential scan already refused
+ *		(the chunk no longer fitted its row group). Refuse here so a fetch cannot
+ *		truncate.
+ */
+static uint32
+pgcolumnar_chunk_value_bytes(uint64 pageLength, int validityBytes, int attnum)
+{
+	uint64		vbytes;
+
+	if (validityBytes < 0)
+		validityBytes = 0;
+	if ((uint64) validityBytes > pageLength)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("columnar chunk for column %d has a validity bitmap longer than the chunk",
+						attnum)));
+	vbytes = pageLength - (uint64) validityBytes;
+	if (vbytes > (uint64) PG_UINT32_MAX)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("columnar chunk for column %d is too large to decode",
+						attnum),
+				 errdetail("Value stream is " UINT64_FORMAT " bytes.", vbytes)));
+	return (uint32) vbytes;
+}
+
 /*
  * pgcolumnar_native_decode_chunk
  *		Reconstruct a native column chunk's raw present-value stream (D4) from its
@@ -2718,7 +2752,9 @@ pgcolumnar_native_load_group(PgColumnarReadState *rs)
 			/* D4: reconstruct the raw present-value stream from the descriptor */
 			rs->nativeValueCursor[cc->columnIndex] =
 				pgcolumnar_native_decode_chunk(rs->groupContext, att, base + validityBytes,
-											 (uint32) (cc->pageLength - validityBytes),
+											 pgcolumnar_chunk_value_bytes(cc->pageLength,
+																		   validityBytes,
+																		   cc->columnIndex + 1),
 											 cc->encodingDescriptor,
 											 cc->encodingDescriptorLen,
 											 cc->blockCodec, &vraw, &vcount,
@@ -4366,7 +4402,9 @@ pgcolumnar_fetch_row(Relation rel, Snapshot snapshot, uint64 rowNumber,
 									COLUMNAR_NATIVE_ENCDESC_BASELINE);
 			MemoryContext decCx;
 			MemoryContext decOld;
-			uint32		vlen = (uint32) (cc->pageLength - validityBytes);
+			uint32		vlen = pgcolumnar_chunk_value_bytes(cc->pageLength,
+												   validityBytes,
+												   c + 1);
 			char	   *vstream;
 
 			if (entry->overflow[c])

test/check_ledger.tsv

@@ -1169,6 +1169,12 @@ harness_selftest	490-a-one-line-body-ends-at-its-own-brace	control: and the reco
 harness_selftest	490-a-one-line-body-ends-at-its-own-brace	premise: the skip-loop tool is present	15;16;17;18;19	never	-
 harness_selftest	490-a-one-line-body-ends-at-its-own-brace	premise: the tool answers with an emitter set, so 'q is absent' is about q	15;16;17;18;19	never	-
 harness_selftest	490-a-one-line-body-ends-at-its-own-brace	premise: the tool reports an unclosed count at all, so a zero below means zero	15;16;17;18;19	never	-
+native_chunk_length_bound	native_chunk_length_bound	a sequential scan of the same poisoned chunk is refused (XX001)	15;16;17;18;19	never	-
+native_chunk_length_bound	native_chunk_length_bound	an index fetch of a chunk whose page_length is 2^32 too large is refused (XX001)	15;16;17;18;19	never	-
+native_chunk_length_bound	native_chunk_length_bound	backend survived the sequential refusal	15;16;17;18;19	never	-
+native_chunk_length_bound	native_chunk_length_bound	backend survived the truncated-length fetch	15;16;17;18;19	never	-
+native_chunk_length_bound	native_chunk_length_bound	premise: a point lookup uses the index, not a sequential columnar scan	15;16;17;18;19	never	-
+native_chunk_length_bound	native_chunk_length_bound	premise: that fetch returns the row	15;16;17;18;19	never	-
 native_join_runtime_filter	native_join_runtime_filter	3-table answer equals filter-off	15;16;17;18;19	never	-
 native_join_runtime_filter	native_join_runtime_filter	3-table join order matches filter-off	15;16;17;18;19	never	-
 native_join_runtime_filter	native_join_runtime_filter	3-table plan has coordinator	15;16;17;18;19	never	-

test/check_ledger_budget.txt

@@ -58,4 +58,4 @@ suites_not_covered 249
 # that is not this one. Neither survives. Re-derived by COUNTING on the merged tree,
 # which is the only resolution this number has:
 #     awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l
-checks_never_observed_red 1222
+checks_never_observed_red 1228

test/native_chunk_length_bound.sh

@@ -0,0 +1,70 @@
+#!/usr/bin/env bash
+#
+# A column chunk's page_length is uint64 in the catalog and on the metadata
+# struct, but both decode entry points cast the value stream to uint32. Adding
+# 2^32 to page_length leaves the low 32 bits unchanged, so an index fetch
+# silently reads the original stream and returns the row. A sequential scan
+# already refuses (the chunk no longer fits its row group). The fetch path
+# never had that check.
+#
+# Public seam: poison pgcolumnar.column_chunk.page_length, then SELECT through
+# a btree. The property is the SQLSTATE, not a cost number.
+#
+# Independent of test/pytest/test_native_chunk_length_bound.py: same public
+# seam, own fixture, own observations.
+#
+# Usage:  test/native_chunk_length_bound.sh [PG_CONFIG]
+# Written fresh for pgColumnar.
+
+set -uo pipefail
+. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
+pgc_setup "${1:-/usr/local/pg17/bin/pg_config}"
+
+q "CREATE EXTENSION IF NOT EXISTS pgcolumnar;" >/dev/null
+q "CREATE TABLE clb (id int, t text) USING pgcolumnar;
+   INSERT INTO clb SELECT g, 'v'||g FROM generate_series(1,5000) g;
+   CREATE INDEX clb_id ON clb(id);
+   ANALYZE clb;" >/dev/null
+SID="$(q "SELECT pgcolumnar.get_storage_id('clb');")"
+
+errcode() {
+	env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \
+		-qtA -v VERBOSITY=sqlstate -c "$1" 2>&1 \
+		| sed -n 's/^ERROR:  \([0-9A-Z]\{5\}\).*/\1/p' | head -1
+}
+alive() { [ "$(q 'SELECT 1;')" = "1" ] && echo yes || echo no; }
+plan_scan() {
+	env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \
+		-Atq -c "SET enable_seqscan=off; SET enable_bitmapscan=off;
+		         SET pgcolumnar.enable_custom_scan=off;
+		         EXPLAIN (COSTS OFF) $1" 2>&1 \
+		| grep -m1 -oE 'Index Scan|Index Only Scan|Bitmap Heap Scan|Custom Scan|Seq Scan'
+}
+
+FETCH="SET enable_seqscan=off; SET enable_bitmapscan=off; SET pgcolumnar.enable_custom_scan=off;"
+
+check "premise: a point lookup uses the index, not a sequential columnar scan" \
+	"$(plan_scan 'SELECT t FROM clb WHERE id = 1')" "Index Scan"
+fetch_val() {
+	env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \
+		-Atq -c "$1" 2>&1 | grep -v '^SET$' | tail -1
+}
+check "premise: that fetch returns the row" \
+	"$(fetch_val "${FETCH} SELECT t FROM clb WHERE id = 1;")" "v1"
+
+q "UPDATE pgcolumnar.column_chunk SET page_length = page_length + 4294967296
+   WHERE storage_id = $SID AND column_index = 1;" >/dev/null
+
+check "an index fetch of a chunk whose page_length is 2^32 too large is refused (XX001)" \
+	"$(errcode "${FETCH} SELECT t FROM clb WHERE id = 1;")" "XX001"
+check "backend survived the truncated-length fetch" "$(alive)" "yes"
+
+# The sequential path already refuses a chunk that does not fit its row group.
+# Pin that so a "fix" that only silences the fetch is not enough, and so this
+# suite still means something if the fetch path starts using the same guard.
+check "a sequential scan of the same poisoned chunk is refused (XX001)" \
+	"$(errcode "SET pgcolumnar.enable_custom_scan=on; SET enable_indexscan=off;
+	            SELECT t FROM clb WHERE id = 1;")" "XX001"
+check "backend survived the sequential refusal" "$(alive)" "yes"
+
+pgc_summary

test/pytest/TESTS.md

@@ -89,6 +89,7 @@ behaviour, the source of that number is named.
 - [41. test_projections.py: a second copy of some columns, kept honest](#41-test_projectionspy-a-second-copy-of-some-columns-kept-honest)
 - [42. test_compression_reaches_the_cascade.py: the codec setting decides encodings too](#42-test_compression_reaches_the_cascadepy-the-codec-setting-decides-encodings-too)
 - [43. test_pgxn_metadata.py: the published distribution metadata, which nothing read](#43-test_pgxn_metadatapy-the-published-distribution-metadata-which-nothing-read)
+- [44. test_native_chunk_length_bound.py: a truncated chunk length cannot fetch](#44-test_native_chunk_length_boundpy-a-truncated-chunk-length-cannot-fetch)
 
 ## 1. How to read a test in here
 
@@ -4338,3 +4339,21 @@ Removal proof, run on both harnesses: restore `META.json` as it shipped and the
 substantive arms redden on each side while every premise stays green. The premises
 hold because the file still parses and still names *a* script -- it names the wrong
 one, which is exactly the distinction the arms draw.
+
+## 44. test_native_chunk_length_bound.py: a truncated chunk length cannot fetch
+
+A column chunk's `page_length` is `uint64` in the catalog. Both decode entry
+points used to cast the value stream to `uint32`. Adding 2^32 leaves the low
+32 bits unchanged, so an index fetch silently read the original stream and
+returned the row. A sequential scan already refused, because the chunk no
+longer fitted its row group.
+
+This file asserts the SQLSTATE, not a cost number. The poison is a catalog
+UPDATE; the property is that a fetch raises XX001 and the backend survives.
+
+Independent of `test/native_chunk_length_bound.sh`. Same public seam, own
+fixture, own observations. Assertion names match the shell suite.
+
+| test | what it asserts |
+| --- | --- |
+| `test_native_chunk_length_bound` | a point lookup uses the index and returns the row; after `page_length` grows by 2^32, both the fetch and a sequential scan raise XX001 and the backend survives each |

test/pytest/expected_tests.txt

@@ -237,4 +237,5 @@ guard_tests 346
 # `guard_tests` was re-derived in the same run and did NOT move -- 342 -- which is
 # the expected answer for a file that needs a cluster, and checking it was the point
 # rather than assuming it.
-cluster_tests 410
+# cluster_tests re-derived by collection on the rebased tree, never by adding a delta measured on another tree.
+cluster_tests 411

test/pytest/test_compare_to_bash.py

@@ -86,7 +86,8 @@
 # The shape is `SHELL_REFERENCES`' in `test_harness_deps.py`, asserted in both
 # directions for the same reason: a one-way list rots into a permanent exemption.
 COMPLETE = ["differential", "hilbert_cluster", "hilbert_locality",
-            "native_ownership", "native_projection", "projection_privilege",
+            "native_chunk_length_bound", "native_ownership", "native_projection",
+            "projection_privilege",
             "projections",
             "sorted_pathkeys", "stats_privilege", "zonemap_boundaries"]
 

test/pytest/test_native_chunk_length_bound.py

@@ -0,0 +1,95 @@
+"""A column chunk's page_length is uint64 in the catalog, but both decode
+entry points cast the value stream to uint32. Adding 2^32 to page_length
+leaves the low 32 bits unchanged, so an index fetch silently reads the
+original stream and returns the row.
+
+Independent of test/native_chunk_length_bound.sh: same public seam, own
+fixture, own observations. Assertion names match the shell suite.
+"""
+
+import pytest
+
+
+def _scan_node(plan):
+    stack = [plan[0]["Plan"]]
+    while stack:
+        node = stack.pop(0)
+        t = node["Node Type"]
+        if t in (
+            "Index Scan",
+            "Index Only Scan",
+            "Bitmap Heap Scan",
+            "Custom Scan",
+            "Seq Scan",
+        ):
+            return t
+        stack.extend(node.get("Plans", ()))
+    return ""
+
+
+def test_native_chunk_length_bound(pgc_conn, expect):
+    with pgc_conn.cursor() as cur:
+        cur.execute("CREATE TABLE clb (id int, t text) USING pgcolumnar")
+        cur.execute(
+            "INSERT INTO clb SELECT g, 'v' || g::text FROM generate_series(1, 5000) g"
+        )
+        cur.execute("CREATE INDEX clb_id ON clb(id)")
+        cur.execute("ANALYZE clb")
+        cur.execute("SELECT pgcolumnar.get_storage_id('clb')")
+        sid = cur.fetchone()[0]
+        cur.execute("SET enable_seqscan=off")
+        cur.execute("SET enable_bitmapscan=off")
+        cur.execute("SET pgcolumnar.enable_custom_scan=off")
+        cur.execute("EXPLAIN (FORMAT JSON, COSTS OFF) SELECT t FROM clb WHERE id = 1")
+        plan = cur.fetchone()[0]
+    expect.text(
+        _scan_node(plan),
+        "Index Scan",
+        "premise: a point lookup uses the index, not a sequential columnar scan",
+    )
+
+    with pgc_conn.cursor() as cur:
+        cur.execute("SET enable_seqscan=off")
+        cur.execute("SET enable_bitmapscan=off")
+        cur.execute("SET pgcolumnar.enable_custom_scan=off")
+        cur.execute("SELECT t FROM clb WHERE id = 1")
+        got = cur.fetchone()[0]
+    expect.text(got, "v1", "premise: that fetch returns the row")
+
+    with pgc_conn.cursor() as cur:
+        cur.execute(
+            "UPDATE pgcolumnar.column_chunk SET page_length = page_length + 4294967296 "
+            "WHERE storage_id = %s AND column_index = 1",
+            (sid,),
+        )
+
+    with pgc_conn.cursor() as cur:
+        cur.execute("SET enable_seqscan=off")
+        cur.execute("SET enable_bitmapscan=off")
+        cur.execute("SET pgcolumnar.enable_custom_scan=off")
+        with pytest.raises(Exception) as fetch_err:
+            cur.execute("SELECT t FROM clb WHERE id = 1")
+    expect.sqlstate(
+        fetch_err.value,
+        "XX001",
+        "an index fetch of a chunk whose page_length is 2^32 too large is refused (XX001)",
+    )
+
+    with pgc_conn.cursor() as cur:
+        cur.execute("SELECT 1")
+        expect.num(cur.fetchone()[0], 1, "backend survived the truncated-length fetch")
+
+    with pgc_conn.cursor() as cur:
+        cur.execute("SET pgcolumnar.enable_custom_scan=on")
+        cur.execute("SET enable_indexscan=off")
+        with pytest.raises(Exception) as scan_err:
+            cur.execute("SELECT t FROM clb WHERE id = 1")
+    expect.sqlstate(
+        scan_err.value,
+        "XX001",
+        "a sequential scan of the same poisoned chunk is refused (XX001)",
+    )
+
+    with pgc_conn.cursor() as cur:
+        cur.execute("SELECT 1")
+        expect.num(cur.fetchone()[0], 1, "backend survived the sequential refusal")

test/run_all_versions.sh

@@ -132,6 +132,7 @@ SUITES=(
 	native_batch_fold_projection
 	native_bloom
 	native_cancel
+	native_chunk_length_bound
 	native_cluster
 	native_compact
 	native_ctas