diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 0214e1c..3a059b6 100644 --- a/src/columnar_reader.c +++ b/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; diff --git a/test/native_saop_pushdown.sh b/test/native_saop_pushdown.sh index 3d2c63a..dbc2b79 100755 --- a/test/native_saop_pushdown.sh +++ b/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