diff --git a/CHANGELOG.md b/CHANGELOG.md index 707a7e5b..0c2406b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,22 @@ true until the next version shipped. `rte->tablesample` is set, so the query reaches the callbacks and raises. An unsampled query on the same table still takes the custom scan. +- A custom scan no longer hides the children of an `INHERITS` hierarchy (#871). + + A legacy inheritance parent is an ordinary `RELKIND_RELATION` carrying + `rte->inh`. `set_rel_pathlist_hook` still runs on that appendrel after core + has built the `Append` over its children, and a custom scan added there reads + only the parent's own storage. Every child's rows became invisible, with no + error and no warning. Measured with a heap mirror as the oracle, parent 1 row + and child 5000: `SELECT count(*) FROM parent` returned **1 where 5001 is + right**, from a plan containing no `Append`. The ungrouped vector aggregate + path had the same hole; the grouped path already refused this shape. + + Both hooks now decline when `rte->inh` is set, leaving the hierarchy to the + ordinary `Append` plan. The children keep the custom scan, because a child's + own range-table entry has `inh` false: the resulting plan is an `Append` of + two `PgColumnarScan` nodes with pushdown intact, not a fallback to `Seq Scan`. + - A shebang and the execute bit go together, and every directory that documents a command is swept (#856). Two things were left over from #852. diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index 90070e53..58f2194d 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -2535,6 +2535,18 @@ PgColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, */ if (rte->tablesample != NULL) return; + /* + * A legacy inheritance parent is a plain RELKIND_RELATION with rte->inh + * set. set_rel_pathlist_hook still runs on that appendrel after core has + * built the Append of its children, and a Custom Scan added here reads + * only the parent's storage. Measured: parent 1 row, child 5000 rows, + * SELECT count(*) FROM parent returned 1 (the heap mirror returned 5001) + * from a plan with no Append. Leave the tree to Append. Children and + * the parent-as-member still receive this scan: those RTEs have inh + * false. Grouped vector aggregation already refuses the same shape. + */ + if (rte->inh) + return; /* * A partition is RELOPT_OTHER_MEMBER_REL, not RELOPT_BASEREL, and excluding * it cost the custom scan entirely: no column projection, no zone-map diff --git a/src/columnar_vector.c b/src/columnar_vector.c index f646d75f..eb5eb231 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -961,6 +961,15 @@ PgColumnarCreateUpperPaths(PlannerInfo *root, UpperRelationKind stage, return; if (rte->tablesample != NULL) return; + /* + * A legacy inheritance parent is a plain RELKIND_RELATION with rte->inh set; + * its children hold rows this single-relation scan would never see. Leave the + * whole tree to the ordinary Append + Agg plan. The grouped path already + * refuses this; the ungrouped path did not, so SELECT count(*) FROM parent + * returned only the parent's own rows. + */ + if (rte->inh) + return; if (!OidIsValid(rte->relid) || !PgColumnarIsColumnarRelation(rte->relid)) return; relid = rte->relid; diff --git a/test/inheritance.sh b/test/inheritance.sh new file mode 100755 index 00000000..c392c0e2 --- /dev/null +++ b/test/inheritance.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# +# pgColumnar: a legacy INHERITS parent must plan as Append of its children, not +# as a custom scan (or vectorized aggregate) of the parent's storage alone. +# +# PgColumnarSetRelPathlist and the ungrouped vector-aggregate path both run on +# the inheritance appendrel (RELKIND_RELATION with rte->inh set). A scan added +# there reads only the parent's own file. Measured: parent 1 row, child 5000 +# rows, SELECT count(*) FROM parent returned 1 where the heap mirror returned +# 5001, from a plan with no Append. Grouped vector aggregation already refused +# this shape; the other two hooks did not. +# +# Declarative partitions are a different RTE (RELKIND_PARTITIONED_TABLE) and +# already go through Append of OTHER_MEMBER_REL children. That path stays. +# +# Usage: test/inheritance.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}" + +plan() { + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq -c "$1" +} + +psql_run "CREATE TABLE inh_h (id int, v text);" +psql_run "CREATE TABLE inh_hc () INHERITS (inh_h);" +psql_run "INSERT INTO inh_h VALUES (1, 'parent');" +psql_run "INSERT INTO inh_hc VALUES (2, 'child');" + +psql_run "CREATE TABLE inh_p (id int, v text) USING pgcolumnar;" +psql_run "CREATE TABLE inh_c () INHERITS (inh_p) USING pgcolumnar;" +psql_run "INSERT INTO inh_p VALUES (1, 'parent');" +psql_run "INSERT INTO inh_c VALUES (2, 'child');" + +check "INHERITS parent includes the child (heap oracle)" \ + "$(q "SELECT count(*) FROM inh_h")" "2" +check "INHERITS parent includes the child (columnar)" \ + "$(q "SELECT count(*) FROM inh_p")" "2" +check "ONLY parent is still just the parent" \ + "$(q "SELECT count(*) FROM ONLY inh_p")" "1" +check "the child is visible through the parent" \ + "$(q "SELECT v FROM inh_p WHERE id = 2")" "child" +check "row identities match the heap" \ + "$(pgc_set_hash 'SELECT id, v FROM inh_p')" \ + "$(pgc_set_hash 'SELECT id, v FROM inh_h')" + +star_plan="$(plan "EXPLAIN (COSTS OFF) SELECT * FROM inh_p;")" +check "SELECT * from an INHERITS parent plans an Append" \ + "$(printf '%s' "$star_plan" | grep -c Append)" "1" + +count_plan="$(plan "EXPLAIN (COSTS OFF) SELECT count(*) FROM inh_p;")" +check "count(*) from an INHERITS parent plans an Append" \ + "$(printf '%s' "$count_plan" | grep -c Append)" "1" + +# A cheap parent and a large child is the shape that made the custom scan win: +# the appendrel is costed from the parent's empty-looking storage. +psql_run "INSERT INTO inh_c SELECT g, 'c'||g FROM generate_series(10,5000) g;" +psql_run "INSERT INTO inh_hc SELECT g, 'c'||g FROM generate_series(10,5000) g;" +psql_run "ANALYZE inh_p; ANALYZE inh_c; ANALYZE inh_h; ANALYZE inh_hc;" + +check "a large child is still visible through the parent" \ + "$(q "SELECT count(*) FROM inh_p")" \ + "$(q "SELECT count(*) FROM inh_h")" +check "filtered rows match the heap" \ + "$(pgc_set_hash 'SELECT id FROM inh_p WHERE id > 100')" \ + "$(pgc_set_hash 'SELECT id FROM inh_h WHERE id > 100')" + +vec_count="$(q "SET pgcolumnar.enable_ungrouped_vector_agg = on; + SELECT count(*) FROM inh_p;")" +check "ungrouped vector aggregation also sees the child rows" \ + "$(printf '%s' "$vec_count" | tail -1)" \ + "$(q "SELECT count(*) FROM inh_h")" + +# Declarative partitions were the #436 case: the custom scan MUST still fire on +# the child, or this refusal of rte->inh would put them back on a seqscan. +psql_run "CREATE TABLE inh_prt (id int, v text) PARTITION BY RANGE (id);" +psql_run "CREATE TABLE inh_prt1 PARTITION OF inh_prt FOR VALUES FROM (0) TO (100000) + USING pgcolumnar;" +psql_run "INSERT INTO inh_prt SELECT g, 'p'||g FROM generate_series(1,2000) g;" +check "a partitioned parent still returns every row" \ + "$(q "SELECT count(*) FROM inh_prt")" "2000" +prt_plan="$(plan "EXPLAIN (COSTS OFF) SELECT * FROM inh_prt;")" +check "and the partition is still a PgColumnarScan (#436)" \ + "$(printf '%s' "$prt_plan" | grep -c 'Custom Scan (PgColumnarScan)')" "1" + +pgc_summary diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 5178861f..3e297c99 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -112,6 +112,7 @@ SUITES=( index_delete_liveness index_fetch_penalty_width index_only + inheritance int8_agg_int128 isolation local_open_race_free