Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 138 additions & 1 deletion src/columnar_tableam.c
Original file line number Diff line number Diff line change
Expand Up @@ -2468,6 +2468,138 @@ pgcolumnar_reject_set_am_to_columnar(AlterTableStmt *stmt)
}
}

/*
* pgcolumnar_reject_drop_projected_column
* Keep DROP COLUMN from leaving a projection pointed at a dropped attribute.
*
* Projection definitions are extension metadata, not pg_depend objects, so core
* cannot discover this dependency. Without this check DROP succeeds, the
* projection retains the old attnum, and its next sorted write asks typcache for
* the dropped pg_attribute's type OID zero. Dropping a stored column that is not
* a sort key is quieter but still wrong: the projection subsequently reads
* empty. Refuse either shape at the DDL boundary until projections can
* participate in DROP ... CASCADE.
*/
static void
pgcolumnar_reject_drop_projected_column(AlterTableStmt *stmt)
{
Oid relid;
List *relations;
ListCell *cmdCell;
bool hasDrop = false;

foreach(cmdCell, stmt->cmds)
{
AlterTableCmd *cmd = (AlterTableCmd *) lfirst(cmdCell);

if (cmd->subtype == AT_DropColumn)
{
hasDrop = true;
break;
}
}
if (!hasDrop)
return;

/*
* Check ownership before taking our lock or reading projection metadata.
* standard_ProcessUtility has not run yet, so without this ordering a caller
* with no rights on the table can distinguish projected from unprojected
* columns by SQLSTATE and retain a ShareUpdateExclusiveLock until transaction
* end.
*/
relid = RangeVarGetRelid(stmt->relation, NoLock, true);
if (!OidIsValid(relid))
return;
PgColumnarRequireTableOwnerByOid(relid);

/*
* add_projection() takes ShareLock, which conflicts with our
* ShareUpdateExclusiveLock. Retain this lock while core upgrades to
* AccessExclusiveLock for the ALTER, so a projection cannot be added between
* this check and DROP. Recheck ownership after locking because ALTER OWNER
* could have raced the unlocked check above.
*/
relid = RangeVarGetRelid(stmt->relation, ShareUpdateExclusiveLock, true);
if (!OidIsValid(relid))
return;
PgColumnarRequireTableOwnerByOid(relid);

if (stmt->relation->inh)
relations = find_all_inheritors(relid, ShareUpdateExclusiveLock, NULL);
else
relations = list_make1_oid(relid);

foreach(cmdCell, stmt->cmds)
{
AlterTableCmd *cmd = (AlterTableCmd *) lfirst(cmdCell);
ListCell *relCell;

if (cmd->subtype != AT_DropColumn || cmd->name == NULL)
continue;

foreach(relCell, relations)
{
Oid kid = lfirst_oid(relCell);
AttrNumber attnum;
Relation rel;
uint64 storageId;
List *projections;
ListCell *projectionCell;

if (get_rel_relkind(kid) != RELKIND_RELATION ||
!PgColumnarIsColumnarRelation(kid))
continue;

attnum = get_attnum(kid, cmd->name);
if (attnum == InvalidAttrNumber)
continue; /* DROP IF EXISTS of a missing column */

rel = table_open(kid, NoLock);
storageId = PgColumnarStorageId(rel);
projections = PgColumnarListProjections(storageId);

foreach(projectionCell, projections)
{
PgColumnarProjection *projection =
(PgColumnarProjection *) lfirst(projectionCell);
int i;

if (projection->projectionId == 0)
continue;

/*
* add_projection() requires every sort-key column to appear in
* columns, so this one loop covers both stored-only columns and
* the sort keys whose dropped type would break the writer.
*/
for (i = 0; i < projection->columnsLen; i++)
{
if (projection->columns[i] != attnum)
continue;

ereport(ERROR,
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
errmsg("cannot drop column \"%s\" because projection \"%s\" depends on it",
cmd->name, projection->name),
errdetail("Projection \"%s\" on table \"%s\" stores this column.",
projection->name,
RelationGetRelationName(rel)),
errhint("Drop the projection with pgcolumnar.drop_projection() before dropping the column.")));
}
}

/*
* Keep the lock until transaction end; standard_ProcessUtility will
* upgrade it for ALTER TABLE after this hook returns.
*/
table_close(rel, NoLock);
}
}

list_free(relations);
}

static void
pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString,
bool readOnlyTree, ProcessUtilityContext context,
Expand All @@ -2478,7 +2610,12 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString,

/* read-only inspection, so readOnlyTree needs no copy of the tree */
if (parsetree != NULL && IsA(parsetree, AlterTableStmt))
pgcolumnar_reject_set_am_to_columnar((AlterTableStmt *) parsetree);
{
AlterTableStmt *stmt = (AlterTableStmt *) parsetree;

pgcolumnar_reject_drop_projected_column(stmt);
pgcolumnar_reject_set_am_to_columnar(stmt);
}

