diff --git a/CHANGELOG.md b/CHANGELOG.md index d72be83ed..c4416dbdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -356,6 +356,15 @@ true until the next version shipped. ### Fixed +- Changing a text column's collation without a rewrite no longer gives wrong + answers (#1327). `ALTER COLUMN ... TYPE text COLLATE ` keeps the + table's files, so the zone maps kept minima and maxima ordered under the old + collation. Ranges, equalities and `min()`/`max()` then skipped rows that the + new collation orders differently. The column's stored bounds are now + forgotten after such a change, in the table and in its projections, and its + extent is dropped. Queries stay correct and read those groups until a + rewrite builds new zone maps. `test/collation_change.sh` and an independent + `test_collation_change.py`. Reported by @jdatcmd. - `test/native_upgrade_converge.sh` no longer leaves its PostgreSQL server running and its workdir in `/tmp` after every run (#1321). It armed its own `trap cleanup EXIT` after `pgc_setup`, which replaced the harness's diff --git a/src/columnar.h b/src/columnar.h index 7ea050e4d..df0fca080 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -629,6 +629,7 @@ extern void PgColumnarInsertExtentRow(uint64 storageId, int columnIndex, bool co const char *maximum, int maximumLen); extern List *PgColumnarReadExtentRows(uint64 storageId, int columnIndex, Snapshot snapshot); extern void PgColumnarDeleteExtentRow(ItemPointer tid); +extern void PgColumnarForgetColumnOrdering(uint64 storageId, int columnIndex); extern Oid PgColumnarStorageRelationOid(uint64 storageId, Snapshot snapshot); /* the extent's lifecycle: columnar_extent.c */ diff --git a/src/columnar_metadata.c b/src/columnar_metadata.c index c5277c037..37228e17a 100644 --- a/src/columnar_metadata.c +++ b/src/columnar_metadata.c @@ -4761,6 +4761,84 @@ PgColumnarDeleteExtentRow(ItemPointer tid) table_close(rel, RowExclusiveLock); } +/* + * PgColumnarForgetColumnOrdering + * The column's stored minima and maxima were computed under an ordering that + * no longer holds: its collation changed and nothing was rewritten (#1327). + * + * Every zone-map row of the column, whole-chunk and per-vector, loses its + * minimum and maximum, and the column's extent rows are deleted. The rows stay, + * with their counts and sums, which no ordering decides. A zone map with values + * and no minimum and maximum is the state a type with no ordering already + * writes, and every reader already treats it as "nothing is known": no group is + * pruned, min() and max() decode, expire keeps the group, and a recompute of the + * extent refuses, so the extent stays absent until a rewrite builds new zone + * maps. + * + * Forgetting rather than recomputing: a recompute decodes the column in every + * group, inside the ALTER and under its ACCESS EXCLUSIVE lock, which is the cost + * the collation change was spared by not rewriting. + */ +void +PgColumnarForgetColumnOrdering(uint64 storageId, int columnIndex) +{ + Relation rel = open_columnar_table("zone_map", RowExclusiveLock); + TupleDesc td = RelationGetDescr(rel); + ScanKeyData key[1]; + SysScanDesc scan; + HeapTuple tuple; + Oid idx; + Datum values[Natts_zone_map]; + bool nulls[Natts_zone_map]; + bool replace[Natts_zone_map]; + + memset(values, 0, sizeof(values)); + memset(nulls, 0, sizeof(nulls)); + memset(replace, 0, sizeof(replace)); + nulls[Anum_zone_map_minimum - 1] = replace[Anum_zone_map_minimum - 1] = true; + nulls[Anum_zone_map_maximum - 1] = replace[Anum_zone_map_maximum - 1] = true; + + ScanKeyInit(&key[0], Anum_zone_map_storage_id, BTEqualStrategyNumber, + F_INT8EQ, Int64GetDatum((int64) storageId)); + idx = pgcolumnar_scan_index_oid(rel, "zone_map_pkey"); + scan = systable_beginscan(rel, idx, OidIsValid(idx), NULL, 1, key); + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + bool isnull; + HeapTuple updated; + + if (DatumGetInt16(heap_getattr(tuple, Anum_zone_map_column_index, td, + &isnull)) != columnIndex) + continue; + (void) heap_getattr(tuple, Anum_zone_map_minimum, td, &isnull); + if (isnull) + continue; /* already no bound: an all-NULL unit */ + updated = heap_modify_tuple(tuple, td, values, nulls, replace); + CatalogTupleUpdate(rel, &updated->t_self, updated); + heap_freetuple(updated); + } + systable_endscan(scan); + table_close(rel, RowExclusiveLock); + + if (PgColumnarExtentCatalogPresent()) + { + ListCell *lc; + Snapshot snap; + + /* + * Registered: PG18 asserts that a snapshot used for a heap scan is + * registered or active, and an assert build aborts on an unregistered + * one (measured: every collation change crashed on 18 and 19). + */ + CommandCounterIncrement(); + snap = RegisterSnapshot(GetLatestSnapshot()); + foreach(lc, PgColumnarReadExtentRows(storageId, columnIndex, snap)) + PgColumnarDeleteExtentRow(&((NativeExtentRow *) lfirst(lc))->tid); + UnregisterSnapshot(snap); + } + CommandCounterIncrement(); +} + /* * The relation a storage id belongs to, from pgcolumnar.storage, or InvalidOid. * A projection's storage names its base relation too; callers that need the diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 647485943..3ccdc5eaf 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -3187,6 +3187,145 @@ pgcolumnar_reject_drop_projected_column(AlterTableStmt *stmt) list_free(relations); } +/* + * A COLLATION CHANGE THAT REWRITES NOTHING (#1327). + * + * ALTER COLUMN ... TYPE text COLLATE keeps the relfilenode when the type + * is unchanged, so no new zone maps are written, and the old ones hold minima + * and maxima under the old collation. The reader compares them under the new + * one, and pruned rows the new collation orders differently: @jdatcmd measured + * min|max B|m where heap said a|m, and WHERE s < 'B' returning nothing. + * + * Before the statement, each columnar relation it could alter -- the named one + * and every inheritor, which is how an ALTER on a partitioned parent reaches a + * columnar partition -- has its relfilenode and its columns' collations + * recorded from the syscache. Opening the relations here would take a lock the + * ALTER then upgrades. After the statement, a relation with the same + * relfilenode was not rewritten, and any column whose collation differs has its + * ordering forgotten (PgColumnarForgetColumnOrdering). A rewritten relation + * wrote new zone maps under the new collation and needs nothing. + */ +typedef struct PgColumnarCollationsBefore +{ + Oid relid; + Oid relfilenode; + int natts; + Oid *collations; /* by attnum - 1 */ +} PgColumnarCollationsBefore; + +static Oid +pgcolumnar_attcollation(Oid relid, AttrNumber attnum) +{ + HeapTuple tp = SearchSysCache2(ATTNUM, ObjectIdGetDatum(relid), Int16GetDatum(attnum)); + Oid coll = InvalidOid; + + if (HeapTupleIsValid(tp)) + { + coll = ((Form_pg_attribute) GETSTRUCT(tp))->attcollation; + ReleaseSysCache(tp); + } + return coll; +} + +static List * +pgcolumnar_collations_before(AlterTableStmt *stmt) +{ + List *out = NIL; + List *rels; + ListCell *lc; + bool changesType = false; + Oid relid; + + foreach(lc, stmt->cmds) + if (((AlterTableCmd *) lfirst(lc))->subtype == AT_AlterColumnType) + changesType = true; + if (!changesType || stmt->relation == NULL) + return NIL; + relid = RangeVarGetRelid(stmt->relation, NoLock, true); + if (!OidIsValid(relid)) + return NIL; + rels = find_all_inheritors(relid, NoLock, NULL); + foreach(lc, rels) + { + Oid r = lfirst_oid(lc); + HeapTuple tp; + Form_pg_class cls; + PgColumnarCollationsBefore *b; + int a; + + if (!PgColumnarIsColumnarRelation(r)) + continue; + tp = SearchSysCache1(RELOID, ObjectIdGetDatum(r)); + if (!HeapTupleIsValid(tp)) + continue; + cls = (Form_pg_class) GETSTRUCT(tp); + b = palloc0(sizeof(PgColumnarCollationsBefore)); + b->relid = r; + b->relfilenode = cls->relfilenode; + b->natts = cls->relnatts; + ReleaseSysCache(tp); + b->collations = palloc0(sizeof(Oid) * Max(b->natts, 1)); + for (a = 1; a <= b->natts; a++) + b->collations[a - 1] = pgcolumnar_attcollation(r, a); + out = lappend(out, b); + } + return out; +} + +static void +pgcolumnar_forget_changed_collations(List *before) +{ + ListCell *lc; + + foreach(lc, before) + { + PgColumnarCollationsBefore *b = (PgColumnarCollationsBefore *) lfirst(lc); + HeapTuple tp; + Oid relfilenode; + Relation rel; + uint64 storageId; + int a; + + tp = SearchSysCache1(RELOID, ObjectIdGetDatum(b->relid)); + if (!HeapTupleIsValid(tp)) + continue; + relfilenode = ((Form_pg_class) GETSTRUCT(tp))->relfilenode; + ReleaseSysCache(tp); + if (relfilenode != b->relfilenode || !PgColumnarIsColumnarRelation(b->relid)) + continue; /* rewritten: its zone maps are new */ + rel = relation_open(b->relid, AccessShareLock); + storageId = PgColumnarStorageId(rel); + for (a = 1; a <= Min(b->natts, RelationGetDescr(rel)->natts); a++) + { + Form_pg_attribute att = TupleDescAttr(RelationGetDescr(rel), a - 1); + + ListCell *pc; + + if (att->attisdropped || att->attcollation == b->collations[a - 1]) + continue; + PgColumnarForgetColumnOrdering(storageId, a - 1); + + /* + * A projection stores the column in its own storage, with zone maps + * of its own, laid out [rownumber, projcol1..projcolK]: the column at + * position j of the projection's list is column index j + 1. + */ + foreach(pc, PgColumnarListProjections(storageId)) + { + PgColumnarProjection *proj = (PgColumnarProjection *) lfirst(pc); + int j; + + if (proj->projectionId == 0) + continue; + for (j = 0; j < proj->columnsLen; j++) + if (proj->columns[j] == a) + PgColumnarForgetColumnOrdering(proj->projStorageId, j + 1); + } + } + relation_close(rel, AccessShareLock); + } +} + static void pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString, bool readOnlyTree, ProcessUtilityContext context, @@ -3194,6 +3333,7 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString, DestReceiver *dest, QueryCompletion *qc) { Node *parsetree = pstmt->utilityStmt; + List *collationsBefore = NIL; /* * Start from empty, but only for the OUTERMOST statement: a statement that @@ -3211,6 +3351,7 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString, pgcolumnar_reject_drop_projected_column(stmt); pgcolumnar_reject_set_am_to_columnar(stmt); + collationsBefore = pgcolumnar_collations_before(stmt); } PG_TRY(); @@ -3229,6 +3370,9 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString, } PG_END_TRY(); + /* #1327: after the statement, so a change that errored leaves nothing */ + pgcolumnar_forget_changed_collations(collationsBefore); + /* * A rewrite loses this relation's declared projections, so re-record them * (#876, #887). diff --git a/src/columnar_vector.c b/src/columnar_vector.c index c02e2accb..81c82fb35 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -3785,6 +3785,21 @@ pgcolumnar_fill_native_metadata_agg(PgColumnarAggScanState *state, int *ndirty) missingColumn = true; break; } + + /* + * Values with no minimum and maximum: a type with no ordering, + * or a column whose collation changed without a rewrite, whose + * bounds were forgotten (#1327). The zone map cannot answer + * min() or max() for it, and skipping it as if all NULL + * returned NULL where heap returned a value. + */ + if ((state->specs[a].kind == COLUMNAR_AGG_MIN || + state->specs[a].kind == COLUMNAR_AGG_MAX) && + !byCol[ai]->hasMinMax && byCol[ai]->valueCount > 0) + { + missingColumn = true; + break; + } } if (missingColumn) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 2b17c703c..1fb66591b 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -204,6 +204,17 @@ catalog_scan_index catalog_scan_index premise: truncating a projected table reac catalog_scan_index catalog_scan_index premise: truncating a projected table took effect 15;16;17;18;19 never - catalog_scan_index catalog_scan_index renaming a projected column does not read pgcolumnar.projection_declaration whole 15;16;17;18;19 2026-10-12 the twelve converted sites reverted to InvalidOid, so each reads its catalog whole catalog_scan_index catalog_scan_index truncating a projected table does not read pgcolumnar.projection_declaration whole 15;16;17;18;19 2026-10-12 the twelve converted sites reverted to InvalidOid, so each reads its catalog whole +collation_change collation_change a collation change made through a partitioned parent reaches the columnar partition 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour);only the named relation, not its inheritors +collation_change collation_change a projection sorted on the column answers a range as heap does 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour);the projection column forgotten at its list position, without the rownumber column's offset;the projection storages' bounds not forgotten +collation_change collation_change a table sorted under the old collation orders ORDER BY ... LIMIT as heap does 15;16;17;18;19 2026-10-13 the sorted-pathkeys claim's refusal of collatable columns removed +collation_change collation_change after the collation change min and max are heap's 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour);the min/max fold skips a group with values and no bounds as all-NULL +collation_change collation_change and a range below an upper-case bound returns heap's rows 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour) +collation_change collation_change and an equality on a value C sorts first returns heap's rows 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour) +collation_change collation_change premise: the collation change rewrote nothing 15;16;17;18;19 never - +collation_change collation_change premise: the columnar table has several row groups 15;16;17;18;19 never - +collation_change collation_change premise: the new collation orders a lower-case letter before an upper-case one that C puts first 15;16;17;18;19 never - +collation_change collation_change rows written after the change join heap's answer 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour) +collation_change collation_change the column's extent is not the old collation's bound 15;16;17;18;19 2026-10-13 nothing forgotten after a collation change (main's behaviour);the column's extent rows kept column_extent column_extent DROP TABLE removes them 15;16;17;18;19 2026-10-13 PgColumnarDeleteMetadata leaves the extent rows column_extent column_extent TRUNCATE removes the old storage's extent rows 15;16;17;18;19 2026-10-13 PgColumnarDeleteMetadata leaves the extent rows column_extent column_extent a column added after older groups has no complete extent (those groups have no zone map for it) 15;16;17;18;19 2026-10-13 a recompute that ignores a group with no zone map for the column;pgcolumnar.column_extent returns a row with no complete extent;the extent trusted without a complete row diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 3d6f745e9..df207daad 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -1559,4 +1559,21 @@ suites_not_covered 246 # never 1745 + 51 = 1796 # dated 447 + 16 = 463 # 1796 + 463 == 2259 -checks_never_observed_red 1796 +# +# #1327, a collation change without a rewrite (OffgridwithJD): 1796 -> 1799. +# collation_change enters the ledger with 11 checks from a five-major run +# (collation_change.sh 11/11 on each of 15-19). Eight arms are dated by seven +# mutations, each merged one log at a time with --mutation and --target: +# nothing forgotten (main's behaviour, seven arms), the projection storages +# skipped, the projection column at the wrong index, the named relation only, +# the min/max fold treating a group with values and no bounds as all-NULL, the +# extent rows kept, and the sorted-pathkeys claim's refusal of collatable +# columns removed. The three never rows are premises about the fixture and the +# server: the collation orders differently from C, several row groups, and no +# rewrite. +# +# ledger rows 2259 + 11 = 2270 +# never 1796 + 3 = 1799 +# dated 463 + 8 = 471 +# 1799 + 471 == 2270 +checks_never_observed_red 1799 diff --git a/test/collation_change.sh b/test/collation_change.sh new file mode 100755 index 000000000..c81fdac3d --- /dev/null +++ b/test/collation_change.sh @@ -0,0 +1,120 @@ +#!/usr/bin/env bash +# +# A column's collation changed without a rewrite (#1327). +# +# ALTER COLUMN ... TYPE text COLLATE keeps the relfilenode: the type is +# the same, so PostgreSQL rewrites nothing. The zone maps kept the min and max +# computed under the OLD collation, and the reader compared them under the new +# one, so chunk-group pruning and the min/max path skipped rows the new +# collation orders differently. Reported by @jdatcmd while reviewing #1325, on +# main at 036d452: min|max read B|m where heap read a|m, and WHERE s < 'B' +# returned nothing where heap returned a,b. +# +# THE ORACLE IS HEAP: the same rows, the same ALTER, the same queries. +# +# A collation that orders 'a' before 'B' is needed, which "C" does not. libc's +# en_US.utf8 is used. A server without one is an environment defect (pgc_skip +# fails it unless PGC_ALLOW_MISSING_EN_US_COLLATION waives it), never a pass on a +# collation that cannot show the bug. +# +# Usage: test/collation_change.sh [PG_CONFIG] + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +q "CREATE EXTENSION IF NOT EXISTS pgcolumnar;" >/dev/null + +COLL="$(q "SELECT collname FROM pg_collation WHERE collname IN ('en_US.utf8', 'en_US.UTF-8', 'en_US') + AND collencoding IN (-1, pg_char_to_encoding(current_setting('server_encoding'))) + ORDER BY collname LIMIT 1;")" +[ -n "$COLL" ] || pgc_skip en_us_collation "the server has no en_US collation to change a C column to" +check "premise: the new collation orders a lower-case letter before an upper-case one that C puts first" \ + "$(q "SELECT ('a' < 'B' COLLATE \"$COLL\")::text || '|' || ('a' < 'B' COLLATE \"C\")::text;")" "true|false" + +# Three row groups, each holding upper- and lower-case values, so a group's C +# minimum is an upper-case letter that the new collation sorts AFTER some of its +# lower-case values: under C every group's minimum is >= 'B', so a stale zone map +# prunes every group from WHERE s < 'B'. +for t in cc ch; do + am="USING pgcolumnar"; [ "$t" = ch ] && am="" + q "CREATE TABLE $t (k int, s text COLLATE \"C\") $am;" >/dev/null +done +q "SELECT pgcolumnar.set_options('cc', stripe_row_limit => 1000, chunk_group_row_limit => 1000);" >/dev/null +for t in cc ch; do + q "INSERT INTO $t SELECT g, CASE g % 4 WHEN 0 THEN 'a' WHEN 1 THEN 'B' WHEN 2 THEN 'm' ELSE 'b' END || (g % 7) + FROM generate_series(1, 3000) g;" >/dev/null +done +check "premise: the columnar table has several row groups" \ + "$(q "SELECT count(*) FROM pgcolumnar.row_group WHERE storage_id = pgcolumnar.get_storage_id('cc');")" "3" +before="$(q "SELECT pg_relation_filenode('cc');")" +for t in cc ch; do + q "ALTER TABLE $t ALTER COLUMN s TYPE text COLLATE \"$COLL\";" >/dev/null +done +check "premise: the collation change rewrote nothing" "$(q "SELECT pg_relation_filenode('cc');")" "$before" + +same() { # same SQL-with-%T -> "same", or both answers when they differ + local c h + c="$(q "${1//%T/cc}")"; h="$(q "${1//%T/ch}")" + [ "$c" = "$h" ] && echo same || echo "columnar [$c] heap [$h]" +} +check "after the collation change min and max are heap's" "$(same "SELECT min(s) || '|' || max(s) FROM %T;")" "same" +check "and a range below an upper-case bound returns heap's rows" \ + "$(same "SELECT count(*) || ':' || string_agg(DISTINCT s, ',' ORDER BY s) FROM %T WHERE s < 'B';")" "same" +check "and an equality on a value C sorts first returns heap's rows" \ + "$(same "SELECT count(*) FROM %T WHERE s = 'a0';")" "same" + +# The extent (#1325) stores a min and max per column too; column_extent() prints +# what the planner would read, which must not be the old collation's. +check "the column's extent is not the old collation's bound" \ + "$(q "SELECT coalesce((SELECT minimum || '|' || maximum FROM pgcolumnar.column_extent('cc', 's')), 'none') + IN ('none', (SELECT min(s) || '|' || max(s) FROM ch))::text;")" "true" + +# Rows written after the change are under the new collation; the answers stay heap's. +for t in cc ch; do + q "INSERT INTO $t SELECT g, 'A' || (g % 5) FROM generate_series(3001, 4200) g;" >/dev/null +done +check "rows written after the change join heap's answer" \ + "$(same "SELECT count(*) || ':' || min(s) || '|' || max(s) FROM %T WHERE s < 'B';")" "same" + +# Through a partitioned parent: the ALTER recurses to a columnar partition. +q "CREATE TABLE pp (k int, s text COLLATE \"C\") PARTITION BY RANGE (k); + CREATE TABLE pp1 PARTITION OF pp FOR VALUES FROM (0) TO (100000) USING pgcolumnar; + INSERT INTO pp SELECT k, s FROM ch WHERE k <= 3000; + ALTER TABLE pp ALTER COLUMN s TYPE text COLLATE \"$COLL\";" >/dev/null +check "a collation change made through a partitioned parent reaches the columnar partition" \ + "$(q "SELECT (SELECT count(*) FROM pp1 WHERE s < 'B') = (SELECT count(*) FROM ch WHERE k <= 3000 AND s < 'B');")" "t" + +# ---- what else trusted the old ordering ---------------------------------------- +# +# vacuum_sorted stores the rows in the column's order, and the scan can claim +# that order to the planner (#751), dropping the Sort an ORDER BY would need. +# Under the new collation the stored order is not the column's order. The claim +# already refuses every collatable column for exactly this reason, so this arm +# is green on main; it is the guard that keeps that refusal, dated by a mutation +# that removes it. +q "CREATE TABLE vs (k int, s text COLLATE \"C\") USING pgcolumnar; + CREATE TABLE vh (k int, s text COLLATE \"C\"); + INSERT INTO vs SELECT k, s FROM ch WHERE k <= 3000; + INSERT INTO vh SELECT k, s FROM ch WHERE k <= 3000; + SELECT pgcolumnar.vacuum_sorted('vs', 's');" >/dev/null +for t in vs vh; do + q "ALTER TABLE $t ALTER COLUMN s TYPE text COLLATE \"$COLL\";" >/dev/null +done +same2() { # same2 SQL-with-%T over vs and vh + local c h + c="$(q "${1//%T/vs}")"; h="$(q "${1//%T/vh}")" + [ "$c" = "$h" ] && echo same || echo "columnar [$c] heap [$h]" +} +check "a table sorted under the old collation orders ORDER BY ... LIMIT as heap does" \ + "$(same2 "SELECT string_agg(s, ',') FROM (SELECT s FROM %T ORDER BY s, k LIMIT 6) x;")" "same" + +# A projection sorted on the column: its rows and its zone maps are in the old order. +q "CREATE TABLE pj (k int, s text COLLATE \"C\") USING pgcolumnar; + INSERT INTO pj SELECT k, s FROM ch WHERE k <= 3000; + SELECT pgcolumnar.add_projection('pj', 'pj_by_s', columns => ARRAY['s', 'k'], sort_key => ARRAY['s']); + ALTER TABLE pj ALTER COLUMN s TYPE text COLLATE \"$COLL\";" >/dev/null +check "a projection sorted on the column answers a range as heap does" \ + "$(q "SELECT (SELECT count(*) FROM pj WHERE s < 'B') = (SELECT count(*) FROM ch WHERE k <= 3000 AND s < 'B');")" "t" + +pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index c6b5b6698..fb85826b7 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -148,6 +148,7 @@ behaviour, the source of that number is named. - [100. test_tier_partition.py: tier_partition moves a cold partition to Parquet, exactly once](#100-test_tier_partitionpy-tier_partition-moves-a-cold-partition-to-parquet-exactly-once) - [101. test_prepared_writes.py: a write prepared and finished in one session leaves the backend alive](#101-test_prepared_writespy-a-write-prepared-and-finished-in-one-session-leaves-the-backend-alive) - [102. test_column_extent.py: the per-column extent is a true bound, recomputed whenever groups go](#102-test_column_extentpy-the-per-column-extent-is-a-true-bound-recomputed-whenever-groups-go) +- [103. test_collation_change.py: a collation change without a rewrite leaves columnar answering as heap does](#103-test_collation_changepy-a-collation-change-without-a-rewrite-leaves-columnar-answering-as-heap-does) ## 1. How to read a test in here @@ -6730,3 +6731,19 @@ Written red first: the shell twin, against main, with the function absent (42883 | `test_a_shrink_is_recomputed_never_guessed` | after `expire`, `compact_rewrite` and VACUUM retire or rewrite groups, the extent equals the data left, and after `expire` it moved | | `test_no_complete_extent_where_the_zone_maps_cannot_give_one` | a column added after older groups, and a `point` column even after a recompute, get no complete extent | | `test_housekeeping_and_privilege` | fifty committing writers leave at most nine rows; TRUNCATE and DROP remove them; an ordinary role cannot read the catalog, nor the function without SELECT | + +## 103. test_collation_change.py: a collation change without a rewrite leaves columnar answering as heap does + +Port of `collation_change.sh` (#1327, found by @jdatcmd reviewing #1325). `ALTER COLUMN ... TYPE text COLLATE ` keeps the relfilenode, so no zone map is rewritten. The stored minima and maxima were the old collation's, and the reader compared them under the new one. Chunk-group pruning and the min/max fold then skipped rows that the new collation orders differently. + +THE ORACLE IS HEAP: the same rows, the same `ALTER` and the same query. A server with no en_US collation cannot show the bug, so the test calls `cannot_run`, which fails the run rather than passing it. + +THE FIX FORGETS THE BOUNDS RATHER THAN RECOMPUTING THEM. After an `ALTER` that changes a type, every columnar relation it reached that was not rewritten has its changed columns' zone-map minima and maxima nulled, in the base storage and in every projection storage. The column's extent rows are deleted. Counts and sums stay. A zone map with values and no bounds is already the state that a type with no ordering writes, and every reader treats it as "nothing is known". The ungrouped min/max fold was the exception: it skipped such a group as if it were all NULL, and returned NULL where heap returned a value. + +Independent of the shell suite: its own values (`c`, `Q`, `x`, `q` against the shell's `a`, `B`, `m`, `b`), its own geometry (four groups), and two list partitions where the shell has one range partition. The projection lists the column second, so its storage index differs from the shell's. The check names match. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_a_collation_change_without_a_rewrite` | after the change, columnar answers as heap does for: `min`/`max`, a range, an equality, rows written afterwards, a columnar partition reached through its parent, a `vacuum_sorted` table under `ORDER BY ... LIMIT`, and a projection sorted on the column. The extent is never the old collation's bound. The relfilenode is unchanged (premise) | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index ff49a70c2..644c80d22 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -594,4 +594,7 @@ guard_tests 419 # # cluster_tests 524 -> 525 for #1310's test_a_no_op_expire_reads_the_extent_not_every_zone_map, # by collection on this branch (stacked on #1304 option 5). -cluster_tests 525 +# +# cluster_tests 525 -> 526 for #1327's test_collation_change.py, one test, by collection +# on this branch rebased onto 4c72242 (#1326). +cluster_tests 526 diff --git a/test/pytest/test_collation_change.py b/test/pytest/test_collation_change.py new file mode 100644 index 000000000..7cfe87759 --- /dev/null +++ b/test/pytest/test_collation_change.py @@ -0,0 +1,105 @@ +"""A column's collation changed without a rewrite (#1327). + +ALTER COLUMN ... TYPE text COLLATE keeps the relfilenode, so no zone map +is rewritten. Before the fix the stored minima and maxima were the old +collation's, and the reader compared them under the new one: chunk-group pruning +and the min/max fold skipped rows the new collation orders differently. + +THE ORACLE IS HEAP: the same rows, the same ALTER, the same query. + +Independent of `test/collation_change.sh`: its own values, row counts and group +geometry; the check names match. A server with no en_US collation cannot show +the bug, and the test says so through `cannot_run`, which fails the run rather +than passing it. +""" + +GROUP = 1000 + + +def _one(conn, sql): + with conn.cursor() as cur: + cur.execute(sql) + row = cur.fetchone() + return None if row is None else row[0] + + +def _same(conn, sql, columnar, heap): + c = _one(conn, sql.replace("%T", columnar)) + h = _one(conn, sql.replace("%T", heap)) + return "same" if c == h else f"columnar [{c}] heap [{h}]" + + +def _value(g): + # Each value starts with one of four letters; under C the upper-case 'Q' sorts + # before every lower-case letter, under en_US it sorts among them. + return f"(ARRAY['c','Q','x','q'])[{g} % 4 + 1] || ({g} % 9)" + + +def test_a_collation_change_without_a_rewrite(pgc_conn, expect): + conn = pgc_conn + coll = _one(conn, "SELECT collname FROM pg_collation WHERE collname IN ('en_US.utf8', 'en_US.UTF-8', 'en_US')" + " AND collencoding IN (-1, pg_char_to_encoding(current_setting('server_encoding')))" + " ORDER BY collname LIMIT 1") + if coll is None: + expect.cannot_run("no en_US collation", name="the server has no en_US collation to change a C column to") + return + expect.text(_one(conn, f"SELECT ('c' < 'Q' COLLATE \"{coll}\")::text || '|' || ('c' < 'Q' COLLATE \"C\")::text"), + "true|false", "premise: the new collation orders a lower-case letter before an upper-case one that C puts first") + + with conn.cursor() as cur: + cur.execute('CREATE TABLE xc (k int, s text COLLATE "C") USING pgcolumnar') + cur.execute('CREATE TABLE xh (k int, s text COLLATE "C")') + cur.execute(f"SELECT pgcolumnar.set_options('xc', stripe_row_limit => {GROUP}, chunk_group_row_limit => {GROUP})") + for t in ("xc", "xh"): + cur.execute(f"INSERT INTO {t} SELECT g, {_value('g')} FROM generate_series(1, {4 * GROUP}) g") + expect.num(_one(conn, "SELECT count(*) FROM pgcolumnar.row_group WHERE storage_id = pgcolumnar.get_storage_id('xc')"), + 4, "premise: the columnar table has several row groups") + before = _one(conn, "SELECT pg_relation_filenode('xc')") + for t in ("xc", "xh"): + conn.execute(f'ALTER TABLE {t} ALTER COLUMN s TYPE text COLLATE "{coll}"') + expect.num(_one(conn, "SELECT pg_relation_filenode('xc')"), before, "premise: the collation change rewrote nothing") + + expect.text(_same(conn, "SELECT min(s) || '|' || max(s) FROM %T", "xc", "xh"), "same", + "after the collation change min and max are heap's") + expect.text(_same(conn, "SELECT count(*) || ':' || string_agg(DISTINCT s, ',' ORDER BY s) FROM %T WHERE s < 'Q'", + "xc", "xh"), "same", "and a range below an upper-case bound returns heap's rows") + expect.text(_same(conn, "SELECT count(*) FROM %T WHERE s = 'c3'", "xc", "xh"), "same", + "and an equality on a value C sorts first returns heap's rows") + expect.text(_one(conn, "SELECT coalesce((SELECT minimum || '|' || maximum FROM pgcolumnar.column_extent('xc', 's'))," + " 'none') IN ('none', (SELECT min(s) || '|' || max(s) FROM xh))::text"), "true", + "the column's extent is not the old collation's bound") + + for t in ("xc", "xh"): + conn.execute(f"INSERT INTO {t} SELECT g, 'P' || (g % 3) FROM generate_series({4 * GROUP + 1}, {5 * GROUP}) g") + expect.text(_same(conn, "SELECT count(*) || ':' || min(s) || '|' || max(s) FROM %T WHERE s < 'Q'", "xc", "xh"), + "same", "rows written after the change join heap's answer") + + with conn.cursor() as cur: + cur.execute('CREATE TABLE xp (k int, s text COLLATE "C") PARTITION BY LIST ((k % 2))') + cur.execute("CREATE TABLE xp0 PARTITION OF xp FOR VALUES IN (0) USING pgcolumnar") + cur.execute("CREATE TABLE xp1 PARTITION OF xp FOR VALUES IN (1) USING pgcolumnar") + cur.execute(f"INSERT INTO xp SELECT k, s FROM xh WHERE k <= {4 * GROUP}") + cur.execute(f'ALTER TABLE xp ALTER COLUMN s TYPE text COLLATE "{coll}"') + expect.text(_one(conn, f"SELECT ((SELECT count(*) FROM xp WHERE s < 'Q')" + f" = (SELECT count(*) FROM xh WHERE k <= {4 * GROUP} AND s < 'Q'))::text"), "true", + "a collation change made through a partitioned parent reaches the columnar partition") + + with conn.cursor() as cur: + cur.execute('CREATE TABLE xs (k int, s text COLLATE "C") USING pgcolumnar') + cur.execute('CREATE TABLE xsh (k int, s text COLLATE "C")') + for t in ("xs", "xsh"): + cur.execute(f"INSERT INTO {t} SELECT k, s FROM xh WHERE k <= {4 * GROUP}") + cur.execute("SELECT pgcolumnar.vacuum_sorted('xs', 's')") + for t in ("xs", "xsh"): + cur.execute(f'ALTER TABLE {t} ALTER COLUMN s TYPE text COLLATE "{coll}"') + expect.text(_same(conn, "SELECT string_agg(s, ',') FROM (SELECT s FROM %T ORDER BY s, k LIMIT 9) x", "xs", "xsh"), + "same", "a table sorted under the old collation orders ORDER BY ... LIMIT as heap does") + + with conn.cursor() as cur: + cur.execute('CREATE TABLE xj (k int, s text COLLATE "C") USING pgcolumnar') + cur.execute(f"INSERT INTO xj SELECT k, s FROM xh WHERE k <= {4 * GROUP}") + cur.execute("SELECT pgcolumnar.add_projection('xj', 'xj_by_s', columns => ARRAY['k', 's'], sort_key => ARRAY['s'])") + cur.execute(f'ALTER TABLE xj ALTER COLUMN s TYPE text COLLATE "{coll}"') + expect.text(_one(conn, f"SELECT ((SELECT count(*) FROM xj WHERE s < 'Q')" + f" = (SELECT count(*) FROM xh WHERE k <= {4 * GROUP} AND s < 'Q'))::text"), "true", + "a projection sorted on the column answers a range as heap does") diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index d243c5d0f..a42a0f71a 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -95,6 +95,7 @@ "catalog_delete_index", "catalog_plan_index", "catalog_scan_index", + "collation_change", "column_extent", "differential", "encode_post_codec", diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 9f0c52540..73d3fbdc1 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -67,6 +67,7 @@ SUITES=( catalog_natts catalog_plan_index catalog_scan_index + collation_change column_extent column_projection concurrency