Parquet: Hive-style partition columns and whole-file pruning

[jdatcmd]

Third and last of the Parquet follow-ons planned in #119. A layout like events/dt=2026-01-02/region=eu/part.parquet carries column values in its directory names; a foreign table can now declare those columns and have whole files dropped before they are opened.

Declared, not inferred

Partition columns come from a partition_columns table option. Inference would mean guessing which path components are partitions, and a wrong guess silently changes which rows a query returns — the same reasoning that makes read_parquet require a column definition list rather than inferring one. An unknown column name is an error, so a typo cannot degrade into "no partitioning". Scope is the FDW: read_parquet has nowhere to declare this, and overloading its column definition list would be guessing again.

Three consequences worth reviewing

• A declared partition column consumes no Parquet leaf, so build_imp_targets skips it. That is what keeps the "every leaf must be declared" identity intact, and it means declaring a column the file actually carries fails. The suite pins that.
• Pruning evaluates the clause rather than reasoning about it. A qual that reads only partition columns is decided completely by the path values, so it runs through ExecQual against a slot holding them. That covers every operator, IN lists, and expressions for free, and it cannot disagree with what the executor would decide about the same rows. Deliberately unlike row-group skipping, which must reason from statistics and is where the NaN and inverted-interval edge cases live.
• A pruned file is never opened, so it costs no I/O and its row groups never reach the Row Groups counter. Asserted (16 groups unfiltered, 4 after pruning to one file) rather than assumed.

Evidence

New suite native_parquet_partition.sh, 20 checks, added to the matrix list in the same commit.

The guard that matters is that a file missing a declared component raises rather than yielding nulls, and it is proven by removal: test/mutate_guard.py partmiss deletes exactly that ereport, and then the malformed tree reads silently and only that check fails.

Being precise about what the rest proves: pre-change the whole suite exits 1 at CREATE FOREIGN TABLE, because the validator rejects the unknown option. That demonstrates the feature is new, not that each check is meaningful, and I am not counting it as more than that.

Deliberately not done

Percent-decoding of partition values. Hive percent-encodes characters that cannot appear in a path component; values are taken literally here, and that is documented in limitations.md alongside partition inference as the remaining Parquet items.

Gate

Full 15 through 19 matrix, since this completes the three follow-ons: PostgreSQL 15.18, 16.14, 17.10, 18.4, 19beta2.

🤖 Generated with Claude Code

[ChronicallyJD]

Reviewed at 4a0b689. Declaring the columns rather than inferring them is the
right call and the reasoning for it is the same one that makes this reader
trustworthy elsewhere. Evaluating the clause instead of reasoning about it is
also right, and it is genuinely stronger than the statistics path for everything
it covers.

But "it cannot disagree with what the executor would decide about the same rows"
is the claim the design rests on, and there is one class of clause where it does
disagree. That plus a lifetime problem in the same function are why I am
requesting changes; both fixes are small and land in the same place.

Finding 1 (blocking): a volatile function in a partition-only clause prunes rows that should have survived

pqfdw_partition_excludes_file decides partitionOnly from pull_varattnos,
which reports Vars and says nothing about volatility. So a clause whose only
column reference is a partition column qualifies even when the rest of it is not
a function of the partition values:

SELECT count(*) FROM ev WHERE dt = DATE '2026-01-02' OR random() < 0.5;

pull_varattnos returns {dt}, partitionOnly stays true, and ExecQual runs
once for the file. For a file whose dt does not match, that single draw
decides the fate of every row in it: below 0.5 the file is kept and the executor
re-filters per row as normal, at or above 0.5 the whole file is pruned. Per row,
roughly half those rows would have passed. The query returns fewer rows than it
should, with no error and nothing in EXPLAIN to suggest it.

This is the difference between evaluating once per file and evaluating per row,
and it is exactly the property the PR body claims. Row-group skipping never had
this exposure because it only ever handled Var op Const with a btree operator,
which cannot contain a volatile call. The flexibility that makes this approach
better for IN lists and arbitrary operators is the same flexibility that lets a
volatile expression in.

The guard is one line, next to the partitionOnly test:

