diff --git a/CHANGELOG.md b/CHANGELOG.md index 94c596e7..a563727a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,39 @@ true until the next version shipped. No bash suite changes, so no ledger row moves and the census does not. `cluster_tests` 418 -> 421, re-derived by collection. +- An encoding was chosen on pre-codec bytes but the chunk is stored post-codec, + so an encoding that shrank the bytes could enlarge the stored chunk (#1132). + + `PgColumnarEncodeChunk` picks the smallest candidate against `bestLen`, which + starts at `rawLen`, and every comparison is on UNCOMPRESSED bytes. The block + codec runs afterwards, once, over the whole encoded region, defaulting to zstd + level 3. Bit-packing whitens a stream the codec was exploiting, so the two + disagree -- and only the codec's answer is what gets written. + + FSST already decided this way, through `PgColumnarFsstHelpsCompressed`. The + writer now asks the same question for the rest: it compresses the encoded + region and the raw one and keeps whichever is smaller, per column chunk, which + is the granularity the codec actually runs at. + + Measured on ClickBench `hits_0.parquet`, 1,000,000 rows and 105 columns, + imported with `pgcolumnar.import_parquet` on PG17: + + stored total 81,869,112 -> 78,109,810 -4.59% + NONE vectors 101 -> 1,621 + RLE / DICT / FOR 5372 / 3782 / 934 -> 4558 / 3390 / 621 + FSST 310 -> 310 unchanged + + FSST is unchanged because it already had this gate; the encoders that did not + are exactly the ones that moved. The worst single column, `ClientEventTime`, + was stored 49.7% smaller. Its shape is why: rare outliers stretch the range + frame-of-reference must size every value for, while the typical value's high + bytes stay constant for the codec to compress. Row counts, two column sums and + an md5 over `URL` are identical across the two loads. + + `pgcolumnar.enable_post_codec_encoding_choice` (default `on`) restores the old + behaviour. It exists because the suites that test the ENCODERS need them to + actually run: a fixture chosen to exercise frame-of-reference packing is not + necessarily one where packing beats the codec. - A covering projection was priced by clauses that merely mention its sort key, rather than by clauses it can prune on (#1126, the remainder of #1107). diff --git a/docs/configuration.md b/docs/configuration.md index 664918e7..8a020ee0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -30,6 +30,7 @@ pgColumnar has two kinds of settings: | `pgcolumnar.compression` | enum | `zstd` | Default block codec for new chunks. One of `none`, `pglz`, `lz4`, `zstd`. `lz4` and `zstd` are available only when the extension was built with those libraries. **This setting also participates in the lightweight encoding decisions that run before the codec**, so `none` is not the same cascade with compression removed; see [Compression and the encoding cascade](administration.md#compression-and-the-encoding-cascade). | | `pgcolumnar.compression_level` | integer | `3` | Level for the `zstd` codec. Range 1 to 22. Higher levels compress more and write more slowly. | | `pgcolumnar.fsst_min_gain_percent` | integer | `5` | Minimum size reduction, in percent, for FSST string encoding to be kept for a column chunk. Range 0 to 99. See below. | +| `pgcolumnar.enable_post_codec_encoding_choice` | boolean | `on` | Keep a lightweight encoding only when the encoded column chunk is smaller than the raw one AFTER the block codec has run. The encoders choose on uncompressed bytes, but what is stored is compressed, and bit-packing can whiten a stream the codec was exploiting; measured on ClickBench `hits`, 16 of 77 fixed-width columns were stored larger encoded than raw. Turning this off restores the pre-1132 behaviour of trusting the pre-codec choice. See [Compression and the encoding cascade](administration.md#compression-and-the-encoding-cascade). | | `pgcolumnar.fsst_verdict_reuse` | integer | `16` | How many later row groups may reuse a column's FSST keep-or-drop verdict before it is decided again. Range 0 to INT_MAX. | | `pgcolumnar.parallel_flush` | boolean | `off` | Opt-in. When on, a stripe flush of two or more columns fans the per-column encode and compress work out to background workers. The stored bytes match the serial path. It helps one large flush of many numeric columns by up to 14 percent. A wide text-heavy flush regresses, because it copies the buffered bytes through shared memory. Frequent small flushes regress too, so it is off by default. Enable it for a wide numeric bulk load in the session that runs it. | diff --git a/src/columnar.h b/src/columnar.h index 1b0b1fbd..68dccafd 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -185,6 +185,7 @@ extern int pgcolumnar_fsst_verdict_reuse; #define COLUMNAR_FSST_HELPS 1 #define COLUMNAR_FSST_HURTS 2 extern bool pgcolumnar_enable_qual_pushdown; +extern bool pgcolumnar_enable_post_codec_encoding_choice; extern int pgcolumnar_qual_skipvec_min_payload_cols; /* #595 width gate */ extern bool pgcolumnar_enable_late_materialization; extern bool pgcolumnar_enable_column_projection; diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 592c8924..698dee57 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -77,6 +77,7 @@ int pgcolumnar_compression_level = 3; int pgcolumnar_fsst_min_gain_percent = 5; int pgcolumnar_qual_skipvec_min_payload_cols = 20; /* #595 width gate */ bool pgcolumnar_enable_qual_pushdown = true; +bool pgcolumnar_enable_post_codec_encoding_choice = true; bool pgcolumnar_enable_late_materialization = true; bool pgcolumnar_enable_column_projection = true; bool pgcolumnar_enable_bloom_filter = true; @@ -3301,6 +3302,28 @@ _PG_init(void) 0, NULL, NULL, NULL); + /* + * #1132. An encoding is chosen on pre-codec bytes but the chunk is stored + * post-codec, so the writer compares the encoded region against the raw one, + * both compressed, and keeps the smaller. Off restores the pre-#1132 + * behaviour of trusting the pre-codec choice. + * + * It exists because the suites that test the ENCODERS need them to actually + * run: a fixture chosen to exercise frame-of-reference packing is not + * necessarily one where packing beats the codec, and those suites assert + * that the packer ran (encode_invariants.sh:139). Separating the two lets + * each be tested for what it is -- the encoders here, the selection policy + * in encode_post_codec.sh. + */ + DefineCustomBoolVariable("pgcolumnar.enable_post_codec_encoding_choice", + "Keep an encoding only when it is smaller after block compression.", + NULL, + &pgcolumnar_enable_post_codec_encoding_choice, + true, + PGC_USERSET, + 0, + NULL, NULL, NULL); + /* * Dev control for #393: off maps every page read 1:1 onto a ranged request * so the request-count suite can measure both arms in one run. Nothing diff --git a/src/columnar_write_state.c b/src/columnar_write_state.c index a12f4a61..ce5669b4 100644 --- a/src/columnar_write_state.c +++ b/src/columnar_write_state.c @@ -1108,6 +1108,8 @@ flush_one_column(Form_pg_attribute att, List *chunkGroups, uint64 rowIdx = 0; StringInfo encoded = makeStringInfo(); StringInfo desc = makeStringInfo(); + StringInfo rawRegion = makeStringInfo(); /* #1132: the unencoded alternative */ + StringInfo rawDesc = makeStringInfo(); uint32 vectorCount = (uint32) list_length(chunkGroups); char *fsstTable = NULL; /* chunk-shared FSST table (E3b), or NULL */ uint32 fsstTableLen = 0; @@ -1157,6 +1159,7 @@ flush_one_column(Form_pg_attribute att, List *chunkGroups, /* descriptor header (columnar_encdesc.h owns the wire layout) */ PgColumnarEncdescPutHeader(desc, vectorCount); + PgColumnarEncdescPutHeader(rawDesc, vectorCount); /* * E3b: build one FSST symbol table for the whole column chunk from a @@ -1324,6 +1327,28 @@ flush_one_column(Form_pg_attribute att, List *chunkGroups, PgColumnarEncdescPutEntry(desc, entryType, entryValueCount, entryRawLen, encLen); + /* + * #1132: carry the unencoded alternative alongside. Encoding is chosen + * per vector on PRE-codec bytes, but the chunk is stored POST-codec, and + * bit-packing whitens a stream the codec was exploiting -- so a vector + * that shrank can still enlarge the stored chunk. Building both here + * lets the decision be made once, below, at the granularity the codec + * actually runs at. + * + * BUILT ONLY WHEN THE DECISION WILL BE TAKEN. This is a full copy of the + * column chunk's value stream, and what a flush holds is already a + * sensitivity here -- see the codec-buffer note below, measured on a + * 200,000-row load in #1075. With the choice off the copy is not made + * and the GUC costs nothing rather than costing memory silently. + */ + if (pgcolumnar_enable_post_codec_encoding_choice) + { + appendBinaryStringInfo(rawRegion, col->valueStream.data, + col->valueStream.len); + PgColumnarEncdescPutEntry(rawDesc, COLUMNAR_ENCODING_NONE, + entryValueCount, entryRawLen, entryRawLen); + } + /* per-vector zone map (native spec 7.1, D5) */ { NativeZoneMapMetadata *z = palloc0(sizeof(NativeZoneMapMetadata)); @@ -1395,6 +1420,18 @@ flush_one_column(Form_pg_attribute att, List *chunkGroups, if (fsstTableLen > 0) appendBinaryStringInfo(desc, fsstTable, fsstTableLen); + /* + * The unencoded alternative needs the same trailing region, but never a + * table: its entries are all COLUMNAR_ENCODING_NONE, so nothing can + * reference one. Writing the length unconditionally keeps the exact-length + * check in columnar_reader.c:1111-1115 satisfied either way. + */ + { + uint32 noSharedTable = 0; + + appendBinaryStringInfo(rawDesc, (char *) &noSharedTable, sizeof(uint32)); + } + /* whole-chunk zone map (vector_index -1) */ { NativeZoneMapMetadata *z = palloc0(sizeof(NativeZoneMapMetadata)); @@ -1487,6 +1524,62 @@ flush_one_column(Form_pg_attribute att, List *chunkGroups, finalLen = compLen; blockCodec = usedType; } + + /* + * #1132: THE ENCODING IS CHOSEN PRE-CODEC AND THE CHUNK IS STORED + * POST-CODEC, so ask the only question that decides the stored size -- + * is the encoded region, once compressed, actually smaller than the raw + * one compressed? Measured on ClickBench hits_0.parquet, the answer was + * no on 16 of 77 fixed-width columns, costing 7.17% of their stored + * bytes, and ClientEventTime alone was stored 2.06x larger than raw. + * + * FSST already decides this way, through PgColumnarFsstHelpsCompressed. + * This is the same question asked for the rest. + * + * ONLY WHEN ENCODING CLAIMED A WIN. `rawRegion->len > encoded->len` is + * the cheap precondition: when the encoders all declined, the two + * regions are the same bytes and compressing twice would buy nothing. + * It also bounds the added cost to chunks where there is a decision to + * make. + */ + if (pgcolumnar_enable_post_codec_encoding_choice && + rawRegion->len > encoded->len) + { + char *rawCodecBuf = NULL; + uint32 rawCompLen; + int rawUsedType; + int rawUsedLevel; + uint32 rawFinalLen; + + PgColumnarCompressValueStream(rawRegion->data, rawRegion->len, + compressionType, + compressionLevel, + &rawCodecBuf, &rawCompLen, + &rawUsedType, &rawUsedLevel); + rawFinalLen = (rawUsedType != COLUMNAR_COMPRESSION_NONE) + ? rawCompLen : rawRegion->len; + + if (rawFinalLen < finalLen) + { + /* + * Storing it unencoded wins. The descriptor must describe the + * bytes actually written, so swap it for the all-NONE one built + * alongside; a descriptor that disagrees with its chunk is a + * decode error, not a size regression. + */ + if (codecBuf != NULL) + pfree(codecBuf); + codecBuf = rawCodecBuf; + finalData = (rawUsedType != COLUMNAR_COMPRESSION_NONE) + ? rawCodecBuf : rawRegion->data; + finalLen = rawFinalLen; + blockCodec = (rawUsedType != COLUMNAR_COMPRESSION_NONE) + ? rawUsedType : COLUMNAR_COMPRESSION_NONE; + desc = rawDesc; + } + else + pfree(rawCodecBuf); + } } if (finalLen > 0) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f8ff057a..a4d6418b 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -202,6 +202,17 @@ differential differential wide point 15;16;17;18;19 never - differential differential wide range 15;16;17;18;19 never - differential differential wide row proj 15;16;17;18;19 never - differential differential wide row scan 15;16;17;18;19 never - +encode_post_codec encode_post_codec a chunk is not stored larger than it would be with no encoding at all 15;16;17;18;19 never - +encode_post_codec encode_post_codec and that column stays far below the no-encoding size, so the win is real 15;16;17;18;19 never - +encode_post_codec encode_post_codec premise: both fixtures hold every row 15;16;17;18;19 never - +encode_post_codec encode_post_codec premise: the tail fixture's range is set by outliers, not by its typical value 15;16;17;18;19 never - +encode_post_codec encode_post_codec the rep column holds no row the heap does not 15;16;17;18;19 never - +encode_post_codec encode_post_codec the rep column preserves its checksum 15;16;17;18;19 never - +encode_post_codec encode_post_codec the rep column reads back exactly what the heap holds 15;16;17;18;19 never - +encode_post_codec encode_post_codec the tail column holds no row the heap does not 15;16;17;18;19 never - +encode_post_codec encode_post_codec the tail column preserves its checksum 15;16;17;18;19 never - +encode_post_codec encode_post_codec the tail column reads back exactly what the heap holds 15;16;17;18;19 never - +encode_post_codec encode_post_codec while a column where encoding genuinely wins still encodes 15;16;17;18;19 never - harness_selftest 030-assertions nothing leaked into the squatter 15;16;17;18;19 never - harness_selftest 030-assertions pgc_port_free says the squatter's port is busy 15;16;17;18;19 never - harness_selftest 030-assertions squatter survived untouched 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 133df503..bc5070c8 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -128,4 +128,4 @@ suites_not_covered 249 # one short. The census command above was never affected because it skips nothing, # but a row count taken the other way disagrees with the tool's own `ledger: rows=` # and reads as an off-by-one in the merge rather than in the command. -checks_never_observed_red 1396 +checks_never_observed_red 1407 diff --git a/test/encode_invariants.sh b/test/encode_invariants.sh index 16dd1a34..f5d31390 100755 --- a/test/encode_invariants.sh +++ b/test/encode_invariants.sh @@ -36,6 +36,15 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" +# #1132: the writer now keeps an encoding only when it is smaller AFTER block +# compression, and this suite's subject is the ENCODERS rather than that choice. +# Its fixtures are chosen to exercise a particular encoder, which is not the same +# as being fixtures where that encoder beats zstd -- two of its controls assert +# the encoder actually ran, and those went red when the choice landed. Pinning the +# pre-#1132 behaviour here keeps this suite testing what it is named for; the +# selection policy has its own suite, encode_post_codec.sh. +psql_run "ALTER DATABASE $PGC_DB SET pgcolumnar.enable_post_codec_encoding_choice = off;" >/dev/null 2>&1 + ROWS="${PGC_ENCINV_ROWS:-4096}" # The C-level half. Bound here rather than shipped, like the other debug hooks. diff --git a/test/encode_post_codec.sh b/test/encode_post_codec.sh new file mode 100755 index 00000000..3f14feaf --- /dev/null +++ b/test/encode_post_codec.sh @@ -0,0 +1,175 @@ +#!/usr/bin/env bash +# +# An encoding is chosen on pre-codec bytes and stored post-codec (#1132). +# +# PgColumnarEncodeChunk picks the smallest candidate against bestLen, which +# starts at rawLen (columnar_encoding.c:2329), and every comparison is +# `len < bestLen` on UNCOMPRESSED bytes. The block codec runs afterwards, once, +# over the whole encoded region (columnar_write_state.c:1479), defaulting to +# zstd level 3. So an encoding that shrinks the bytes can still ENLARGE the +# stored chunk, because bit-packing whitens a stream the codec was exploiting. +# +# FSST is the one encoder that already decides post-codec, through +# PgColumnarFsstHelpsCompressed. This suite is the same question asked of the +# fixed-width encoders, which had no such gate. +# +# THE FIXTURE IS THE ARGUMENT, so it is built from the shape that fails rather +# than from a shape that happens to be convenient. Measured on ClickBench +# hits_0.parquet, the worst column was ClientEventTime: DELTA+FOR stored it +# 2.06x larger than storing it unencoded. Its shape is a HEAVY TAIL -- +# 91,735 distinct values over a range of 1,707,676,369, because rare outliers +# reach back to 1971 while the typical value sits in a narrow recent band. +# +# That splits the two cost models exactly: +# - FOR prices by RANGE, so it must size every value for the outliers; +# - zstd prices by BYTE REDUNDANCY, and the typical value's high bytes are +# constant, so it compresses what FOR spent bits on. +# +# Repetition alone does NOT reproduce it: a narrow-range column with 7,000 +# scattered repeats was measured at 0.71x, encoding genuinely winning. The tail +# is the load-bearing ingredient, which is why the premise below asserts it. +# +# It asserts: +# 1. the tail fixture really is tail-shaped -- its range is set by outliers +# and not by its typical value, observed rather than assumed; +# 2. a chunk is never stored larger than it would be unencoded, which is the +# whole claim; +# 3. a column where encoding genuinely wins still encodes. This is the silent +# direction: declining every encoding would satisfy 2 and reddens nothing; +# 4. the rows read back byte-identical against a heap mirror at every arm. +# +# Usage: test/encode_post_codec.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}" + +ROWS="${PGC_POST_CODEC_ROWS:-200000}" + +# How many vectors of column 0 chose an encoding other than NONE (type 0). +# +# Same descriptor decode as fsst_margin.sh and write_fsst_compressed.sh: a +# 6-byte header (version, reserved, uint32 vector count) then that many 13-byte +# entries. Reading past the entries would score the chunk's trailing shared +# table as encoding types. +encoded_vectors() { # table -> count of non-NONE vectors + q "SELECT coalesce(sum(n), 0) FROM ( + SELECT (SELECT count(*) + FROM generate_series(0, + get_byte(c.encoding_descriptor, 2) + + get_byte(c.encoding_descriptor, 3) * 256 + + get_byte(c.encoding_descriptor, 4) * 65536 + + get_byte(c.encoding_descriptor, 5) * 16777216 - 1) i + WHERE get_byte(c.encoding_descriptor, 6 + i * 13) <> 0) AS n + FROM pgcolumnar.column_chunk c + JOIN pgcolumnar.storage s ON s.storage_id = c.storage_id + WHERE s.relation_oid = '$1'::regclass + AND c.column_index = 0) t;" | tail -1 +} + +# Stored bytes of the column's value stream: the pages minus the validity +# bitmap, which is one bit per row and is written raw ahead of the codec +# (columnar_write_state.c:1079). Subtracted so the number moves only with the +# encoding decision, which is what this suite is about. +value_bytes() { # table -> bytes + q "SELECT coalesce(sum(c.page_length) - sum((c.value_count + 7) / 8), 0) + FROM pgcolumnar.column_chunk c + JOIN pgcolumnar.storage s ON s.storage_id = c.storage_id + WHERE s.relation_oid = '$1'::regclass + AND c.column_index = 0;" | tail -1 +} + +psql_run "SET pgcolumnar.compression = 'zstd';" + +# THE TAIL FIXTURE. 99.9% of values sit in a 5,000-wide band; one in a thousand +# is drawn from a 1.7-billion-wide range. Seeded, so the arms below are reading +# one corpus and not one sample of a family of corpora. +psql_run "SELECT setseed(0.11);" +psql_run "CREATE TABLE epc_tail (v bigint) USING pgcolumnar;" +psql_run "INSERT INTO epc_tail + SELECT CASE WHEN random() < 0.001 + THEN 31525449 + floor(random() * 1700000000)::bigint + ELSE 1739000000 + floor(random() * 5000)::bigint END + FROM generate_series(1, $ROWS) g;" +psql_run "CREATE TABLE epc_tail_h AS SELECT * FROM epc_tail;" + +# THE CONTROL FIXTURE. A narrow range where every value repeats many times, the +# shape hits.EventTime has. Encoding earns its place here: measured 0.29x, so an +# over-eager decline would cost 3.5x on this column. +psql_run "SELECT setseed(0.11);" +psql_run "CREATE TABLE epc_rep (v bigint) USING pgcolumnar;" +psql_run "INSERT INTO epc_rep + SELECT 1700000000 + (g % 86400)::bigint FROM generate_series(1, $ROWS) g;" +psql_run "CREATE TABLE epc_rep_h AS SELECT * FROM epc_rep;" + +tail_range="$(q "SELECT max(v) - min(v) FROM epc_tail;" | tail -1)" +# 1st to 99th percentile, NOT 0.1st to 99.9th. One row in a thousand is an +# outlier, so a 99.9th percentile lands exactly ON the boundary and reads either +# the band width or the full range depending on whether the draw produced a +# hair more or fewer than 200 outliers. Measured both ways from the same seed: +# 4,994 and 53,786,536. The 99th is far enough inside the body to be a property +# of the fixture rather than of the sample. +tail_body="$(q "SELECT (percentile_disc(0.99) WITHIN GROUP (ORDER BY v) + - percentile_disc(0.01) WITHIN GROUP (ORDER BY v)) FROM epc_tail;" | tail -1)" +tail_enc="$(encoded_vectors epc_tail)" +rep_enc="$(encoded_vectors epc_rep)" +tail_bytes="$(value_bytes epc_tail)" +rep_bytes="$(value_bytes epc_rep)" +raw_bytes=$((ROWS * 8)) + +echo "-- tail: range=$tail_range body-spread=$tail_body encoded_vectors=$tail_enc bytes=$tail_bytes" +echo "-- rep : encoded_vectors=$rep_enc bytes=$rep_bytes raw=$raw_bytes" + +# PREMISE: the fixture is tail-shaped. Without this the arms below could pass on +# a corpus that is merely narrow, where nothing interesting is being decided. +check "premise: the tail fixture's range is set by outliers, not by its typical value" \ + "$(awk -v r="$tail_range" -v p="$tail_body" \ + 'BEGIN { print (r > 1000000000 && p < 100000) ? "tail-shaped" : "not-tail-shaped" }')" \ + "tail-shaped" + +check "premise: both fixtures hold every row" \ + "$(q "SELECT (SELECT count(*) FROM epc_tail) || '/' || (SELECT count(*) FROM epc_rep);" | tail -1)" \ + "$ROWS/$ROWS" + +# THE ARM. Storing the chunk unencoded is always available, so no encoding +# decision may ever land above it. Measured before this was fixed: FOR was +# chosen and stored 681,699 bytes where unencoded zstd stores 440,492, which is +# 42.6% and 27.5% of raw respectively. The 35% gate sits between them. +check "a chunk is not stored larger than it would be with no encoding at all" \ + "$(awk -v b="$tail_bytes" -v r="$raw_bytes" \ + 'BEGIN { print (b <= r * 0.35) ? "not-inflated" : "inflated" }')" \ + "not-inflated" + +# THE SILENT DIRECTION. Declining every encoding would satisfy the arm above and +# redden nothing, so the control ships beside it rather than after it. +check "while a column where encoding genuinely wins still encodes" \ + "$(awk -v n="$rep_enc" 'BEGIN { print (n + 0 > 0) ? "encoded" : "declined" }')" \ + "encoded" + +check "and that column stays far below the no-encoding size, so the win is real" \ + "$(awk -v b="$rep_bytes" -v r="$raw_bytes" \ + 'BEGIN { print (b <= r * 0.10) ? "small" : "large" }')" \ + "small" + +# THE INVARIANT. 2 and 3 exist to prove this one is not vacuous: a decision that +# changes what comes back out is a data-loss bug, not a size regression. +# Named for the COLUMN SHAPE rather than the table, so the property is the name +# and the pytest twin can assert it over its own fixture (CONTEXT.md's +# independence rule) instead of inheriting this suite's table names. +for t in tail rep; do + tbl="epc_$t" + check "the $t column reads back exactly what the heap holds" \ + "$(q "SELECT count(*) FROM ( + SELECT v FROM $tbl EXCEPT ALL SELECT v FROM ${tbl}_h) d;" | tail -1)" \ + "0" + check "the $t column holds no row the heap does not" \ + "$(q "SELECT count(*) FROM ( + SELECT v FROM ${tbl}_h EXCEPT ALL SELECT v FROM $tbl) d;" | tail -1)" \ + "0" + check "the $t column preserves its checksum" \ + "$(q "SELECT coalesce(sum(v), 0) FROM $tbl;" | tail -1)" \ + "$(q "SELECT coalesce(sum(v), 0) FROM ${tbl}_h;" | tail -1)" +done + +pgc_summary diff --git a/test/native_dict_underfill.sh b/test/native_dict_underfill.sh index 0cbcaa9f..bf32d082 100755 --- a/test/native_dict_underfill.sh +++ b/test/native_dict_underfill.sh @@ -7,6 +7,15 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" +# #1132: the writer now keeps an encoding only when it is smaller AFTER block +# compression, and this suite's subject is the ENCODERS rather than that choice. +# Its fixtures are chosen to exercise a particular encoder, which is not the same +# as being fixtures where that encoder beats zstd -- two of its controls assert +# the encoder actually ran, and those went red when the choice landed. Pinning the +# pre-#1132 behaviour here keeps this suite testing what it is named for; the +# selection policy has its own suite, encode_post_codec.sh. +psql_run "ALTER DATABASE $PGC_DB SET pgcolumnar.enable_post_codec_encoding_choice = off;" >/dev/null 2>&1 + q "CREATE TABLE dt (id int, v text) USING pgcolumnar" >/dev/null # low-cardinality short text -> DICT-encoded varlena column, one small chunk q "INSERT INTO dt SELECT g, (ARRAY['aa','bb','cc','dd'])[1+(g%4)] FROM generate_series(1,300) g" >/dev/null diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 33fd40ab..9f73d87c 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -101,6 +101,7 @@ behaviour, the source of that number is named. - [53. test_analyze_reltuples.py: ANALYZE must estimate the row count, not zero](#53-test_analyze_reltuplespy-analyze-must-estimate-the-row-count-not-zero) - [54. test_projection_update.py: UPDATE must fan the new row number out to projections](#54-test_projection_updatepy-update-must-fan-the-new-row-number-out-to-projections) - [55. test_projection_drop_column.py: DROP COLUMN must not invalidate a projection](#55-test_projection_drop_columnpy-drop-column-must-not-invalidate-a-projection) +- [56. test_encode_post_codec.py: an encoding must be smaller after the codec](#56-test_encode_post_codecpy-an-encoding-must-be-smaller-after-the-codec) ## 1. How to read a test in here @@ -4709,3 +4710,47 @@ compares them to each other, so indistinguishability is asserted rather than imp | --- | --- | | `test_drop_column_is_refused_while_a_projection_depends_on_it` | every arm: the two non-owner refusals, the dependency refusal, writability afterwards, the unrelated column, and the partitioned parent | +## 56. test_encode_post_codec.py: an encoding must be smaller after the codec + +#1132. `PgColumnarEncodeChunk` compares every candidate against `bestLen`, +which starts at `rawLen`, and every comparison is on UNCOMPRESSED bytes. The +block codec runs afterwards, once, over the whole encoded region, defaulting to +zstd level 3. Bit-packing whitens a stream the codec was exploiting, so an +encoding that shrank the bytes can **enlarge** the stored chunk. + +FSST already decided post-codec through `PgColumnarFsstHelpsCompressed`. This +file asks the same question of the encoders that had no such gate. Measured on +ClickBench `hits_0.parquet`, 16 of 77 fixed-width columns were stored larger +encoded than raw, costing 7.17% of their stored bytes. + +**THE FIXTURE IS THE ARGUMENT, and five synthetic shapes failed to reproduce the +defect before the sixth did.** The worst real column, `ClientEventTime`, is a +HEAVY TAIL: rare outliers stretch the range while the typical value stays in a +narrow band. That splits the two cost models exactly -- frame-of-reference +prices by RANGE and must size every value for the outliers, while zstd prices by +BYTE REDUNDANCY and the typical value's high bytes are constant. Repetition +alone does not reproduce it (measured 0.71x, encoding winning), so the tail is +load-bearing and the first premise asserts it. + +**That premise was a coin flip in its first version.** It read the 0.1st-to-99.9th +percentile spread, which with one outlier in a thousand lands exactly ON the +boundary: from one seed it measured 4,994 on one run and 53,786,536 on another, +green then red with no code change between. It reads the 1st-to-99th percentile +now, measured at 4905 / 4905 / 4904 across three runs. + +Public seam: the encoding descriptor and `column_chunk.page_length`. Independent +of `test/encode_post_codec.sh`, which reads the descriptor through `get_byte()` +in SQL while this decodes it in Python, over its own cluster, its own table +names and its own corpus constants. Neither file names the other. + +### Every arm + +| test | what it holds | +| --- | --- | +| `test_a_chunk_is_never_stored_larger_than_unencoded` | the arm, its two premises, and both controls | +| `test_the_decision_never_changes_what_comes_back_out` | the invariant the size arms exist to prove is not vacuous | + +The controls are the load-bearing half. Declining every encoding would satisfy +"never larger than unencoded" and redden nothing, so the repeating column ships +beside the tail one and asserts that encoding is still chosen where it wins. + diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 91e624ac..2e441d7c 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -297,4 +297,17 @@ guard_tests 380 # test_analyze_reltuples.py, test_projection_update.py and # test_projection_drop_column.py, one collected test each. Re-derived BY COLLECTION on # this tree, never by adding three. -cluster_tests 421 +# 421 -> 423 when test_encode_post_codec.py landed (#1132): two arms over the +# post-codec encoding choice -- the size arm with its two premises and both +# controls, and the read-back invariant those exist to prove is not vacuous. +# +# DERIVED BY COLLECTION AFTER THE REBASE, which is the only reason it is right. +# This branch first stated 421 against a base of 418 and the rebase onto main +# carrying #1133 moved the base to 421 underneath it, so the committed number +# described a tree that no longer existed. CI named it -- `collected 423 test(s) +# but expected 421` -- rather than running a different corpus than the one +# declared, which is the whole point of the key. +# `guard_tests` was re-derived in the same run and did NOT move: 380. That is the +# expected answer for a file that needs a cluster, and checking it rather than +# assuming it is what this file asks for. +cluster_tests 423 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 720f0f6b..160021ad 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -86,7 +86,7 @@ # The shape is `SHELL_REFERENCES`' in `test_harness_deps.py`, asserted in both # directions for the same reason: a one-way list rots into a permanent exemption. COMPLETE = ["analyze_reltuples", - "differential", "hilbert_cluster", "hilbert_locality", + "differential", "encode_post_codec", "hilbert_cluster", "hilbert_locality", "native_chunk_length_bound", "native_fetch_coalesce", "native_ownership", "native_projection", "parallel_am_scan", "projection_drop_column", "projection_privilege", diff --git a/test/pytest/test_encode_post_codec.py b/test/pytest/test_encode_post_codec.py new file mode 100644 index 00000000..8743881d --- /dev/null +++ b/test/pytest/test_encode_post_codec.py @@ -0,0 +1,201 @@ +"""An encoding is chosen pre-codec and stored post-codec (#1132). + +`PgColumnarEncodeChunk` picks the smallest candidate against `bestLen`, which +starts at `rawLen`, and every comparison is on UNCOMPRESSED bytes. The block +codec runs afterwards, once, over the whole encoded region. So an encoding that +shrinks the bytes can still ENLARGE the stored chunk, because bit-packing +whitens a stream the codec was exploiting. + +FSST already decides post-codec through `PgColumnarFsstHelpsCompressed`. This +is the same question asked of the encoders that had no such gate. + +Independent of `test/encode_post_codec.sh` per CONTEXT.md: same public seams -- +the encoding descriptor and `column_chunk.page_length` -- but its own cluster, +its own table names, its own corpus constants, and the descriptor decoded here +in Python rather than through `get_byte()` in SQL. The two agree on the +property, not on the implementation. + +THE FIXTURE IS THE ARGUMENT. Measured on ClickBench `hits_0.parquet`, the worst +column was `ClientEventTime`: stored 2.06x larger encoded than raw. Its shape is +a heavy tail -- rare outliers stretch the range while the typical value stays in +a narrow band -- which splits the two cost models exactly: + + * FOR prices by RANGE, so every value is sized for the outliers; + * zstd prices by BYTE REDUNDANCY, and the typical value's high bytes are + constant, so it compresses what FOR spent bits on. + +Repetition alone does NOT reproduce it (measured 0.71x, encoding winning), which +is why the first premise asserts the tail rather than assuming it. +""" + +ROWS = 120000 +BAND = 4000 # width of the band the typical value sits in +TAIL_ONE_IN = 1000 # one row in this many is an outlier +TAIL_SPAN = 1700000000 # how far the outliers reach back + +NONE_ENCODING_TYPE = 0 + + +def _descriptor_encodings(cur, table): + """Encoding type of every vector of column 0, read from the descriptor. + + 6-byte header -- version, a reserved byte, then the vector count as uint32 + little-endian -- followed by that many 13-byte entries whose first byte is + the encoding type. The count bounds the scan: reading to the descriptor's + length would score the trailing shared-table region as encoding types. + """ + cur.execute( + """ + SELECT c.encoding_descriptor + FROM pgcolumnar.column_chunk c + JOIN pgcolumnar.storage s ON s.storage_id = c.storage_id + WHERE s.relation_oid = %s::regclass + AND c.column_index = 0 + """, + (table,), + ) + out = [] + for (desc,) in cur.fetchall(): + blob = bytes(desc) + if len(blob) < 6: + continue + count = int.from_bytes(blob[2:6], "little") + for i in range(count): + at = 6 + i * 13 + if at < len(blob): + out.append(blob[at]) + return out + + +def _value_bytes(cur, table): + """Stored bytes of the value stream: pages minus the validity bitmap. + + The bitmap is one bit per row and is written raw ahead of the codec, so it + is subtracted to leave a number that moves only with the encoding decision. + """ + cur.execute( + """ + SELECT coalesce(sum(c.page_length) - sum((c.value_count + 7) / 8), 0) + FROM pgcolumnar.column_chunk c + JOIN pgcolumnar.storage s ON s.storage_id = c.storage_id + WHERE s.relation_oid = %s::regclass + AND c.column_index = 0 + """, + (table,), + ) + return int(cur.fetchone()[0]) + + +def _load(cur, table, expr): + cur.execute(f"DROP TABLE IF EXISTS {table}") + cur.execute(f"DROP TABLE IF EXISTS {table}_h") + cur.execute("SET pgcolumnar.compression = 'zstd'") + cur.execute("SELECT setseed(0.11)") + cur.execute(f"CREATE TABLE {table} (v bigint) USING pgcolumnar") + cur.execute( + f"INSERT INTO {table} SELECT {expr} FROM generate_series(1, {ROWS}) g" + ) + cur.execute(f"CREATE TABLE {table}_h AS SELECT * FROM {table}") + + +TAIL_EXPR = ( + f"CASE WHEN random() < {1.0 / TAIL_ONE_IN} " + f"THEN 31525449 + floor(random() * {TAIL_SPAN})::bigint " + f"ELSE 1739000000 + floor(random() * {BAND})::bigint END" +) +REP_EXPR = "1700000000 + (g % 86400)::bigint" + + +def test_a_chunk_is_never_stored_larger_than_unencoded(pgc_conn, expect): + """The arm, with the control that stops it being satisfied by giving up. + + Storing the chunk unencoded is always available, so no encoding decision may + land above it. Declining every encoding would satisfy that and redden + nothing, so the repeating column ships beside the tail one. + """ + with pgc_conn.cursor() as c: + _load(c, "pc_tail", TAIL_EXPR) + _load(c, "pc_rep", REP_EXPR) + + c.execute("SELECT max(v) - min(v) FROM pc_tail") + rng = int(c.fetchone()[0]) + # 1st to 99th percentile, NOT 0.1st to 99.9th. One row in a thousand is + # an outlier, so a 99.9th percentile sits exactly ON the boundary and + # reads either the band width or the full range depending on whether the + # draw produced a hair more or fewer outliers than expected. Measured + # both ways from one seed: 4,994 and 53,786,536. + c.execute( + "SELECT percentile_disc(0.99) WITHIN GROUP (ORDER BY v)" + " - percentile_disc(0.01) WITHIN GROUP (ORDER BY v) FROM pc_tail" + ) + spread = int(c.fetchone()[0]) + + expect.text( + "tail-shaped" if rng > 1000000000 and spread < 100000 else "not-tail-shaped", + "tail-shaped", + "premise: the tail fixture's range is set by outliers, not by its typical value", + ) + + c.execute("SELECT (SELECT count(*) FROM pc_tail), (SELECT count(*) FROM pc_rep)") + n_tail, n_rep = c.fetchone() + expect.text( + f"{n_tail}/{n_rep}", f"{ROWS}/{ROWS}", + "premise: both fixtures hold every row", + ) + + raw = ROWS * 8 + tail_bytes = _value_bytes(c, "pc_tail") + rep_bytes = _value_bytes(c, "pc_rep") + + expect.text( + "not-inflated" if tail_bytes <= raw * 0.35 else "inflated", + "not-inflated", + "a chunk is not stored larger than it would be with no encoding at all", + ) + + rep_encodings = _descriptor_encodings(c, "pc_rep") + expect.text( + "encoded" if any(e != NONE_ENCODING_TYPE for e in rep_encodings) else "declined", + "encoded", + "while a column where encoding genuinely wins still encodes", + ) + expect.text( + "small" if rep_bytes <= raw * 0.10 else "large", + "small", + "and that column stays far below the no-encoding size, so the win is real", + ) + + +def test_the_decision_never_changes_what_comes_back_out(pgc_conn, expect): + """The invariant the size arms exist to prove is not vacuous. + + A decision that changes the rows is a data-loss bug, not a size regression, + and swapping the descriptor for the chunk is exactly where that would go + wrong. + """ + with pgc_conn.cursor() as c: + _load(c, "pc_tail", TAIL_EXPR) + _load(c, "pc_rep", REP_EXPR) + + for label, tbl in (("tail", "pc_tail"), ("rep", "pc_rep")): + c.execute( + f"SELECT count(*) FROM (SELECT v FROM {tbl} " + f"EXCEPT ALL SELECT v FROM {tbl}_h) d" + ) + expect.num( + int(c.fetchone()[0]), 0, + f"the {label} column reads back exactly what the heap holds", + ) + c.execute( + f"SELECT count(*) FROM (SELECT v FROM {tbl}_h " + f"EXCEPT ALL SELECT v FROM {tbl}) d" + ) + expect.num( + int(c.fetchone()[0]), 0, + f"the {label} column holds no row the heap does not", + ) + c.execute(f"SELECT coalesce(sum(v), 0) FROM {tbl}") + got = int(c.fetchone()[0]) + c.execute(f"SELECT coalesce(sum(v), 0) FROM {tbl}_h") + want = int(c.fetchone()[0]) + expect.num(got, want, f"the {label} column preserves its checksum") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index c6d74c28..1ae9071d 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -77,6 +77,7 @@ SUITES=( eager_ordering_record encode_effort encode_invariants + encode_post_codec entry_point_privilege estimate_deleted export_sink