Repository navigation
Let ORCA plan in-place UPDATE of AO tables; fix AO row scan expanding unread columns - #2094
Conversation
dd81d08 to
afbf741
Compare
|
Please add initial authors of the patches and the projects where these commits came from in commit messages. |
|
I've noticed some UPDATEs now dispatch to all segments and use two-phase commit instead of single-segment execution. Could you benchmark this regression, especially on larger clusters? |
initial authors is me
I have't test this. But I just enable update in orca, and you can use PG optimizer if you like. It's just optional. |
Sorry. I couldn't find the initial commit and when you linked to the commit of another author in the PR I thought it was another author. As for test, I have noticed that the query plan in one of the tests became worse and because of this suggested to run benchmark. |
Any UPDATE on an AO, AOCS or PAX table that does not modify a
distribution key column made GPORCA give up and silently fall back to
the Postgres planner:
INFO: GPORCA failed to produce a plan, falling back to Postgres-based
planner
DETAIL: DXL-to-PlStmt Translation: Attribute number 7 not found in
project list
With optimizer_trace_fallback off, which is the default, none of this
reaches the client; only EXPLAIN's "Optimizer:" line gives it away.
Since PG14 a non-split UPDATE builds the new tuple from the plan slot
plus the old tuple, and ExecModifyTable fetches that old tuple by tid.
Append-optimized tables cannot do that lookup -- it would mean scanning
the block directory and the visibility map per tuple, and the block
directory is not guaranteed to exist -- so appendonly_tuple_fetch_row_
version() errors out with "feature not supported on appendoptimized
relations".
The planner works around this by adding a wholerow junk column for such
tables (see the GPDB_14_MERGE_FIXME in add_row_identity_columns), which
makes oldtuple non-NULL and skips the fetch. ORCA has no such column,
so CTranslatorDXLToPlStmt::TranslateDXLDml() forced a split update
instead. But CExpressionPreprocessor::ConvertSplitUpdateToInPlaceUpdate()
had already downgraded the plan to an in-place update -- it only looks at
whether a distribution or partition key column is modified, never at the
access method -- so the DMLAction column the forced split needs was not
in the plan, and the translation failed.
The old tuple is only needed for the columns the subplan does not
assign. ORCA's non-split UPDATE assigns every live column, so nothing
reads it. Record that in ri_projectNewNeedsOldTuple while building the
update projection and skip the fetch when it is false, which lets an
append-optimized table run the plan, and drop the forced split.
This is what the FIXME asks for: the update plan now has the same shape
whether it comes from the planner or from ORCA, and it matches what the
storage engine already does, since appendonly_tuple_update() is itself a
delete followed by an append. Iceberg needs no special case either --
its UPDATE writes the position delete and the new row in one
ExecForeignUpdate call.
The skip is limited to append-optimized relations for now. Letting heap
take it as well would remove the wholerow junk column the planner ships
for those tables, but needs the trigger, EPQ and cross-partition update
paths audited first.
Forcing the split the other way round was tried first and rejected: it
made nine queries that update the same row twice in one statement fail
with "multiple updates to a row by the same query is not allowed", which
only a split update raises.
The answer files change where a recorded fallback disappears, where an
UPDATE plan gains its ORCA shape, and in ao_locks and
uao/parallel_update_{row,column}, which go back to locking all segments
because ORCA does not do direct dispatch for these updates.
See: Issue#603
(cherry picked from commit befb03fea31fd7d4f75a0ab4cdd624f83705777a)
…gineering/dev_collaboration#965
## Summary
Fixes field-engineering/dev_collaboration#965: a `count(1)` over a 281 column
append-only **row** table, qualified on three date columns, was 6x slower than the same
data in a heap table and 14x slower than AOCS.
### Root cause
`AppendOnlyExecutorReadBlock_ProcessTuple()` called `memtuple_deform()` and then
`ExecStoreVirtualTuple()`, and `appendonly_slot_callbacks()` returned `TTSOpsVirtual`.
A virtual slot promises its values are already in `tts_values[]`, so
`ExecStoreVirtualTuple()` sets `tts_nvalid = natts` and `slot_getattr()` never reaches
`getsomeattrs` — the scan has no choice but to expand **every** column of **every**
tuple. Heap tables are unaffected because `TTSOpsBufferHeapTuple` deforms incrementally
up to the highest attribute number the query references.
Scan cost therefore grew with the **width of the table** instead of the columns actually
read. Measured on a release build (`-O3`, no cassert), 500k rows, query touching only
attno 2:
| table columns | AO row before | heap |
|---|---|---|
| 20 | 46.9 ms | 35.7 ms |
| 100 | 114.2 ms | 36.8 ms |
| 281 | **236.0 ms** | 39.1 ms |
This is not how the format is meant to be read. A MemTuple keeps precomputed attribute
offsets in its binding, so any attribute is reachable in constant time, and before the
PostgreSQL 12 merge (`19cd1cf4b6`) the slot carried that binding and fetched attributes
on demand. That merge dropped the MemTuple slot shape — which at the time could only
exist as extra fields in the shared `TupleTableSlot` struct — and converted the tuple
eagerly instead. PostgreSQL 12 also introduced `TupleTableSlotOps` precisely so an access
method can keep tuples in its native format, which is what this MR finally uses.
**The same code is present in GPDB 7 and Apache Cloudberry main**, both verified to show
the identical regression (GPDB 7 release: 193.8 ms vs heap 28.9 ms at 281 columns). This
is an inherited PG12-merge artifact, not something introduced by Lightning.
### Change
Add `TTSOpsMemTuple`, a slot holding the MemTuple together with the binding needed to
interpret it, expanding attributes in `getsomeattrs`. Because offsets are precomputed,
`tts_nvalid` alone is the resume point — no separate deforming cursor (the `off` field
heap and minimal tuple slots carry) is needed. `memtuple_deform_range()` reuses the
existing inlinable internal helper so full-column access keeps the old bulk-deform speed.
Two details are easy to miss when adding a slot type here, both found by running the full
regression suite:
- `gp_segment_id` and `gp_foreign_server` are answered by the slot even though no tuple
format carries them, so `getsysattr` must keep the special cases that
`tts_virtual_getsysattr` documents. Omitting them broke ~59 tests.
- Not every slot reaches the scan through `table_slot_create()`.
`AppendOnlySegmentFileFullCompaction()` built a virtual slot directly and passed it to
`appendonly_getnextslot()`; in a release build the assertion is compiled out and the
larger slot struct would be written past the end of the smaller one. It now asks the
access method for the slot type, and `ProcessTuple` additionally falls back to expanding
the tuple up front for any non-MemTuple slot, so a caller bringing its own slot cannot
be handed one it cannot read.
## Test plan
Release build (`-O3`, `debug_assertions=off`), demo cluster, 500k rows.
The two tables below pin one variable each. Query is
`SELECT count(1) FROM t WHERE c2 > 0`, i.e. it references attno 2 only, unless stated.
**1. Cost no longer depends on how wide the table is.** Query fixed on attno 2, table width varied:
| table columns | before | after | heap |
|---|---|---|---|
| 20 | 46.9 ms | 35.9 ms | 35.7 ms |
| 50 | 67.3 ms | 36.9 ms | 38.4 ms |
| 100 | 114.2 ms | 36.0 ms | 36.8 ms |
| 200 | 171.1 ms | 37.6 ms | 39.0 ms |
| **281** | **236.0 ms** | **41.4 ms** | 39.1 ms |
| 281, all columns populated | 415.0 ms | 112.7 ms | 103.6 ms |
**2. Cost now depends on the highest attribute number referenced, exactly as heap does.**
Table fixed at 281 columns, referenced column varied (`count(cN)`):
| referenced up to | after | heap |
|---|---|---|
| c2 | 37.3 ms | 36.1 ms |
| c50 | 68.2 ms | 56.4 ms |
| c140 | 121.5 ms | 95.4 ms |
| c281 | 212.4 ms | 156.2 ms |
Slopes: AO 0.63 ms per column per 500k rows, heap 0.43 ms. So the win is not that MemTuple
deforms faster than heap — it does not — it is that unread columns are no longer deformed at
all. A query reaching the last column costs what it always cost; the flat line in table 1 is
a consequence of referencing attno 2, not an unconditional property.
The reported query references attno 9, 12 and 13, so it now expands 13 columns instead of
281. `getsomeattrs` fills a prefix, so the gap columns (1-8, 10, 11) are still expanded;
skipping those would need per-column projection, which `tts_nvalid` cannot express (it is a
single watermark, so validity is necessarily a prefix) and which would mean claiming
unfilled columns are valid. Not attempted here.
Full-column access is not penalised: `count(c281)` 217.7 ms (before the change a bare scan
already cost 236.0 ms because it deformed everything), `sum(c281)` on fully populated data
415.9 ms vs 415.0 ms.
**Correctness**
- [x] `SELECT * FROM ao EXCEPT SELECT * FROM heap` and the reverse both return 0 rows, over
all 281 columns, for sparse and fully populated data
- [x] System columns unchanged: `ctid` / `tableoid` / `gp_segment_id` work, the four MVCC
columns still error (a MemTuple carries no MVCC fields; visibility lives in the visimap)
- [x] Materialize-dependent paths: CTE MATERIALIZED, ORDER BY, CTAS round trip, cursor
across statements
- [x] `make installcheck` A/B on the same cluster: **baseline 36/723 failed, with this
change 36/723 failed, failure sets identical**. The 36 are environmental in this
build (no pax_storage / iceberg AM, ORCA disabled).
- [x] Builds clean under `-O3 -Werror`
**Not done**
- [ ] No regression test added for the wide-table-narrow-projection behaviour. Both bugs
above surfaced only from the full suite, so this path is under-covered; a test
exercising a wide table plus the VACUUM / CLUSTER / CREATE INDEX paths would be
worth adding. Happy to add it to this MR if preferred.
- [ ] Not yet verified on GPDB 7 / upstream Cloudberry, though the patch applies to the
identical code there.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
(cherry picked from commit 47a358712540acfa3ecd7ee1cb778ed01d5298f4)
afbf741 to
db834d4
Compare
Cherry-picked two commits from the hashdata-lightning repo.
Commits
befb03fea31)47a358712540)Conflict resolution notes
src/test/regress/greenplum_schedule: added onlyorca_split_updateto the ORCA group. Droppedorca_material_rescanfrom the lightning hunk because its test files do not exist in this repo.contrib/pax_storage/src/test/vec/expected/partition_prune_optimizer.outandcontrib/vectorization/src/test/regress/expected/partition_prune_aocs_optimizer.outdeleted (modify/delete conflicts — these files do not exist in cloudberry).🤖 Generated with Claude Code