From 697a4106ae0c8e71ef8a1847e57e8b472edd9cb3 Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:43:53 +0000 Subject: [PATCH 1/2] fix: carry projection declarations through column renames Co-authored-by: Cursor --- src/columnar_metadata.c | 112 ++++++++++++++++++++++++++++++ src/columnar_metadata.h | 4 ++ src/columnar_tableam.c | 3 + test/projection_rename_restore.sh | 56 +++++++++++++++ test/run_all_versions.sh | 1 + 5 files changed, 176 insertions(+) create mode 100755 test/projection_rename_restore.sh diff --git a/src/columnar_metadata.c b/src/columnar_metadata.c index 856666a5..f6e7f6ec 100644 --- a/src/columnar_metadata.c +++ b/src/columnar_metadata.c @@ -3540,6 +3540,118 @@ PgColumnarRecordProjectionDeclaration(Oid relid, const char *name, CommandCounterIncrement(); } +/* + * rename_projection_declaration_array + * Replace oldName in one text[] declaration field. + */ +static Datum +rename_projection_declaration_array(Datum value, const char *oldName, + const char *newName, bool *changed) +{ + ArrayType *arr = DatumGetArrayTypeP(value); + Datum *elems; + bool *nulls; + int nelems; + int i; + + deconstruct_array(arr, TEXTOID, -1, false, TYPALIGN_INT, + &elems, &nulls, &nelems); + + for (i = 0; i < nelems; i++) + { + char *name; + + if (nulls[i]) + continue; + + name = TextDatumGetCString(elems[i]); + if (strcmp(name, oldName) == 0) + { + elems[i] = CStringGetTextDatum(newName); + *changed = true; + } + } + + return PointerGetDatum(construct_array(elems, nelems, TEXTOID, + -1, false, TYPALIGN_INT)); +} + +/* + * PgColumnarRenameProjectionDeclarationColumn + * Carry ALTER TABLE ... RENAME COLUMN through dumpable declarations. + * + * The materialized projection stores attnums, so it follows a rename without + * any catalog change. The declaration deliberately stores names because + * pg_dump restores a table with newly assigned attnums. Leaving its old name + * behind therefore breaks rebuild_projections() after restore even though the + * live projection continued to work before the backup. + */ +void +PgColumnarRenameProjectionDeclarationColumn(Oid relid, const char *oldName, + const char *newName) +{ + Relation rel; + TupleDesc tupdesc; + ScanKeyData key[1]; + SysScanDesc scan; + HeapTuple tuple; + + rel = open_columnar_table("projection_declaration", RowExclusiveLock); + tupdesc = RelationGetDescr(rel); + ScanKeyInit(&key[0], Anum_projection_declaration_rel, BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(relid)); + scan = systable_beginscan(rel, InvalidOid, false, NULL, 1, key); + + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + Datum values[Natts_projection_declaration]; + bool nulls[Natts_projection_declaration]; + bool replace[Natts_projection_declaration]; + bool changed = false; + bool isnull; + Datum d; + HeapTuple newTuple; + + memset(values, 0, sizeof(values)); + memset(nulls, false, sizeof(nulls)); + memset(replace, false, sizeof(replace)); + + d = heap_getattr(tuple, Anum_projection_declaration_columns, + tupdesc, &isnull); + if (!isnull) + { + values[Anum_projection_declaration_columns - 1] = + rename_projection_declaration_array(d, oldName, newName, + &changed); + replace[Anum_projection_declaration_columns - 1] = changed; + } + + d = heap_getattr(tuple, Anum_projection_declaration_sort_key, + tupdesc, &isnull); + if (!isnull) + { + bool sortChanged = false; + + values[Anum_projection_declaration_sort_key - 1] = + rename_projection_declaration_array(d, oldName, newName, + &sortChanged); + replace[Anum_projection_declaration_sort_key - 1] = sortChanged; + changed = changed || sortChanged; + } + + if (!changed) + continue; + + newTuple = heap_modify_tuple(tuple, tupdesc, values, nulls, replace); + CatalogTupleUpdate(rel, &newTuple->t_self, newTuple); + heap_freetuple(newTuple); + } + + systable_endscan(scan); + table_close(rel, RowExclusiveLock); + CommandCounterIncrement(); +} + /* * PgColumnarDeleteProjectionDeclaration * Forget the declaration for one projection. Called when it is dropped, and diff --git a/src/columnar_metadata.h b/src/columnar_metadata.h index 739bf4e0..ccf59332 100644 --- a/src/columnar_metadata.h +++ b/src/columnar_metadata.h @@ -90,6 +90,10 @@ extern void PgColumnarRecordProjectionDeclaration(Oid relid, const char *name, ArrayType *columns, ArrayType *sortKey); +extern void PgColumnarRenameProjectionDeclarationColumn(Oid relid, + const char *oldName, + const char *newName); + extern void PgColumnarDeleteProjectionDeclaration(Oid relid, const char *name); extern void PgColumnarDeleteProjectionDeclarationsForRel(Oid relid); diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 145c55db..514ffe63 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -2572,6 +2572,9 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString, */ PgColumnarRenameDeclaredSortByColumn(kid, rs->subname, rs->newname); + PgColumnarRenameProjectionDeclarationColumn(kid, + rs->subname, + rs->newname); } list_free(kin); } diff --git a/test/projection_rename_restore.sh b/test/projection_rename_restore.sh new file mode 100755 index 00000000..d454e7a2 --- /dev/null +++ b/test/projection_rename_restore.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# +# pgColumnar: a projection declaration must follow a column rename. +# +# The materialized projection records attnums and survives a rename, but the +# declaration carried by pg_dump records names. The rename hook maintained the +# table sort_by option and physical ordering mark, but not projection +# declarations. A backup therefore restored the old column name and +# rebuild_projections() failed instead of recreating the projection. +# +# This test stays at the public boundary: create, rename, pg_dump, restore, +# rebuild, and read. It does not infer correctness from internal catalog rows. +# +# Usage: test/projection_rename_restore.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}" + +DUMP="$PGC_WORKDIR/projection-rename.sql" +RESTORE_DB="${PGC_DB}_renamed" +run() { env PATH="$PGC_BINDIR:$PATH" "$@"; } +on() { run psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$1" -Atq -c "$2"; } + +psql_run "CREATE TABLE prr (id int, payload text, sort_key int) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.add_projection( + 'prr', 'by_sort', ARRAY['id','sort_key'], ARRAY['sort_key']);" +psql_run "INSERT INTO prr + SELECT g, 'row-' || g, (g * 7) % 100 FROM generate_series(1,2000) g;" +psql_run "ALTER TABLE prr RENAME COLUMN sort_key TO renamed_key;" + +check "the live projection still reads after the rename" \ + "$(q "SELECT count(*) FROM pgcolumnar.read_projection('prr','by_sort')")" \ + "2000" + +run pg_dump -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \ + -f "$DUMP" +check "pg_dump succeeds" "$?" "0" + +on postgres "DROP DATABASE IF EXISTS $RESTORE_DB;" >/dev/null 2>&1 +on postgres "CREATE DATABASE $RESTORE_DB;" >/dev/null +run psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$RESTORE_DB" \ + -v ON_ERROR_STOP=1 -q -f "$DUMP" >/dev/null 2>&1 +check "restore succeeds" "$?" "0" + +# Red before the fix: this errors because the declaration still names sort_key. +rebuilt="$(on "$RESTORE_DB" "SELECT pgcolumnar.rebuild_projections('prr');" 2>&1)" +check "the renamed projection rebuilds after restore" "$rebuilt" "1" +check "the rebuilt projection contains every restored row" \ + "$(on "$RESTORE_DB" \ + "SELECT count(*) FROM pgcolumnar.read_projection('prr','by_sort');")" \ + "2000" + +on postgres "DROP DATABASE $RESTORE_DB;" >/dev/null +pgc_summary diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 39ecc7df..b8eef90a 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -241,6 +241,7 @@ SUITES=( planner_choice_quality preimage_rewrite projection_privilege + projection_rename_restore projection_update projections pushdown_report From 2ba0f9104470ff59a9d2075639aa7af126fd1035 Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:52:40 +0000 Subject: [PATCH 2/2] test: cover renamed projection descendants Co-authored-by: Cursor --- src/columnar_metadata.c | 18 ++++++++++++++++-- test/projection_rename_restore.sh | 30 ++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/columnar_metadata.c b/src/columnar_metadata.c index f6e7f6ec..f4ba835b 100644 --- a/src/columnar_metadata.c +++ b/src/columnar_metadata.c @@ -3572,8 +3572,17 @@ rename_projection_declaration_array(Datum value, const char *oldName, } } - return PointerGetDatum(construct_array(elems, nelems, TEXTOID, - -1, false, TYPALIGN_INT)); + /* + * resolve_columns() rejects NULL elements before add_projection() records a + * declaration. Preserve the bitmap anyway, so this helper remains safe if a + * catalog row created outside that path contains one. + */ + return PointerGetDatum(construct_md_array(elems, nulls, + ARR_NDIM(arr), + ARR_DIMS(arr), + ARR_LBOUND(arr), + TEXTOID, -1, false, + TYPALIGN_INT)); } /* @@ -3639,6 +3648,11 @@ PgColumnarRenameProjectionDeclarationColumn(Oid relid, const char *oldName, changed = changed || sortChanged; } + /* + * This is an unindexed catalog seqscan updated in place. If it encounters + * its updated tuple again, the old name is absent and this idempotent arm + * prevents a second CatalogTupleUpdate. + */ if (!changed) continue; diff --git a/test/projection_rename_restore.sh b/test/projection_rename_restore.sh index d454e7a2..6d742255 100755 --- a/test/projection_rename_restore.sh +++ b/test/projection_rename_restore.sh @@ -34,15 +34,30 @@ check "the live projection still reads after the rename" \ "$(q "SELECT count(*) FROM pgcolumnar.read_projection('prr','by_sort')")" \ "2000" +# A rename on a non-columnar partitioned parent reaches a separate columnar +# relation with its own declaration. This protects the inheritor walk: using +# the named parent's oid in place of each descendant leaves this arm stale. +psql_run "CREATE TABLE prr_parent (id int, sort_key int) + PARTITION BY RANGE (id);" +psql_run "CREATE TABLE prr_child PARTITION OF prr_parent + FOR VALUES FROM (0) TO (100) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.add_projection( + 'prr_child', 'child_sort', ARRAY['id','sort_key'], ARRAY['sort_key']);" +psql_run "INSERT INTO prr_parent SELECT g, g % 10 FROM generate_series(1,99) g;" +psql_run "ALTER TABLE prr_parent RENAME COLUMN sort_key TO renamed_key;" +check "the child projection still reads after a parent rename" \ + "$(q "SELECT count(*) FROM pgcolumnar.read_projection( + 'prr_child','child_sort')")" "99" + run pg_dump -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \ - -f "$DUMP" -check "pg_dump succeeds" "$?" "0" + -f "$DUMP"; dump_rc=$? +check "pg_dump succeeds" "$dump_rc" "0" on postgres "DROP DATABASE IF EXISTS $RESTORE_DB;" >/dev/null 2>&1 on postgres "CREATE DATABASE $RESTORE_DB;" >/dev/null run psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$RESTORE_DB" \ - -v ON_ERROR_STOP=1 -q -f "$DUMP" >/dev/null 2>&1 -check "restore succeeds" "$?" "0" + -v ON_ERROR_STOP=1 -q -f "$DUMP" >/dev/null 2>&1; restore_rc=$? +check "restore succeeds" "$restore_rc" "0" # Red before the fix: this errors because the declaration still names sort_key. rebuilt="$(on "$RESTORE_DB" "SELECT pgcolumnar.rebuild_projections('prr');" 2>&1)" @@ -51,6 +66,13 @@ check "the rebuilt projection contains every restored row" \ "$(on "$RESTORE_DB" \ "SELECT count(*) FROM pgcolumnar.read_projection('prr','by_sort');")" \ "2000" +check "the descendant declaration follows the parent rename" \ + "$(on "$RESTORE_DB" \ + "SELECT pgcolumnar.rebuild_projections('prr_child');")" "1" +check "the rebuilt child projection contains every restored row" \ + "$(on "$RESTORE_DB" \ + "SELECT count(*) FROM pgcolumnar.read_projection( + 'prr_child','child_sort');")" "99" on postgres "DROP DATABASE $RESTORE_DB;" >/dev/null pgc_summary