if (contain_volatile_functions(clause))
    continue;               /* not decidable once per file */

(optimizer/optimizer.h.) Stable and immutable functions are fine and should
stay eligible: stable is constant within a statement, which is exactly the
guarantee this needs. Only volatile has to be excluded.

Worth a check in the suite too. A deterministic one is easy: make the volatile
half of the clause always true and assert nothing is pruned, e.g. a clause of the
form dt = <no file matches> OR random() >= 0 should prune 0 files and return
every row. With the guard removed, random() >= 0 is still always true, so that
particular test would not fail. random() < 2 has the same problem. What
separates them is a volatile function whose value the planner cannot fold and
whose truth flips per row -- pruning on dt = X OR random() < 0.5 over enough
files and asserting the row count is exact would do it, at the cost of being
probabilistic. A cleaner option is a volatile SQL function with a counter (say
nextval on a sequence, or a function returning currval-driven truth) so the
per-file versus per-row difference is deterministic rather than statistical.

Finding 2 (blocking): ExecInitQual runs inside fileCtx, which is reset every file

pqfdw_partition_excludes_file is called per file with CurrentMemoryContext
set to fileCtx, and it calls ExecInitQual(list_make1(clause), (PlanState *) node)
per clause. Two problems, one cosmetic and one not:

• The same clause list is recompiled for every file. On a tree with a few hundred
  partitions that is a few hundred expression compilations of an identical
  expression, all discarded.

• ExecInitQual is passed the ForeignScanState as parent, and for a clause
  containing a SubPlan or AlternativeSubPlan, ExecInitExprRec appends the
  new SubPlanState to parent->subPlan. That list cell and the state hang off
  node->ss.ps, which lives for the whole scan, but they are allocated in
  fileCtx, which MemoryContextReset clears at the bottom of the same
  iteration. ExecEndNode walks node->subPlan later.

  A partition-only clause can contain a SubPlan: WHERE dt IN (SELECT d FROM dates) leaves dt = ANY (SubPlan 1) in the scan's qual list, and
  pull_varattnos over scanrelid reports only dt, so it passes the
  partitionOnly test and gets initialised.

Both go away by compiling once before the file loop, in the query context, and
evaluating per file:

/* before the loop, in the caller's context */
foreach(lc, fs->scan.plan.qual)
    if (partition_only(clause) && !contain_volatile_functions(clause))
        partQuals = lappend(partQuals, ExecInitQual(list_make1(clause), node));

Then the per-file work is ExecStoreVirtualTuple plus ExecQual over a
prebuilt list, which is also where finding 1's guard naturally goes. The slot can
be built once and reused the same way, instead of MakeSingleTupleTableSlot and
ExecDropSingleTupleTableSlot per file.

Verified

• The leaf-binding identity survives. build_imp_targets skipping masked
  attributes is what keeps "every leaf must be declared" true, and the
  consequence -- declaring a partition column the file actually carries now fails
  the leaf count -- is the right trade and is pinned by the suite.
• Partition value lifetime is correct, which was the first thing I checked
  given fileCtx. partVals and the Datums from OidInputFunctionCall live in
  fileCtx, the slot points at them for every row of that file, and
  pq_tuplestore_sink materialises each row into the tuplestore before the reset
  at the bottom of the iteration. Nothing survives the reset that is read after
  it.
• Missing component is an error, not a null, and it is the one guard proven
  by removal. That is the right thing to have proven: nulls there would be the
  silent-wrong-answer shape.
• Pruning genuinely precedes the open -- the continue is before
  pq_source_open -- so a pruned file costs no I/O and contributes no row groups.
  Asserting 16 versus 4 on the row-group counter is a better check than
  timing.
• SplitIdentifierString gives correct identifier semantics, so unquoted
  names downcase and "DT" works if quoted in the option, matching what a user
  expects from every other identifier list in PostgreSQL. Duplicate and unknown
  names are both errors.
• ps_ExprContext is available at BeginForeignScan time (ExecInitForeignScan
  calls ExecAssignExprContext before the callback), so the econtext use is
  valid.
• Clauses with no Vars of this rel, whole-row references, and system columns all
  fall through to "not ours to decide" rather than being decided wrongly.

