Refuse an array scan key before anything can build one (#752)

src/columnar_reader.c

@@ -648,10 +648,28 @@ pgcolumnar_make_predicates(SkipPredicate *out, int nkeys, ScanKey keys,
 		Oid			argType;
 		bool		crossType;
 
-		/* only plain "column op const" comparison keys are usable */
+		/*
+		 * Only plain "column op const" comparison keys are usable.
+		 *
+		 * SK_SEARCHARRAY is in this list although nothing in this extension
+		 * currently sets it, and the reason is the point. A key carrying that
+		 * flag holds an ARRAY in sk_argument, not a scalar. Without this arm the
+		 * key falls through, compareValue is set to the array's Datum, and every
+		 * zone-map comparison below runs the column's scalar btree function
+		 * against a pointer to an array header. That is not a wrong answer that
+		 * a test would catch; it is a comparison of unrelated things whose
+		 * result is whatever the memory happens to say.
+		 *
+		 * It is guarded BEFORE anything can produce the shape rather than
+		 * beside it. #752 measures a per-element path for `= ANY` worth 13 to 16
+		 * chunk groups of 27, and the obvious way to build it is to stop
+		 * collapsing the array in pgcolumnar_saop_range_scankey and mark the key
+		 * SK_SEARCHARRAY. Whoever does that should have to add handling here
+		 * deliberately, not discover that this function accepted it silently.
+		 */
 		if (key->sk_flags & (SK_ISNULL | SK_ROW_HEADER | SK_ROW_MEMBER |
 							 SK_ROW_END | SK_SEARCHNULL | SK_SEARCHNOTNULL |
-							 SK_ORDER_BY))
+							 SK_ORDER_BY | SK_SEARCHARRAY))
 			continue;
 		if (key->sk_attno < 1 || key->sk_attno > natts)
 			continue;

test/native_saop_pushdown.sh

@@ -218,6 +218,55 @@ check "a correlated PARAM_EXEC array is not mis-pruned (results stay correct)" \
 	"$(q 'SELECT sum(c) FROM thr, LATERAL (SELECT count(*) c FROM t WHERE t.ts = ANY (thr.lo)) s;')" \
 	"3"
 
+# --- an array scan key must be refused before one can be built ------------
+#
+# This is a source assertion, in the style of native_fetch_cache.sh, and the
+# reason is the same one that suite records: there is no behavioural test to
+# write, because nothing in the extension can currently produce the shape. The
+# builder above collapses every multi-element array into two range keys, so no
+# SK_SEARCHARRAY key ever reaches pgcolumnar_make_predicates.
+#
+# It is guarded anyway, because the defect it prevents is silent. Such a key
+# holds an ARRAY in sk_argument. Unrejected, it becomes a SkipPredicate whose
+# compareValue is the array's Datum, and the column's scalar btree comparison
+# then runs against a pointer to an array header. That is not a wrong answer a
+# suite could catch, it is a comparison of unrelated things.
+#
+# THE ASSERTION IS ABOUT THE EXPRESSION, NOT ABOUT THE FILE, and that distinction
+# is the whole check. An earlier draft grepped each flag name over the whole
+# file, which is a claim about the file: deleting SK_ISNULL from the guard while
+# naming it in a nearby comment left the suite printing
+# "PASS  and it still refuses SK_ISNULL" on a tree that no longer refused it.
+# Measured, not supposed. The convention this very guard introduces -- explain in
+# prose why a flag is rejected -- is what would blind a file-wide grep for the
+# next flag anyone documents.
+#
+# So the reject expression is extracted once and membership is asserted inside
+# it. Extraction is a premise, because an expression that failed to extract is
+# an empty string and every "is this flag in it" test would then compare one
+# blank with another and pass (#823).
+SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/src"
+
+check "premise: the predicate builder has exactly one sk_flags reject guard" \
+	"$(grep -c 'key->sk_flags &' "$SRC/columnar_reader.c")" "1"
+
+# From the `if (key->sk_flags & (` line through the closing `))`, comments
+# excluded by construction: the range starts at the `if`, so prose above it
+# cannot be captured.
+guard="$(awk '/if \(key->sk_flags & \(/,/\)\)/' "$SRC/columnar_reader.c" | tr -d ' \t\n')"
+
+check "premise: the reject expression was extracted, not blank" \
+	"$([ -n "$guard" ] && [ "${guard#*sk_flags}" != "$guard" ] && echo yes \
+		|| echo "extracted=<${guard:-empty}>")" "yes"
+
+# Membership, not adjacency. The flag list is a SET; asserting the order of it
+# would redden on a harmless reflow and would assert more than the code means.
+for _f in SK_ISNULL SK_ROW_HEADER SK_ROW_MEMBER SK_ROW_END SK_SEARCHNULL \
+		  SK_SEARCHNOTNULL SK_ORDER_BY SK_SEARCHARRAY; do
+	check "the predicate builder's reject expression contains $_f" \
+		"$(case "$guard" in *"$_f"*) echo yes ;; *) echo "absent from <$guard>" ;; esac)" "yes"
+done
+
 check "backend alive" "$(q 'SELECT 1;')" "1"
 
 pgc_summary