fix: do not let a custom scan hide INHERITS children

[OffgridwithJD]

Summary

• PgColumnarSetRelPathlist and the ungrouped vector-aggregate path both ran on a legacy inheritance appendrel (RELKIND_RELATION with rte->inh). A scan added there reads only the parent's storage.
• SELECT count(*) FROM parent returned 1 against a heap mirror of 5001 (parent 1 row, child 5000), from a plan with no Append. Grouped vector aggregation already refused this shape.
• Skip those hooks when rte->inh is set. Children and the parent-as-member still get a custom scan (inh is false there). Declarative partitions are unchanged (RELKIND_PARTITIONED_TABLE).

Test plan

• test/inheritance.sh on PostgreSQL 18 in cusor-2604 (INHERITS matches heap; Append in the plan; partitioned parent still uses PgColumnarScan)
• SELECT * / count(*) / filtered queries on an INHERITS parent with data in both parent and child
• Declarative PARTITION BY parent still returns every row via PgColumnarScan on the partition

Made with Cursor

[OffgridwithJD]

Premise verified — and it is the most serious of the current batch

Measured on pg18a against 8b39053, with a heap mirror as the oracle: parent
1 row, child 5000 rows.

                              main      #871
heap parent count(*)          5001      5001
columnar parent count(*)      1         5001
Append nodes in the plan      0         1

SELECT count(*) FROM parent returns 1 where the answer is 5001. No error, no
warning, and the query is about as ordinary as they come: an unqualified count on a
standard PostgreSQL feature. 5000 rows are simply invisible. The summary is
accurate and, if anything, undersells it.

The blast radius is contained — measured, since this is a return added to a hook

if (rte->inh) return; in PgColumnarSetRelPathlist disables the custom scan for a
whole class of RTE, so the question is what else loses it. Same run, both arms:

                                    main    #871
plain table, PgColumnarScan          1       1
plain table count(*) plan node       PgColumnarScan  PgColumnarScan
SELECT ... FROM ONLY parent          1       1

So rte->inh is not set for a childless table or an ONLY reference by the time
the hook runs, and neither loses its scan. That is the thing I would not have taken
on trust, because if it had gone the other way every correctness suite in the tree
would still have passed — the answers would be right and merely slow.

The claim about children holds

The comment says children and the parent-as-member keep the scan. They do:

Append
  ->  Custom Scan (PgColumnarScan) on ic ic_1      Pushed-Down Filters: 1
  ->  Custom Scan (PgColumnarScan) on icc ic_2     Pushed-Down Filters: 1
Seq Scan nodes: 0

Projection and qual pushdown both survive. Nothing is deoptimized.

That also answers your two unchecked boxes: filtered and count(*) queries on a
parent with data on both sides match the heap, and the declarative partition still
plans a PgColumnarScan.

The gap I would close before this lands

No arm pins that the inheritance children keep their custom scan. Your suite
asserts the counts match the heap, that an Append appears, and that a partition
is still a PgColumnarScan. A broader version of this fix — one that also skipped
the children, not just the appendrel parent — would pass every one of those arms
while quietly turning every inheritance query into Seq Scan. The counts would
still be right, which is exactly why the suite would not notice.

The plan above is the missing assertion, and it is one line:

inh_plan="$(plan "EXPLAIN (COSTS OFF) SELECT id FROM inh_p WHERE id > 100;")"
check "the Append's members are still columnar scans, not seq scans" \
    "$(printf '%s' "$inh_plan" | grep -c 'Custom Scan (PgColumnarScan)')" "2"

Without it the suite pins correctness but not the cost of the fix, and the cost is
the whole reason the hook exists.

No docs

Four files, none of them docs/ or CHANGELOG.md. The rule here is that a PR
ships its documentation. This one changes wrong results into right ones on a
feature the docs never mention — INHERITS appears nowhere under docs/, only in
CHANGELOG.md — so at minimum it needs the CHANGELOG entry, and docs/features.md
or docs/limitations.md should say what a columnar inheritance tree now does.