Minor

• The whole absolute path is scanned for name=value, including the file's own
  basename and any component above the declared root. A file literally named
  dt=2026-01-02.parquet would set dt from its basename, and a root path that
  happens to sit under something like /exports/region=eu/ would pick that up
  for a column named region. Restricting the walk to the components between the
  resolved root and the file removes both.
• __HIVE_DEFAULT_PARTITION__ is taken literally. That is the marker Hive and
  Spark write when a partition value is null, so a dt=__HIVE_DEFAULT_PARTITION__
  directory fails the date input function with a parse error. Worth a sentence
  in limitations.md beside the percent-decoding note, since the two come from
  the same writers.
• Params are safe only by accident, and worth a comment saying so. ExecQual
  will happily read $1, and that is correct today only because
  pqfdwGetForeignPaths advertises no parameterized path, so a rescan cannot
  arrive with different values while ReScanForeignScan merely rewinds the
  tuplestore. The comment at the head of pqfdw_compute_skip already records
  this reasoning for row-group skipping; pruning now depends on it too, and the
  dependency is invisible.

Verdict

Requesting changes on findings 1 and 2. Both are small, both live in
pqfdw_partition_excludes_file, and hoisting the compile out of the loop is the
same edit that gives the volatility guard somewhere natural to sit. Everything
else here is solid: the declared-not-inferred decision, the error-not-null rule
with its mutant, and asserting pruning through the row-group counter rather than
through timing.

[jdatcmd]

Both findings taken in 4783cab.

Finding 1. You are right, and it breaks the exact property the PR body claims. pull_varattnos reports Vars and says nothing about volatility, so dt = X OR random() < 0.5 counted as partition-only and one draw decided a whole file. A clause containing a volatile function is now left to the executor; stable and immutable stay eligible, for the reason you gave.

Your point about the test being hard to make deterministic was the useful part. random() >= 0 and random() < 2 both survive the guard's removal, as you said. What works is your sequence suggestion: vol_odd() advances volseq, files are read in sorted path order, and OR short-circuits, so without the guard the first dt=2026-01-01 file calls it and is kept, the second calls it and is pruned, and the two dt=2026-01-02 files never call it because dt already matched. Exactly one file is pruned without the guard, none with it. Verified both ways: with the guard removed, only that check fails, got [1] want [0].

Finding 2. Also right, and reachable exactly as you describe. The quals are compiled once before the loop in the scan's own context, and the slot is built once as well. That removed the per-file compile and the dangling SubPlanState, and it gave finding 1's guard the natural place to sit, as you predicted.

Minors, all taken:

• Only the components between the declared path and the file are scanned now, so neither a component above the path nor a file named dt=2026-01-02.parquet sets a column. The basename case is in the suite.
• __HIVE_DEFAULT_PARTITION__ is documented beside the percent-decoding note, along with the volatile rule, since a user whose scan stops pruning deserves to find out why.
• The Params reasoning is now written where pruning depends on it, pointing at pqfdwGetForeignPaths, rather than being invisible.

One thing your review caught that I want to flag as a pattern: both findings came from the same root, which is that evaluating a clause once per file is only equivalent to per-row evaluation under conditions the Var analysis cannot see. That is worth remembering the next time this looks like an easy win.

Gate: full 15 through 19 matrix on the reviewed code, ALL VERSIONS PASSED, partition suite green on all five majors.

[jdatcmd]

Merging on the strength of the fixes and the gate, without waiting further for a re-review, since the owner asked for this to keep moving overnight. Recording that plainly rather than implying an approval that is not there:

• Finding 1 fixed and proven by removal: with the volatility guard deleted, only a volatile clause prunes nothing fails, got [1] want [0], which is the single-file pruning your analysis predicted.
• Finding 2 fixed: quals compiled once before the loop in the scan's context, so the per-file compile and the dangling SubPlanState are both gone.
• All three minors taken.
• Full 15 through 19 matrix on the reviewed code: ALL VERSIONS PASSED, partition suite green on all five majors.

If anything in the re-review lands after this, I will take it in a follow-up PR rather than leaving it on a merged branch.