Compare zone min/max directly for integer types, and what the write path actually costs (#155) - #160
Conversation
Tracking a chunk's min and max costs two comparisons per value per column, and
routing both through fmgr is most of what they cost: removing min/max tracking
altogether saved 15% of a five-int-column load, where the comparison itself is a
subtraction.
Integer-family columns now compare directly, using the same list of types the
vectorized filter's fast path already uses. The maximum is tested first and the
minimum only when the maximum does not settle it, since a value above the
running maximum cannot also be below the running minimum -- so an ascending load,
which is what a bulk load of a serial or timestamp column produces, costs one
comparison per value rather than two.
float4 and float8 are deliberately excluded even though the filter fast path
takes them. btree float ordering puts NaN above every other value, and a C
comparison gets that wrong: every comparison against NaN is false, so NaN reads
as equal and never becomes a chunk's maximum. The zone map would then rule out a
group that does hold matching rows. Demonstrated rather than assumed -- with
floats added to the fast list, a row holding NaN stops being findable at all:
the equality predicate returns 0 rows where heap returns 1.
Measured on 18.4, 3,000,000 rows, median of three, same machine and session:
1be027b this branch
5 int columns 3587.3 ms 3395.8 ms 5.3%
1 text column 4496.7 ms 4183.0 ms 7.0%
1 timestamptz column 968.2 ms 924.8 ms 4.5%
The text column gains without being on the fast list, because it takes the
short-circuit: one collation-aware comparison per value instead of two.
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
The measurement is worth more than the patch, as you say, and I am acting on it:
design/IMPORT_THROUGHPUT_PLAN.md is mine and its model does not survive what
you measured. Merging with the suite registered.
The finding
At one integer column the columnar write path is faster than heap (0.71x).
That single number kills step 1 of my plan. I reasoned that both readers decode a
columnar file into per-row Datums and the writer copies them back, so the per-row
interface must be the waste and a batching entry point was "worth doing first
even without the column-wise work". If per-call overhead were the cost, a
one-column load would show it as clearly as a five-column one. It does not, so
there is no per-call overhead to amortise, and someone would have spent the first
of two-to-four dev-months finding that out.
That the cost is per value and roughly additive per column, and that one text
column costs more than five integer columns, relocates the whole thing: the 4.9x
in #155 is "five columns, one of them text", and the varlena write path is where
it lives. I am rewriting the plan around that rather than leaving a document that
contradicts your measurements.
The compression result is worth having recorded too. none against zstd 3 at
2% says the codec is not worth touching, which is the kind of negative result
that saves an afternoon.
The patch
Correct, and the part I checked hardest is the part you got right without being
asked: float4 and float8 are excluded from the direct comparison, with NaN
ordering as the reason. That is the trap in this change. A C comparison against
NaN is false in both directions, so NaN would never become a chunk's maximum, and
a zone map whose maximum is too low makes the reader skip a row group that holds
matching rows. Silent missing rows, found by nothing in the suite. Keeping the
fast path to int2/int4/int8/date/timestamp/timestamptz, where C ordering is btree
ordering, is exactly the right line, and the comment explains why the filter path
can take a shortcut this one cannot.
The max-first reordering is sound: a value above the running maximum cannot also
be below the running minimum, so the second comparison is only needed when the
first does not settle it, and the old code could never update both from one value
anyway. Behaviour is unchanged; only the number of comparisons moves.
Presenting the individual runs rather than only the medians is the right way to
show a 5% effect. Non-overlapping, so it is a result rather than noise.
Gate
PG18 and PG19, full suite, ALL VERSIONS PASSED, with write_minmax_fastpath,
native_zonemap, native_skip and native_bloom green, which is the set that
would catch a wrong bound.
An earlier run of this gate reported native_agg_deletes=FAIL on PG18. That was
mine, not yours: I had a crash reproduction running against the same box, and the
suite log shows the harness's cluster-ownership retry loop rather than a failing
check. It passes on an idle box.
Registration
test/write_minmax_fastpath.sh was not in test/run_all_versions.sh. Registered
here. That is four PRs in a row now, so rather than keep noting it I am adding a
harness check that fails when a suite exists and no gate runs it.
The document names the commit it was measured at, which is the right model for a benchmark record, but a reader should not have to diff the log to learn that the ingestion rows are already a floor. #160 improved the write path 5 to 7% after this run; query latency and storage are untouched by it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb
Neither plan survived contact with an implementation, and leaving them as written would be the thing #165 just prohibited: a document that reads as guidance while being known-wrong. Statistics. The cluster-sampling trap this plan leads with does not produce the symptom it predicts. #159 implemented the whole-group mapping to check, and n_distinct came out fine (1000 against a true 1001) because offering every row of a group per block hands core far more rows than it asked for and the reservoir samples most of the table. What breaks is reltuples, by a factor of twenty, since those rows are counted against the fraction of blocks visited. The slice mapping is still right and the reason is now the measured one. A test written to watch n_distinct on a clustered table would have passed against the wrong implementation, which is the general lesson worth keeping. The claim that correlation "falls out of the design rather than needing special handling" was simply false. acquire_sample_rows sorts by item pointer and the sample arrives through ExecCopySlotHeapTuple, whose virtual-slot implementation re-forms the tuple and drops tts_tid, so every sampled row carries an invalid pointer: noise on a non-assert build, an aborted backend on an assert one. The access method has to supply its own copy_heap_tuple. That claim cost a revision to discover and now carries what was found instead. Throughput. Step 1 was a batching entry point, on the reasoning that the per-row round trip is the waste. #160 measured a one-column load: at one integer column the columnar write path is faster than heap, 908 ms against 1272. There is no per-call overhead to amortise, the cost is per value and additive per column, and one text column costs more than five integer ones. The order now starts at the varlena path, where the 4.9x actually lives, and batching drops to last as something that may not be needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb
2 participants
Towards #155. The measurement below matters more than the patch, because it contradicts the model
design/IMPORT_THROUGHPUT_PLAN.mdis built on. The patch is a real but modest 5–7%.The plan's model does not survive measurement
The plan says the waste is the per-row round trip: both readers decode a columnar file into per-row
Datumarrays, hand each row totable_tuple_insert, and the write path copies each value back into per-column buffers. Step 1 is therefore a batching entry point, "worth doing first even without the column-wise work: it amortises the per-call overhead".If per-call overhead were the problem, a one-column load would show it as clearly as a five-column one — same rows, same number of
table_tuple_insertcalls. It does not:18.4, 3,000,000 rows,
INSERT INTO ... SELECT, no file involved, same machine and session:At one integer column the columnar write path is faster than heap. There is no per-row overhead to amortise; the row-at-a-time interface is not what costs. The cost is per value and roughly additive per column — about 900 ms per numeric column and about 4,600 ms for a text column, which is why the 4.9x in the issue is really "five columns, one of them text".
Two more things the plan does not mention:
compression = noneis 2% faster thanzstd 3(8846 ms against 9022), andlz4is indistinguishable. Not worth touching.My reading is that step 1 as written would deliver close to nothing, and step 2 should be judged per column type rather than per nullability/width combination. I would rather say that before someone spends the first of the two-to-four dev-months than after.
Where the remaining per-value cost sits
Ablated by building with each piece disabled, 3,000,000 rows:
No single dominant term; min/max is the largest identified piece at 15–17%, and the rest is spread across encoding, the value stream and the flush. That 15% on five integer columns, where the comparison itself is a subtraction, says the cost is the fmgr indirection rather than the comparing.
What this patch does
Two changes to the min/max tracking, which is the one piece with a clean fix:
COLUMNAR_VECFAST_*).Median of three, same machine and session:
1be027bIndividual runs, so the separation can be judged rather than taken on trust: 5 int columns
3635.7 / 3532.2 / 3587.3against3395.8 / 3395.2 / 3396.9; text4600.9 / 4496.7 / 4399.6against4307.5 / 4135.2 / 4183.0. Non-overlapping.floats are deliberately excluded, and I can show why
btree float ordering puts NaN above every other value. A C comparison gets this wrong — every comparison against NaN is false, so NaN reads as equal and never becomes a chunk's maximum, and the zone map then rules out a group that does hold matching rows.
I built that mistake on purpose to see what it costs. With float4/float8 added to the fast list, a row holding NaN stops being findable:
WHERE f8 = 'NaN'returns 0 rows where heap returns 1. Silent row loss, which is the failure mode this project's bar is explicitly about.The filter fast path can take floats safely because it compares to answer one predicate; this path compares to build stored bounds a later scan trusts. Different obligations, same-looking code.
Tests
New suite
test/write_minmax_fastpath.sh, 9 checks: differential against a heap mirror across twelve columns under predicates placed on chunk-group boundaries, the stored bounds themselves, a check that skipping still happens (a fast path that quietly widened every bound would pass the first two), and the float/NaN/infinity cases.These are correctness guards, so they pass on
maintoo — that is the point of them. I proved they discriminate by mutation rather than assuming it, and the first attempt did not:NaNas the first row of its chunk, so it became both bounds before any comparison ran. Every behavioural check passed against the deliberately broken build. Only the source-grep check failed, which would have been a false sense of cover.Gate
Full suite on 18.4: 78 pass, 0 fail. Non-passing entries are
devloop, which needsPGC_SRCand fails the same way onmain, andanalyze_stats, which belongs to #159 and is not part of this branch.Not run on 17. The two-major gate is yours.
Scope, honestly
This is not #155. It is 5–7% of a 4.9x gap, plus the measurement that says where the rest is and that step 1 of the plan is aimed at something that is not there. I did not build the batching entry point, because I do not think it would pay and I would rather show you the numbers than write it and find out.
Happy to take the varlena write path next if you agree that is where the 4.9x actually lives.