Same note as on #869; it is the one thing missing from both.

Reviewed as OffgridwithJD. Not approving — same account as the author.

[OffgridwithJD]

Correction to my previous comment — I tested my own claim and it failed

I wrote that a broader version of this fix, one that also skipped the children,
"would pass every one of those arms while quietly turning every inheritance query
into Seq Scan." That was a reading of your suite, not a run of it. I ran it.

Mutation on your branch: drop RELOPT_OTHER_MEMBER_REL from the reloptkind test,
so children and partitions both lose the custom scan. Your suite:

accounting: 11 passed + 1 failed + 0 unrunnable = 12
FAIL  and the partition is still a PgColumnarScan (#436): got [0] want [1]

Your suite catches it. The #436 partition arm reddens, because an inheritance
child and a partition child are the same RELOPT_OTHER_MEMBER_REL, so any natural
mistake that drops one drops the other and that arm fires. My "would pass every
arm" was wrong, and the gap I described is not a gap for any mutation a person
would plausibly write.

What survives is much weaker, and I would not block on it: no arm names an
inheritance child directly, so the property is defended incidentally by a
partition test rather than by an assertion about the thing it is protecting. Adding
the one-liner is still worth a minute, because a reader of inheritance.sh cannot
see that the children stay columnar:

check "the Append's members are still columnar scans" \
    "$(printf '%s' "$inh_plan" | grep -c 'Custom Scan (PgColumnarScan)')" "2"

But it is a readability improvement, not a hole. Treat my earlier paragraph as
withdrawn.

Everything else in that comment stands and was measured: the premise (heap 5001
against columnar 1, no Append), the fix (5001, Append present), the blast radius
(plain table and ONLY parent both keep PgColumnarScan on both arms), and the
children keeping their scan under the real fix (Append of two PgColumnarScans,
zero Seq Scans, pushdown intact). So does the missing CHANGELOG.

Reviewed as OffgridwithJD. Not approving — same account as the author.

[OffgridwithJD]

I pushed the missing CHANGELOG entry: 2d37d34, docs only

Green on all 12 checks, no CHANGELOG.md entry. One file changed, one entry under
## [Unreleased] / ### Fixed; no source, no test, no behaviour. docs_style 9/9.

Every number in the entry is one I measured on this branch: parent 1 row and child
5000, SELECT count(*) FROM parent returning 1 where 5001 is right from a plan
with no Append, and the blast radius — children keep the custom scan, so the
fixed plan is an Append of two PgColumnarScan nodes with pushdown intact rather
than a fallback to Seq Scan.

The entry deliberately records that the grouped vector path already refused this
shape and the ungrouped one did not. That asymmetry is the reason the bug survived,
and it is the thing a future reader needs when they wonder why two adjacent hooks
disagreed.

Still open from my earlier review, not addressed here: no arm pins that the
children keep the custom scan. I retracted my claim that a broader fix would pass
your whole suite — I mutated the branch and the #436 partition arm reddens, so the
suite does defend the property for partitions. It does not defend it for
inheritance children, and that is a one-line arm.

Note before this lands: #866 and #871 edit the same three hook sites
(columnar_customscan.c:2527, columnar_vector.c:959 and :1783). Whichever
merges first, the other needs a rebase.

[OffgridwithJD]

The #866 / #871 overlap, measured rather than predicted

I said in my previous comment that these two would conflict with each other. That
was a reading of the diffs, so I ran it. Trial-merging audit/inherit-custom-scan
into audit/tablesample-ignored:

CHANGELOG.md               1 conflict hunk
src/columnar_customscan.c  1 conflict hunk
src/columnar_vector.c      1 conflict hunk

Both PRs add an early return to the same guard block in
PgColumnarSetRelPathlist and to the same place in PgColumnarCreateUpperPaths,
so the second to merge needs a rebase, and the resolution is to keep BOTH returns
rather than either. Neither conflicts with main.

They are independent fixes for independent bugs — rte->tablesample and
rte->inh — so this is a merge-order question, not a design question.

[OffgridwithJD]

Rebased onto b4f0a45 at 9410242, and the #866 overlap is resolved by keeping BOTH guards

#866 merged, so this went CONFLICTING at exactly the three sites I measured earlier.
The resolution is not mechanical and I want it on the record before anyone merges:

    if (rte->tablesample != NULL)     /* #866, now in main */
        return;
    if (rte->inh)                     /* this PR */
        return;

Both hooks now carry both guards, each keeping its own comment. Neither subsumes the
other — one is about a sampled scan reaching the AM callbacks, the other about an
inheritance parent's Append — so dropping either would silently restore a bug that
has a test.

Proof that keeping both works, on the rebased tree:

test/inheritance.sh    12 passed + 0 failed = 12     (this PR's behaviour)
test/tablesample.sh     8 passed + 0 failed = 8      (#866's, now in main)
test/docs_style.sh      9 checks, exit 0

That pairing is the check: either guard alone would leave one of those suites red.

The src/test half of the patch adds exactly the same 111 lines and removes
none, verified against the pre-rebase head rather than assumed — only the diff
context moved, because main gained the tablesample guard above it.

A trap I nearly walked into, since it would have been invisible

My first rebase attempt built on a stale remote-tracking ref. git fetch origin 'refs/heads/audit/*:refs/remotes/origin/audit/*' without a leading + silently
declines to update a ref that was force-pushed, so origin/audit/* still pointed at
the pre-rebase heads from hours earlier and reported them without complaint. The
rebase would have succeeded and produced a plausible branch built on the wrong base.

It happened to be content-equivalent here, which I checked rather than assumed —
#860 and #863 patches byte-identical outside CHANGELOG.md, #871 adding the
identical 111 lines. But that was luck, not method. The fetch refspec needs the +,
and the lease values on the force-push must be read from GitHub rather than from a
local ref that may be stale.

[jdatcmd]

Approved at 9410242.

Reviewed adversarially: three independent skeptics were pointed at every finding
raised against this PR with instructions to refute it and to default to refuted
when uncertain. Nothing survived. That is a weak result on its own, so I did not
stop there.

The part I would not take on anyone's word is the resolution against merged
#866, because it keeps both the rte->tablesample and the rte->inh guard
in the same block, and a reviewer cannot tell by reading whether the second one
is load-bearing or decoration. So I removed each guard and measured, on the
composed tree (main + this PR + #868 + #874), PG 17.10:

tree	inheritance	tablesample
both guards, as merged	PASS	PASS
rte->inh neutered, all 3 sites	FAILED	PASS
rte->tablesample neutered, all 3 sites	PASS	FAILED

Each guard reddens one suite and only that suite. The off-diagonal passes are
the half that makes this evidence rather than noise: they show the mutation is
specific, not a build that breaks everything. Both mutations were asserted to
have applied before building — 3 markers present, 0 original guards remaining,
in each tree — so neither column can be a mutation that silently did not land.

Composition, not the branch. Two individually green branches are not proof
that they compose; this repo has been bitten by that. So the number that matters
is a full PG 17.10 matrix on the merged tree, against a matrix on main
b4f0a456 run in the same isolated prefix:

	verdicts	PASS	SKIP	FAIL
main b4f0a45	239	234	5	0
main + #871 + #868 + #874	241	236	5	0

Set difference composed − baseline is exactly estimate_deleted=PASS and
inheritance=PASS, the two suites those PRs add. Baseline − composed is
empty: nothing that passed on main stopped passing. inputs == sum(buckets)
on both sides (234+5=239, 236+5=241), counted from the run output rather than
retyped.

Both runs used a private prefix (/usr/local/pg17_m902*) so no concurrent
make install could overwrite the .so under either arm.

Nits, none blocking, not conditions of this approval: they are on the record in
the review pass and none of them touch behaviour.