if (prev_process_utility_hook)
prev_process_utility_hook(pstmt, queryString, readOnlyTree, context,
Expand Down
80 changes: 80 additions & 0 deletions test/projection_drop_column.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
#!/usr/bin/env bash
#
# pgColumnar: DROP COLUMN must not invalidate a materialized projection.
#
# Projections are extension metadata rather than pg_depend objects. PostgreSQL
# therefore accepted DROP COLUMN even when a projection stored that column.
# Dropping its sort key left the projection's attnum pointing at a dropped
# pg_attribute row; the next INSERT failed in lookup_type_cache with
# "type with OID 0 does not exist", making the table unwritable.
#
# Until projections can participate in DROP ... CASCADE, reject the dependent
# DROP at its public DDL boundary and tell the operator to drop the projection.
#
# Usage: test/projection_drop_column.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}"

sqlstate_as() {
local role="$1" sql="$2" out code
out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" \
-U "$role" -d "$PGC_DB" -Atq -v VERBOSITY=verbose \
-c "\\set VERBOSITY verbose" -c "$sql" 2>&1)"
code="$(printf '%s\n' "$out" |
sed -n 's/.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\):.*/\1/p' |
head -1)"
if [ -n "$code" ]; then printf '%s\n' "$code"; else printf '00000\n'; fi
}
sqlstate() { sqlstate_as postgres "$1"; }

psql_run "CREATE TABLE pdc (id int, payload text, sort_key int) USING pgcolumnar;"
psql_run "SELECT pgcolumnar.add_projection(
'pdc', 'by_sort', ARRAY['id','sort_key'], ARRAY['sort_key']);"
psql_run "INSERT INTO pdc
SELECT g, 'row-' || g, g % 10 FROM generate_series(1,1000) g;"

# The pre-statement dependency check must not tell a stranger which column the
# projection covers, or let that role take the lock retained by the owner path.
psql_run "CREATE ROLE pdc_nobody LOGIN;"
check "a non-owner learns nothing from a projected column" \
"$(sqlstate_as pdc_nobody 'ALTER TABLE pdc DROP COLUMN sort_key;')" "42501"
check "the same non-owner error is returned for an unprojected column" \
"$(sqlstate_as pdc_nobody 'ALTER TABLE pdc DROP COLUMN payload;')" "42501"

# Red before the fix: PostgreSQL accepts this (00000), and the next INSERT
# reaches the projection writer with a type OID of zero.
check "dropping a projected column is refused as a dependency" \
"$(sqlstate 'ALTER TABLE pdc DROP COLUMN sort_key;')" "2BP01"

psql_run "INSERT INTO pdc VALUES (1001, 'still-writable', 1);"
check "the rejected DDL leaves the table writable" \
"$(q 'SELECT count(*) FROM pdc')" "1001"
check "and the projection still receives the row" \
"$(q "SELECT count(*) FROM pgcolumnar.read_projection('pdc','by_sort')")" \
"1001"

# A column outside every projection remains ordinary DDL.
psql_run "ALTER TABLE pdc DROP COLUMN payload;"
psql_run "INSERT INTO pdc VALUES (1002, 2);"
check "dropping an unrelated column remains allowed" \
"$(q 'SELECT count(*) FROM pdc')" "1002"

# A partitioned parent has no storage itself, but DROP COLUMN recurses into its
# columnar partitions. The dependency check must walk the same hierarchy.
psql_run "CREATE TABLE pdc_parent (id int, sort_key int)
PARTITION BY RANGE (id);"
psql_run "CREATE TABLE pdc_child PARTITION OF pdc_parent
FOR VALUES FROM (0) TO (100) USING pgcolumnar;"
psql_run "SELECT pgcolumnar.add_projection(
'pdc_child', 'child_sort', ARRAY['id','sort_key'], ARRAY['sort_key']);"
psql_run "INSERT INTO pdc_parent VALUES (1, 1);"
check "a parent DROP sees projections on columnar partitions" \
"$(sqlstate 'ALTER TABLE pdc_parent DROP COLUMN sort_key;')" "2BP01"
psql_run "INSERT INTO pdc_parent VALUES (2, 2);"
check "the rejected parent DDL leaves its partition writable" \
"$(q 'SELECT count(*) FROM pdc_parent')" "2"

pgc_summary
1 change: 1 addition & 0 deletions test/run_all_versions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ SUITES=(
phase6
planner_choice_quality
preimage_rewrite
projection_drop_column
projection_privilege
projection_update
projections
Expand Down
Loading