public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 2/2] ALTER TABLE ... DETACH CONCURRENTLY 17+ messages / 8 participants [nested] [flat]
* [PATCH v5 2/2] ALTER TABLE ... DETACH CONCURRENTLY @ 2020-08-03 23:21 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 17+ messages in thread From: Alvaro Herrera @ 2020-08-03 23:21 UTC (permalink / raw) --- doc/src/sgml/catalogs.sgml | 10 + doc/src/sgml/ref/alter_table.sgml | 25 +- src/backend/catalog/heap.c | 13 +- src/backend/catalog/index.c | 4 +- src/backend/catalog/partition.c | 28 +- src/backend/catalog/pg_inherits.c | 51 +- src/backend/commands/indexcmds.c | 7 +- src/backend/commands/tablecmds.c | 568 ++++++++++++++---- src/backend/commands/trigger.c | 5 +- src/backend/executor/execPartition.c | 29 +- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/optimizer/util/plancat.c | 11 +- src/backend/parser/gram.y | 23 +- src/backend/partitioning/partbounds.c | 6 +- src/backend/partitioning/partdesc.c | 21 +- src/backend/tcop/utility.c | 19 + src/backend/utils/cache/partcache.c | 15 +- src/bin/psql/describe.c | 40 +- src/include/catalog/pg_inherits.h | 7 +- src/include/nodes/parsenodes.h | 2 + src/include/parser/kwlist.h | 1 + src/include/partitioning/partdesc.h | 5 +- src/include/utils/snapmgr.h | 1 + .../detach-partition-concurrently-1.out | 301 ++++++++++ .../detach-partition-concurrently-2.out | 66 ++ src/test/isolation/isolation_schedule | 2 + .../detach-partition-concurrently-1.spec | 90 +++ .../detach-partition-concurrently-2.spec | 41 ++ src/test/modules/delay_execution/Makefile | 3 +- .../expected/partition-removal-1.out | 79 +++ .../specs/partition-removal-1.spec | 45 ++ 32 files changed, 1347 insertions(+), 173 deletions(-) create mode 100644 src/test/isolation/expected/detach-partition-concurrently-1.out create mode 100644 src/test/isolation/expected/detach-partition-concurrently-2.out create mode 100644 src/test/isolation/specs/detach-partition-concurrently-1.spec create mode 100644 src/test/isolation/specs/detach-partition-concurrently-2.spec create mode 100644 src/test/modules/delay_execution/expected/partition-removal-1.out create mode 100644 src/test/modules/delay_execution/specs/partition-removal-1.spec diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 3a2266526c..759dd8da5b 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -4482,6 +4482,16 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l when using declarative partitioning. </para></entry> </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>inhdetached</structfield> <type>bool</type> + </para> + <para> + Set to true for a partition that is in the process of being detached; + false otherwise. + </para></entry> + </row> </tbody> </tgroup> </table> diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index c25ef5abd6..09673dcf1c 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -36,7 +36,9 @@ ALTER TABLE ALL IN TABLESPACE <replaceable class="parameter">name</replaceable> ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ATTACH PARTITION <replaceable class="parameter">partition_name</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> - DETACH PARTITION <replaceable class="parameter">partition_name</replaceable> + DETACH PARTITION <replaceable class="parameter">partition_name</replaceable> [ CONCURRENTLY ] +ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> + DETACH PARTITION <replaceable class="parameter">partition_name</replaceable> FINALIZE <phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase> @@ -938,7 +940,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM </varlistentry> <varlistentry> - <term><literal>DETACH PARTITION</literal> <replaceable class="parameter">partition_name</replaceable></term> + <term><literal>DETACH PARTITION</literal> <replaceable class="parameter">partition_name</replaceable> [ { CONCURRENTLY | FINALIZE } ]</term> + <listitem> <para> This form detaches the specified partition of the target table. The detached @@ -947,6 +950,24 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM attached to the target table's indexes are detached. Any triggers that were created as clones of those in the target table are removed. </para> + <para> + If <literal>CONCURRENTLY</literal> is specified, this process runs in two + transactions in order to avoid blocking other sessions that might be accessing + the partitioned table. During the first transaction, a + <literal>SHARE UPDATE EXCLUSIVE</literal> lock is taken on both parent table and + partition, and its partition is marked detached; at that point, the transaction + is committed and all transactions using the partitioned table are waited for. + Once all those transactions are gone, the second stage acquires + <literal>ACCESS EXCLUSIVE</literal> on the partition, and the detach process + completes. + <literal>CONCURRENTLY</literal> is not allowed if the + partitioned table contains a default partition. + </para> + <para> + If <literal>FINALIZE</literal> is specified, a previous + <literal>DETACH CONCURRENTLY</literal> invocation that was cancelled or + interrupted is completed. + </para> </listitem> </varlistentry> diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 21f2240ade..73e26fa400 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2546,10 +2546,12 @@ StoreConstraints(Relation rel, List *cooked_constraints, bool is_internal) * Returns a list of CookedConstraint nodes that shows the cooked form of * the default and constraint expressions added to the relation. * - * NB: caller should have opened rel with AccessExclusiveLock, and should - * hold that lock till end of transaction. Also, we assume the caller has - * done a CommandCounterIncrement if necessary to make the relation's catalog - * tuples visible. + * NB: caller should have opened rel with some self-conflicting lock mode, + * and should hold that lock till end of transaction; for normal cases that'll + * be AccessExclusiveLock, but if caller knows that the constraint is already + * enforced by some other means, it can be ShareUpdateExclusiveLock. Also, we + * assume the caller has done a CommandCounterIncrement if necessary to make + * the relation's catalog tuples visible. */ List * AddRelationNewConstraints(Relation rel, @@ -3818,7 +3820,8 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound) * relcache entry for that partition every time a partition is added or * removed. */ - defaultPartOid = get_default_oid_from_partdesc(RelationGetPartitionDesc(parent)); + defaultPartOid = + get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, false)); if (OidIsValid(defaultPartOid)) CacheInvalidateRelcacheByRelid(defaultPartOid); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index cffbc0ac38..0dc53b25b8 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1831,7 +1831,7 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName) List *ancestors = get_partition_ancestors(oldIndexId); Oid parentIndexRelid = linitial_oid(ancestors); - DeleteInheritsTuple(oldIndexId, parentIndexRelid); + DeleteInheritsTuple(oldIndexId, parentIndexRelid, false, NULL); StoreSingleInheritance(newIndexId, parentIndexRelid, 1); list_free(ancestors); @@ -2425,7 +2425,7 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) /* * fix INHERITS relation */ - DeleteInheritsTuple(indexId, InvalidOid); + DeleteInheritsTuple(indexId, InvalidOid, false, NULL); /* * We are presently too lazy to attempt to compute the new correct value diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index af7754d6ab..4327a0b150 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -32,7 +32,7 @@ #include "utils/rel.h" #include "utils/syscache.h" -static Oid get_partition_parent_worker(Relation inhRel, Oid relid); +static Oid get_partition_parent_worker(Relation inhRel, Oid relid, bool *detached); static void get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors); @@ -42,6 +42,9 @@ static void get_partition_ancestors_worker(Relation inhRel, Oid relid, * * Returns inheritance parent of a partition by scanning pg_inherits * + * If the partition is in the process of being detached, InvalidOid is + * returned and no error is thrown. + * * Note: Because this function assumes that the relation whose OID is passed * as an argument will have precisely one parent, it should only be called * when it is known that the relation is a partition. @@ -51,12 +54,15 @@ get_partition_parent(Oid relid) { Relation catalogRelation; Oid result; + bool detached; catalogRelation = table_open(InheritsRelationId, AccessShareLock); - result = get_partition_parent_worker(catalogRelation, relid); + result = get_partition_parent_worker(catalogRelation, relid, &detached); - if (!OidIsValid(result)) + if (detached) + result = InvalidOid; + else if (!OidIsValid(result)) elog(ERROR, "could not find tuple for parent of relation %u", relid); table_close(catalogRelation, AccessShareLock); @@ -70,13 +76,15 @@ get_partition_parent(Oid relid) * given relation */ static Oid -get_partition_parent_worker(Relation inhRel, Oid relid) +get_partition_parent_worker(Relation inhRel, Oid relid, bool *detached) { SysScanDesc scan; ScanKeyData key[2]; Oid result = InvalidOid; HeapTuple tuple; + *detached = false; + ScanKeyInit(&key[0], Anum_pg_inherits_inhrelid, BTEqualStrategyNumber, F_OIDEQ, @@ -93,7 +101,14 @@ get_partition_parent_worker(Relation inhRel, Oid relid) { Form_pg_inherits form = (Form_pg_inherits) GETSTRUCT(tuple); - result = form->inhparent; + /* A partition being detached has no parent */ + if (form->inhdetached) + { + *detached = true; + result = InvalidOid; + } + else + result = form->inhparent; } systable_endscan(scan); @@ -134,9 +149,10 @@ static void get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors) { Oid parentOid; + bool detached; /* Recursion ends at the topmost level, ie., when there's no parent */ - parentOid = get_partition_parent_worker(inhRel, relid); + parentOid = get_partition_parent_worker(inhRel, relid, &detached); if (parentOid == InvalidOid) return; diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index f3783961b7..3aec885288 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -52,7 +52,8 @@ typedef struct SeenRelsEntry * against possible DROPs of child relations. */ List * -find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) +find_inheritance_children(Oid parentrelId, bool include_detached, + LOCKMODE lockmode) { List *list = NIL; Relation relation; @@ -91,7 +92,13 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) while ((inheritsTuple = systable_getnext(scan)) != NULL) { + /* skip detached at caller's request */ + if (((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhdetached && + !include_detached) + continue; + inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid; + if (numoids >= maxoids) { maxoids *= 2; @@ -160,6 +167,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) * given rel; caller should already have locked it). If lockmode is NoLock * then no locks are acquired, but caller must beware of race conditions * against possible DROPs of child relations. + * + * NB - No current callers of this routine are interested in children being + * concurrently detached, so there's no provision to include them. */ List * find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) @@ -199,7 +209,8 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) ListCell *lc; /* Get the direct children of this rel */ - currentchildren = find_inheritance_children(currentrel, lockmode); + currentchildren = find_inheritance_children(currentrel, false, + lockmode); /* * Add to the queue only those children not already seen. This avoids @@ -428,6 +439,7 @@ StoreSingleInheritance(Oid relationId, Oid parentOid, int32 seqNumber) values[Anum_pg_inherits_inhrelid - 1] = ObjectIdGetDatum(relationId); values[Anum_pg_inherits_inhparent - 1] = ObjectIdGetDatum(parentOid); values[Anum_pg_inherits_inhseqno - 1] = Int32GetDatum(seqNumber); + values[Anum_pg_inherits_inhdetached - 1] = BoolGetDatum(false); memset(nulls, 0, sizeof(nulls)); @@ -447,10 +459,21 @@ StoreSingleInheritance(Oid relationId, Oid parentOid, int32 seqNumber) * as InvalidOid, in which case all tuples matching inhrelid are deleted; * otherwise only delete tuples with the specified inhparent. * + * 'detached' is the expected state of the inhdetached flag. If the catalog + * row does not match that state, an error is raised. When used on + * partitions, the partition name must be passed, for possible error messages. + * + * If allow_detached is passed false, then an error is raised if the child is + * already marked detached. A name must be supplied in that case, for the + * error message. This should only be used with table partitions. + * XXX the name bit is pretty weird and problematic for non-partition cases; + * what if the flag check fails? + * * Returns whether at least one row was deleted. */ bool -DeleteInheritsTuple(Oid inhrelid, Oid inhparent) +DeleteInheritsTuple(Oid inhrelid, Oid inhparent, bool detached, + const char *childname) { bool found = false; Relation catalogRelation; @@ -477,6 +500,28 @@ DeleteInheritsTuple(Oid inhrelid, Oid inhparent) parent = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhparent; if (!OidIsValid(inhparent) || parent == inhparent) { + bool inhdetached; + + inhdetached = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhdetached; + + /* + * Raise error depending on state. This should only happen for + * partitions, but we have no way to cross-check. + */ + if (inhdetached && !detached) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot detach partition \"%s\"", + childname), + errdetail("The partition is being detached concurrently or has an unfinished detach."), + errhint("Use ALTER TABLE ... DETACH PARTITION ... FINALIZE to complete the pending detach operation"))); + if (!inhdetached && detached) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot complete detaching partition \"%s\"", + childname), + errdetail("There's no pending concurrent detach."))); + CatalogTupleDelete(catalogRelation, &inheritsTuple->t_self); found = true; } diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 992f4813b4..ee4af9ea32 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -401,7 +401,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts) * GetCurrentVirtualXIDs. If, during any iteration, a particular vxid * doesn't show up in the output, we know we can forget about it. */ -static void +void WaitForOlderSnapshots(TransactionId limitXmin, bool progress) { int n_old_snapshots; @@ -1114,7 +1114,7 @@ DefineIndex(Oid relationId, */ if (partitioned && stmt->relation && !stmt->relation->inh) { - PartitionDesc pd = RelationGetPartitionDesc(rel); + PartitionDesc pd = RelationGetPartitionDesc(rel, false); if (pd->nparts != 0) flags |= INDEX_CREATE_INVALID; @@ -1171,7 +1171,7 @@ DefineIndex(Oid relationId, * * If we're called internally (no stmt->relation), recurse always. */ - partdesc = RelationGetPartitionDesc(rel); + partdesc = RelationGetPartitionDesc(rel, false); if ((!stmt->relation || stmt->relation->inh) && partdesc->nparts > 0) { int nparts = partdesc->nparts; @@ -3849,6 +3849,7 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid) values[Anum_pg_inherits_inhparent - 1] = ObjectIdGetDatum(parentOid); values[Anum_pg_inherits_inhseqno - 1] = Int32GetDatum(1); + values[Anum_pg_inherits_inhdetached - 1] = BoolGetDatum(false); memset(isnull, false, sizeof(isnull)); tuple = heap_form_tuple(RelationGetDescr(pg_inherits), diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a8528a3423..d7b9c63e5f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -542,7 +542,8 @@ static PartitionSpec *transformPartitionSpec(Relation rel, PartitionSpec *partsp static void ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNumber *partattrs, List **partexprs, Oid *partopclass, Oid *partcollation, char strategy); static void CreateInheritance(Relation child_rel, Relation parent_rel); -static void RemoveInheritance(Relation child_rel, Relation parent_rel); +static void RemoveInheritance(Relation child_rel, Relation parent_rel, + bool allow_detached); static ObjectAddress ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, AlterTableUtilityContext *context); @@ -551,8 +552,14 @@ static void QueuePartitionConstraintValidation(List **wqueue, Relation scanrel, List *partConstraint, bool validate_default); static void CloneRowTriggersToPartition(Relation parent, Relation partition); +static void DetachAddConstraintIfNeeded(List **wqueue, Relation partRel); static void DropClonedTriggersFromPartition(Oid partitionId); -static ObjectAddress ATExecDetachPartition(Relation rel, RangeVar *name); +static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, + Relation rel, RangeVar *name, + bool concurrent); +static void DetachPartitionFinalize(Relation rel, Relation partRel, + bool concurrent, Oid defaultPartOid, bool wait); +static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name); static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation rel, RangeVar *name); static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl); @@ -987,7 +994,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * lock the partition so as to avoid a deadlock. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(parent)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, + false)); if (OidIsValid(defaultPartOid)) defaultRel = table_open(defaultPartOid, AccessExclusiveLock); @@ -3158,7 +3166,7 @@ renameatt_internal(Oid myrelid, * expected_parents will only be 0 if we are not already recursing. */ if (expected_parents == 0 && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, false, NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited column \"%s\" must be renamed in child tables too", @@ -3357,7 +3365,7 @@ rename_constraint_internal(Oid myrelid, else { if (expected_parents == 0 && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, false, NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited constraint \"%s\" must be renamed in child tables too", @@ -3976,7 +3984,14 @@ AlterTableGetLockLevel(List *cmds) break; case AT_DetachPartition: - cmd_lockmode = AccessExclusiveLock; + if (((PartitionCmd *) cmd->def)->concurrent) + cmd_lockmode = ShareUpdateExclusiveLock; + else + cmd_lockmode = AccessExclusiveLock; + break; + + case AT_DetachPartitionFinalize: + cmd_lockmode = ShareUpdateExclusiveLock; break; case AT_CheckNotNull: @@ -4367,6 +4382,11 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, /* No command-specific prep needed */ pass = AT_PASS_MISC; break; + case AT_DetachPartitionFinalize: + ATSimplePermissions(rel, ATT_TABLE); + /* No command-specific prep needed */ + pass = AT_PASS_MISC; + break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); @@ -4752,7 +4772,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Assert(cmd != NULL); /* ATPrepCmd ensures it must be a table */ Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); - ATExecDetachPartition(rel, ((PartitionCmd *) cmd->def)->name); + ATExecDetachPartition(wqueue, tab, rel, + ((PartitionCmd *) cmd->def)->name, + ((PartitionCmd *) cmd->def)->concurrent); + break; + case AT_DetachPartitionFinalize: + ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); break; case AT_AlterCollationRefreshVersion: /* ATPrepCmd ensured it must be an index */ @@ -6184,7 +6209,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, */ if (colDef->identity && recurse && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, false, NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot recursively add identity column to table that has child tables"))); @@ -6417,7 +6442,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ - children = find_inheritance_children(RelationGetRelid(rel), lockmode); + children = + find_inheritance_children(RelationGetRelid(rel), false, lockmode); /* * If we are told not to recurse, there had better not be any child @@ -6571,7 +6597,7 @@ ATPrepDropNotNull(Relation rel, bool recurse, bool recursing) */ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(rel); + PartitionDesc partdesc = RelationGetPartitionDesc(rel, false); Assert(partdesc != NULL); if (partdesc->nparts > 0 && !recurse && !recursing) @@ -6671,17 +6697,22 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) if (rel->rd_rel->relispartition) { Oid parentId = get_partition_parent(RelationGetRelid(rel)); - Relation parent = table_open(parentId, AccessShareLock); - TupleDesc tupDesc = RelationGetDescr(parent); + Relation parent; + TupleDesc tupDesc; AttrNumber parent_attnum; - parent_attnum = get_attnum(parentId, colName); - if (TupleDescAttr(tupDesc, parent_attnum - 1)->attnotnull) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("column \"%s\" is marked NOT NULL in parent table", - colName))); - table_close(parent, AccessShareLock); + if (parentId != InvalidOid) + { + parent = table_open(parentId, AccessShareLock); + tupDesc = RelationGetDescr(parent); + parent_attnum = get_attnum(parentId, colName); + if (TupleDescAttr(tupDesc, parent_attnum - 1)->attnotnull) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("column \"%s\" is marked NOT NULL in parent table", + colName))); + table_close(parent, AccessShareLock); + } } /* @@ -7280,7 +7311,7 @@ ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recurse, bool recurs * resulting state can be properly dumped and restored. */ if (!recurse && - find_inheritance_children(RelationGetRelid(rel), lockmode)) + find_inheritance_children(RelationGetRelid(rel), false, lockmode)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("ALTER TABLE / DROP EXPRESSION must be applied to child tables too"))); @@ -7863,7 +7894,8 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ - children = find_inheritance_children(RelationGetRelid(rel), lockmode); + children = + find_inheritance_children(RelationGetRelid(rel), false, lockmode); if (children) { @@ -8327,7 +8359,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ - children = find_inheritance_children(RelationGetRelid(rel), lockmode); + children = + find_inheritance_children(RelationGetRelid(rel), false, lockmode); /* * Check if ONLY was specified with ALTER TABLE. If so, allow the @@ -8942,7 +8975,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, */ if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc pd = RelationGetPartitionDesc(pkrel); + PartitionDesc pd = RelationGetPartitionDesc(pkrel, false); for (int i = 0; i < pd->nparts; i++) { @@ -9076,7 +9109,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, } else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc pd = RelationGetPartitionDesc(rel); + PartitionDesc pd = RelationGetPartitionDesc(rel, false); /* * Recurse to take appropriate action on each partition; either we @@ -10860,7 +10893,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, * use find_all_inheritors to do it in one pass. */ if (!is_no_inherit_constraint) - children = find_inheritance_children(RelationGetRelid(rel), lockmode); + children = + find_inheritance_children(RelationGetRelid(rel), false, lockmode); else children = NIL; @@ -11244,7 +11278,8 @@ ATPrepAlterColumnType(List **wqueue, } } else if (!recursing && - find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) + find_inheritance_children(RelationGetRelid(rel), false, + NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("type of inherited column \"%s\" must be changed in child tables too", @@ -14139,7 +14174,7 @@ ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode) */ /* Off to RemoveInheritance() where most of the work happens */ - RemoveInheritance(rel, parent_rel); + RemoveInheritance(rel, parent_rel, false); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(parent_rel)); @@ -14150,12 +14185,70 @@ ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode) return address; } +/* + * MarkInheritDetached + * + * When a partition is detached from its parent concurrently, we don't + * remove the pg_inherits row until a second transaction; as a preparatory + * step, this function marks the entry as 'detached', so that other + * transactions know to ignore the partition. + */ +static void +MarkInheritDetached(Relation child_rel, Relation parent_rel) +{ + Relation catalogRelation; + SysScanDesc scan; + ScanKeyData key; + HeapTuple inheritsTuple; + bool found = false; + + /* + * Find pg_inherits entries by inhrelid. + */ + catalogRelation = table_open(InheritsRelationId, RowExclusiveLock); + ScanKeyInit(&key, + Anum_pg_inherits_inhrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(child_rel))); + scan = systable_beginscan(catalogRelation, InheritsRelidSeqnoIndexId, + true, NULL, 1, &key); + + while (HeapTupleIsValid(inheritsTuple = systable_getnext(scan))) + { + HeapTuple newtup; + + if (((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhparent != + RelationGetRelid(parent_rel)) + elog(ERROR, "bad parent tuple found for partition %u", + RelationGetRelid(child_rel)); + + newtup = heap_copytuple(inheritsTuple); + ((Form_pg_inherits) GETSTRUCT(newtup))->inhdetached = true; + + CatalogTupleUpdate(catalogRelation, + &inheritsTuple->t_self, + newtup); + found = true; + } + + /* Done */ + systable_endscan(scan); + table_close(catalogRelation, RowExclusiveLock); + + if (!found) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_TABLE), + errmsg("relation \"%s\" is not a partition of relation \"%s\"", + RelationGetRelationName(child_rel), + RelationGetRelationName(parent_rel)))); +} + /* * RemoveInheritance * * Drop a parent from the child's parents. This just adjusts the attinhcount * and attislocal of the columns and removes the pg_inherit and pg_depend - * entries. + * entries. expect_detached is passed down to DeleteInheritsTuple, q.v.. * * If attinhcount goes to 0 then attislocal gets set to true. If it goes back * up attislocal stays true, which means if a child is ever removed from a @@ -14169,7 +14262,7 @@ ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode) * Common to ATExecDropInherit() and ATExecDetachPartition(). */ static void -RemoveInheritance(Relation child_rel, Relation parent_rel) +RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) { Relation catalogRelation; SysScanDesc scan; @@ -14185,7 +14278,9 @@ RemoveInheritance(Relation child_rel, Relation parent_rel) child_is_partition = true; found = DeleteInheritsTuple(RelationGetRelid(child_rel), - RelationGetRelid(parent_rel)); + RelationGetRelid(parent_rel), + expect_detached, + RelationGetRelationName(child_rel)); if (!found) { if (child_is_partition) @@ -16293,7 +16388,7 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel, } else if (scanrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(scanrel); + PartitionDesc partdesc = RelationGetPartitionDesc(scanrel, false); int i; for (i = 0; i < partdesc->nparts; i++) @@ -16353,7 +16448,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, * new partition will change its partition constraint. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(rel)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, false)); if (OidIsValid(defaultPartOid)) LockRelationOid(defaultPartOid, AccessExclusiveLock); @@ -16942,105 +17037,230 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) * ALTER TABLE DETACH PARTITION * * Return the address of the relation that is no longer a partition of rel. + * + * If concurrent mode is requested, we run in two transactions. A side- + * effect is that this command cannot run in a multi-part ALTER TABLE. + * Currently, that's enforced by the grammar. + * + * The strategy for concurrency is to first modify the partition catalog + * rows to make it visible to everyone that the partition is detached, + * lock the partition against writes, and commit the transaction; anyone + * who requests the partition descriptor from that point onwards has to + * ignore such a partition. In a second transaction, we wait until all + * transactions that could have seen the partition as attached are gone, + * then we remove the rest of partition metadata (pg_inherits and + * pg_class.relpartbounds). */ static ObjectAddress -ATExecDetachPartition(Relation rel, RangeVar *name) +ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, + RangeVar *name, bool concurrent) { - Relation partRel, - classRel; - HeapTuple tuple, - newtuple; - Datum new_val[Natts_pg_class]; - bool new_null[Natts_pg_class], - new_repl[Natts_pg_class]; + Relation partRel; ObjectAddress address; Oid defaultPartOid; - List *indexes; - List *fks; - ListCell *cell; /* * We must lock the default partition, because detaching this partition * will change its partition constraint. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(rel)); - if (OidIsValid(defaultPartOid)) - LockRelationOid(defaultPartOid, AccessExclusiveLock); - - partRel = table_openrv(name, ShareUpdateExclusiveLock); - - /* Ensure that foreign keys still hold after this detach */ - ATDetachCheckNoForeignKeyRefs(partRel); - - /* All inheritance related checks are performed within the function */ - RemoveInheritance(partRel, rel); - - /* Update pg_class tuple */ - classRel = table_open(RelationRelationId, RowExclusiveLock); - tuple = SearchSysCacheCopy1(RELOID, - ObjectIdGetDatum(RelationGetRelid(partRel))); - if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for relation %u", - RelationGetRelid(partRel)); - Assert(((Form_pg_class) GETSTRUCT(tuple))->relispartition); - - /* Clear relpartbound and reset relispartition */ - memset(new_val, 0, sizeof(new_val)); - memset(new_null, false, sizeof(new_null)); - memset(new_repl, false, sizeof(new_repl)); - new_val[Anum_pg_class_relpartbound - 1] = (Datum) 0; - new_null[Anum_pg_class_relpartbound - 1] = true; - new_repl[Anum_pg_class_relpartbound - 1] = true; - newtuple = heap_modify_tuple(tuple, RelationGetDescr(classRel), - new_val, new_null, new_repl); - - ((Form_pg_class) GETSTRUCT(newtuple))->relispartition = false; - CatalogTupleUpdate(classRel, &newtuple->t_self, newtuple); - heap_freetuple(newtuple); - + get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, false)); if (OidIsValid(defaultPartOid)) { /* - * If the relation being detached is the default partition itself, - * remove it from the parent's pg_partitioned_table entry. + * Concurrent detaching when a default partition exists is not + * supported. The main problem is that the default partition + * constraint would change. And there's a definitional problem: what + * should happen to the tuples that are being inserted that belong to + * the partition being detached? Putting them on the partition being + * detached would be wrong, since they'd become "lost" after the but + * we cannot put them in the default partition either until we alter + * its partition constraint. * - * If not, we must invalidate default partition's relcache entry, as - * in StorePartitionBound: its partition constraint depends on every - * other partition's partition constraint. + * I think we could solve this problem if we effected the constraint + * change before committing the first transaction. But the lock would + * have to remain AEL and it would cause concurrent query planning to + * be blocked, so changing it that way would be even worse. */ - if (RelationGetRelid(partRel) == defaultPartOid) - update_default_partition_oid(RelationGetRelid(rel), InvalidOid); - else - CacheInvalidateRelcacheByRelid(defaultPartOid); + if (concurrent) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot detach partitions concurrently when a default partition exists"))); + LockRelationOid(defaultPartOid, AccessExclusiveLock); } - /* detach indexes too */ - indexes = RelationGetIndexList(partRel); - foreach(cell, indexes) + /* + * In concurrent mode, the partition is locked with share-update-exclusive + * in the first transaction. This allows concurrent transactions to be + * doing DML to the partition. + */ + partRel = table_openrv(name, concurrent ? ShareLock : + AccessExclusiveLock); + + /* + * Check inheritance conditions and either delete the pg_inherits row + * (in non-concurrent mode) or just set the inhisdetached flag. + */ + if (!concurrent) + RemoveInheritance(partRel, rel, false); + else + MarkInheritDetached(partRel, rel); + + /* + * Ensure that foreign keys still hold after this detach. This keeps + * locks on the referencing tables, which prevents concurrent transactions + * from adding rows that we wouldn't see. For this to work in concurrent + * mode, it is critical that the partition appears as no longer attached + * for the RI queries as soon as the first transaction commits. + */ + ATDetachCheckNoForeignKeyRefs(partRel); + + /* + * Concurrent mode has to work harder; first we add a new constraint to the + * partition that matches the partition constraint, if there isn't a matching + * one already. The reason for this is that the planner may have made + * optimizations that depend on the constraint. XXX Isn't it sufficient to + * invalidate the partition's relcache entry? + * + * Then we close our existing transaction, and in a new one wait for + * all processes to catch up on the catalog updates we've done so far; at + * that point we can complete the operation. + */ + if (concurrent) { - Oid idxid = lfirst_oid(cell); - Relation idx; - Oid constrOid; + Oid partrelid, + parentrelid; + LOCKTAG tag; + char *parentrelname; + char *partrelname; - if (!has_superclass(idxid)) - continue; + /* Add constraint, if needed. XXX hopefully we can just remove this */ + DetachAddConstraintIfNeeded(wqueue, partRel); - Assert((IndexGetRelation(get_partition_parent(idxid), false) == - RelationGetRelid(rel))); + /* + * We're almost done now; the only traces that remain are the + * pg_inherits tuple and the partition's relpartbounds. Before we can + * remove those, we need to wait until all transactions that know that + * this is a partition are gone. + */ - idx = index_open(idxid, AccessExclusiveLock); - IndexSetParentIndex(idx, InvalidOid); + /* + * Remember relation OIDs to re-acquire them later; and relation names + * too, for error messages if something is dropped in between. + */ + partrelid = RelationGetRelid(partRel); + parentrelid = RelationGetRelid(rel); + parentrelname = MemoryContextStrdup(PortalContext, + RelationGetRelationName(rel)); + partrelname = MemoryContextStrdup(PortalContext, + RelationGetRelationName(partRel)); - /* If there's a constraint associated with the index, detach it too */ - constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel), - idxid); - if (OidIsValid(constrOid)) - ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid); + /* Invalidate relcache entries for the parent -- must be before close */ + CacheInvalidateRelcache(rel); + + table_close(partRel, NoLock); + table_close(rel, NoLock); + tab->rel = NULL; + + /* Make updated catalog entry visible */ + PopActiveSnapshot(); + CommitTransactionCommand(); + + StartTransactionCommand(); + + /* + * Now wait. This ensures that all queries that were planned including + * the partition are finished before we remove the rest of catalog + * entries. We don't need or indeed want to acquire this lock, though + * -- that would block later queries. + * + * We don't need to concern ourselves with waiting for a lock on the + * partition itself, since we will acquire AccessExclusiveLock below. + */ + SET_LOCKTAG_RELATION(tag, MyDatabaseId, parentrelid); + WaitForLockersMultiple(list_make1(&tag), AccessExclusiveLock, false); + + /* + * Now acquire locks in both relations again. Note they may have been + * removed in the meantime, so care is required. + */ + rel = try_relation_open(parentrelid, ShareUpdateExclusiveLock); + partRel = try_relation_open(partrelid, AccessExclusiveLock); + + /* If the relations aren't there, something bad happened; bail out */ + if (rel == NULL) + { + if (partRel != NULL) /* shouldn't happen */ + elog(WARNING, "dangling partition \"%s\" remains, can't fix", + partrelname); + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("partitioned table \"%s\" was removed concurrently", + parentrelname))); + } + if (partRel == NULL) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("partition \"%s\" was removed concurrently", partrelname))); + + tab->rel = rel; - index_close(idx, NoLock); } - table_close(classRel, RowExclusiveLock); + + /* Do the final part of detaching */ + DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid, false); + + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); + + /* keep our lock until commit */ + table_close(partRel, NoLock); + + return address; +} + +/* + * Second part of ALTER TABLE .. DETACH. + * + * This is separate so that it can be run independently when the second + * transaction of the concurrent algorithm fails (crash or abort). + */ +static void +DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, + Oid defaultPartOid, bool wait) +{ + Relation classRel; + List *fks; + ListCell *cell; + List *indexes; + Datum new_val[Natts_pg_class]; + bool new_null[Natts_pg_class], + new_repl[Natts_pg_class]; + HeapTuple tuple, + newtuple; + + /* + * If asked to, wait until existing snapshots are gone. This is important + * if the second transaction of DETACH PARTITION CONCURRENTLY is canceled: + * the user could immediately run DETACH FINALIZE without actually waiting + * for existing transactions. We must not complete the detach action until + * all such queries are complete (otherwise we would present them with an + * inconsistent view of catalogs). + */ + if (wait) + { + Snapshot snap = GetActiveSnapshot(); + + WaitForOlderSnapshots(snap->xmin, false); + } + + if (concurrent) + { + /* + * We can remove the pg_inherits row now. (In the non-concurrent case, + * this was already done). + */ + RemoveInheritance(partRel, rel, true); + } /* Drop any triggers that were cloned on creation/attach. */ DropClonedTriggersFromPartition(RelationGetRelid(partRel)); @@ -17113,22 +17333,150 @@ ATExecDetachPartition(Relation rel, RangeVar *name) ObjectAddressSet(constraint, ConstraintRelationId, constrOid); performDeletion(&constraint, DROP_RESTRICT, 0); } - CommandCounterIncrement(); + + /* Now we can detach indexes */ + indexes = RelationGetIndexList(partRel); + foreach(cell, indexes) + { + Oid idxid = lfirst_oid(cell); + Relation idx; + Oid constrOid; + + if (!has_superclass(idxid)) + continue; + + Assert((IndexGetRelation(get_partition_parent(idxid), false) == + RelationGetRelid(rel))); + + idx = index_open(idxid, AccessExclusiveLock); + IndexSetParentIndex(idx, InvalidOid); + + /* If there's a constraint associated with the index, detach it too */ + constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel), + idxid); + if (OidIsValid(constrOid)) + ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid); + + index_close(idx, NoLock); + } + + /* Update pg_class tuple */ + classRel = table_open(RelationRelationId, RowExclusiveLock); + tuple = SearchSysCacheCopy1(RELOID, + ObjectIdGetDatum(RelationGetRelid(partRel))); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", + RelationGetRelid(partRel)); + Assert(((Form_pg_class) GETSTRUCT(tuple))->relispartition); + + /* Clear relpartbound and reset relispartition */ + memset(new_val, 0, sizeof(new_val)); + memset(new_null, false, sizeof(new_null)); + memset(new_repl, false, sizeof(new_repl)); + new_val[Anum_pg_class_relpartbound - 1] = (Datum) 0; + new_null[Anum_pg_class_relpartbound - 1] = true; + new_repl[Anum_pg_class_relpartbound - 1] = true; + newtuple = heap_modify_tuple(tuple, RelationGetDescr(classRel), + new_val, new_null, new_repl); + + ((Form_pg_class) GETSTRUCT(newtuple))->relispartition = false; + CatalogTupleUpdate(classRel, &newtuple->t_self, newtuple); + heap_freetuple(newtuple); + table_close(classRel, RowExclusiveLock); + + if (OidIsValid(defaultPartOid)) + { + /* + * If the relation being detached is the default partition itself, + * remove it from the parent's pg_partitioned_table entry. + * + * If not, we must invalidate default partition's relcache entry, as + * in StorePartitionBound: its partition constraint depends on every + * other partition's partition constraint. + */ + if (RelationGetRelid(partRel) == defaultPartOid) + update_default_partition_oid(RelationGetRelid(rel), InvalidOid); + else + CacheInvalidateRelcacheByRelid(defaultPartOid); + } /* * Invalidate the parent's relcache so that the partition is no longer * included in its partition descriptor. */ CacheInvalidateRelcache(rel); +} + +/* + * ALTER TABLE ... DETACH PARTITION ... FINALIZE + * + * To use when a DETACH PARTITION command previously did not run to + * completion; this completes the detaching process. + */ +static ObjectAddress +ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) +{ + Relation partRel; + ObjectAddress address; + + partRel = table_openrv(name, AccessExclusiveLock); + + DetachPartitionFinalize(rel, partRel, true, InvalidOid, true); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); - /* keep our lock until commit */ table_close(partRel, NoLock); return address; } +/* + * DetachAddConstraintIfNeeded + * Subroutine for ATExecDetachPartition. Create a constraint that + * takes the place of the partition constraint, but avoid creating + * a dupe if an equivalent constraint already exists. + */ +static void +DetachAddConstraintIfNeeded(List **wqueue, Relation partRel) +{ + AlteredTableInfo *tab; + Expr *constraintExpr; + TupleDesc td = RelationGetDescr(partRel); + Constraint *n; + + constraintExpr = make_ands_explicit(RelationGetPartitionQual(partRel)); + + /* If an identical constraint exists, we don't need to create one */ + if (td->constr && td->constr->num_check > 0) + { + for (int i = 0; i < td->constr->num_check; i++) + { + Node *thisconstr; + + thisconstr = stringToNode(td->constr->check[i].ccbin); + + if (equal(constraintExpr, thisconstr)) + return; + } + } + + tab = ATGetQueueEntry(wqueue, partRel); + + /* Add constraint on partition, equivalent to the partition constraint */ + n = makeNode(Constraint); + n->contype = CONSTR_CHECK; + n->conname = NULL; + n->location = -1; + n->is_no_inherit = false; + n->raw_expr = NULL; + n->cooked_expr = nodeToString(constraintExpr); + n->initially_valid = true; + n->skip_validation = true; + /* It's a re-add, since it nominally already exists */ + ATAddCheckConstraint(wqueue, tab, partRel, n, + true, false, true, ShareUpdateExclusiveLock); +} + /* * DropClonedTriggersFromPartition * subroutine for ATExecDetachPartition to remove any triggers that were @@ -17324,7 +17672,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) RelationGetRelationName(partIdx)))); /* Make sure it indexes a partition of the other index's table */ - partDesc = RelationGetPartitionDesc(parentTbl); + partDesc = RelationGetPartitionDesc(parentTbl, false); found = false; for (i = 0; i < partDesc->nparts; i++) { @@ -17478,7 +17826,7 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) * If we found as many inherited indexes as the partitioned table has * partitions, we're good; update pg_index to set indisvalid. */ - if (tuples == RelationGetPartitionDesc(partedTbl)->nparts) + if (tuples == RelationGetPartitionDesc(partedTbl, false)->nparts) { Relation idxRel; HeapTuple newtup; diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 12229364f1..d422d332d5 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1129,7 +1129,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, */ if (partition_recurse) { - PartitionDesc partdesc = RelationGetPartitionDesc(rel); + PartitionDesc partdesc = RelationGetPartitionDesc(rel, false); List *idxs = NIL; List *childTbls = NIL; ListCell *l; @@ -1151,7 +1151,8 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ListCell *l; List *idxs = NIL; - idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock); + idxs = find_inheritance_children(indexOid, false, + ShareRowExclusiveLock); foreach(l, idxs) childTbls = lappend_oid(childTbls, IndexGetRelation(lfirst_oid(l), diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 941731a0a9..87336d81e0 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -567,6 +567,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, int partidx) { ModifyTable *node = (ModifyTable *) mtstate->ps.plan; + Oid partOid = dispatch->partdesc->oids[partidx]; Relation rootrel = rootResultRelInfo->ri_RelationDesc, partrel; Relation firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc; @@ -577,7 +578,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, oldcxt = MemoryContextSwitchTo(proute->memcxt); - partrel = table_open(dispatch->partdesc->oids[partidx], RowExclusiveLock); + partrel = table_open(partOid, RowExclusiveLock); leaf_part_rri = makeNode(ResultRelInfo); InitResultRelInfo(leaf_part_rri, @@ -1046,9 +1047,21 @@ ExecInitPartitionDispatchInfo(EState *estate, int dispatchidx; MemoryContext oldcxt; + /* + * For data modification, it is better that executor does not include + * partitions being detached, except in snapshot-isolation mode. This + * means that a read-committed transaction immediately gets a "no + * partition for tuple" error when a tuple is inserted into a partition + * that's being detached concurrently, but a transaction in repeatable- + * read mode can still use the partition. Note that because partition + * detach uses ShareLock on the partition (which conflicts with DML), + * we're certain that the detach won't be able to complete until any + * inserting transaction is done. + */ if (estate->es_partition_directory == NULL) estate->es_partition_directory = - CreatePartitionDirectory(estate->es_query_cxt); + CreatePartitionDirectory(estate->es_query_cxt, + IsolationUsesXactSnapshot()); oldcxt = MemoryContextSwitchTo(proute->memcxt); @@ -1628,9 +1641,10 @@ ExecCreatePartitionPruneState(PlanState *planstate, ListCell *lc; int i; + /* Executor must always include detached partitions */ if (estate->es_partition_directory == NULL) estate->es_partition_directory = - CreatePartitionDirectory(estate->es_query_cxt); + CreatePartitionDirectory(estate->es_query_cxt, true); n_part_hierarchies = list_length(partitionpruneinfo->prune_infos); Assert(n_part_hierarchies > 0); @@ -1696,9 +1710,12 @@ ExecCreatePartitionPruneState(PlanState *planstate, partrel); /* - * Initialize the subplan_map and subpart_map. Since detaching a - * partition requires AccessExclusiveLock, no partitions can have - * disappeared, nor can the bounds for any partition have changed. + * Initialize the subplan_map and subpart_map. + * + * Because we request detached partitions to be included, and + * detaching waits for old transactions, it is safe to assume that + * no partitions have disappeared since this query was planned. + * * However, new partitions may have been added. */ Assert(partdesc->nparts >= pinfo->nparts); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index ba3ccc712c..80685bdff9 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4678,6 +4678,7 @@ _copyPartitionCmd(const PartitionCmd *from) COPY_NODE_FIELD(name); COPY_NODE_FIELD(bound); + COPY_SCALAR_FIELD(concurrent); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index a2ef853dc2..1e73308280 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2941,6 +2941,7 @@ _equalPartitionCmd(const PartitionCmd *a, const PartitionCmd *b) { COMPARE_NODE_FIELD(name); COMPARE_NODE_FIELD(bound); + COMPARE_SCALAR_FIELD(concurrent); return true; } diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index da322b453e..da5a8c5f5a 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -2135,10 +2135,17 @@ set_relation_partition_info(PlannerInfo *root, RelOptInfo *rel, { PartitionDesc partdesc; - /* Create the PartitionDirectory infrastructure if we didn't already */ + /* + * Create the PartitionDirectory infrastructure if we didn't already. + * In snapshot-isolation transactions, we always include detached + * partitions, because otherwise they become invisible too soon. + */ if (root->glob->partition_directory == NULL) + { root->glob->partition_directory = - CreatePartitionDirectory(CurrentMemoryContext); + CreatePartitionDirectory(CurrentMemoryContext, + IsolationUsesXactSnapshot()); + } partdesc = PartitionDirectoryLookup(root->glob->partition_directory, relation); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 31c95443a5..4ae8caf79f 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -644,7 +644,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN EXPRESSION EXTENSION EXTERNAL EXTRACT - FALSE_P FAMILY FETCH FILTER FIRST_P FLOAT_P FOLLOWING FOR + FALSE_P FAMILY FETCH FILTER FINALIZE FIRST_P FLOAT_P FOLLOWING FOR FORCE FOREIGN FORWARD FREEZE FROM FULL FUNCTION FUNCTIONS GENERATED GLOBAL GRANT GRANTED GREATEST GROUP_P GROUPING GROUPS @@ -2094,12 +2094,13 @@ partition_cmd: n->subtype = AT_AttachPartition; cmd->name = $3; cmd->bound = $4; + cmd->concurrent = false; n->def = (Node *) cmd; $$ = (Node *) n; } - /* ALTER TABLE <name> DETACH PARTITION <partition_name> */ - | DETACH PARTITION qualified_name + /* ALTER TABLE <name> DETACH PARTITION <partition_name> [CONCURRENTLY] */ + | DETACH PARTITION qualified_name opt_concurrently { AlterTableCmd *n = makeNode(AlterTableCmd); PartitionCmd *cmd = makeNode(PartitionCmd); @@ -2107,8 +2108,21 @@ partition_cmd: n->subtype = AT_DetachPartition; cmd->name = $3; cmd->bound = NULL; + cmd->concurrent = $4; n->def = (Node *) cmd; + $$ = (Node *) n; + } + | DETACH PARTITION qualified_name FINALIZE + { + AlterTableCmd *n = makeNode(AlterTableCmd); + PartitionCmd *cmd = makeNode(PartitionCmd); + + n->subtype = AT_DetachPartitionFinalize; + cmd->name = $3; + cmd->bound = NULL; + cmd->concurrent = false; + n->def = (Node *) cmd; $$ = (Node *) n; } ; @@ -2123,6 +2137,7 @@ index_partition_cmd: n->subtype = AT_AttachPartition; cmd->name = $3; cmd->bound = NULL; + cmd->concurrent = false; n->def = (Node *) cmd; $$ = (Node *) n; @@ -15276,6 +15291,7 @@ unreserved_keyword: | EXTERNAL | FAMILY | FILTER + | FINALIZE | FIRST_P | FOLLOWING | FORCE @@ -15814,6 +15830,7 @@ bare_label_keyword: | EXTRACT | FALSE_P | FAMILY + | FINALIZE | FIRST_P | FLOAT_P | FOLLOWING diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index b9aeb77bc2..336cc1db9b 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -2808,7 +2808,7 @@ check_new_partition_bound(char *relname, Relation parent, PartitionBoundSpec *spec, ParseState *pstate) { PartitionKey key = RelationGetPartitionKey(parent); - PartitionDesc partdesc = RelationGetPartitionDesc(parent); + PartitionDesc partdesc = RelationGetPartitionDesc(parent, true); PartitionBoundInfo boundinfo = partdesc->boundinfo; int with = -1; bool overlap = false; @@ -4017,7 +4017,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) { int i; int ndatums = 0; - PartitionDesc pdesc = RelationGetPartitionDesc(parent); + PartitionDesc pdesc = RelationGetPartitionDesc(parent, true); /* XXX correct? */ PartitionBoundInfo boundinfo = pdesc->boundinfo; if (boundinfo) @@ -4217,7 +4217,7 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec, if (spec->is_default) { List *or_expr_args = NIL; - PartitionDesc pdesc = RelationGetPartitionDesc(parent); + PartitionDesc pdesc = RelationGetPartitionDesc(parent, true); /* XXX correct? */ Oid *inhoids = pdesc->oids; int nparts = pdesc->nparts, i; diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c index f852b6e99d..58570fecfd 100644 --- a/src/backend/partitioning/partdesc.c +++ b/src/backend/partitioning/partdesc.c @@ -37,6 +37,7 @@ typedef struct PartitionDirectoryData { MemoryContext pdir_mcxt; HTAB *pdir_hash; + bool include_detached; } PartitionDirectoryData; typedef struct PartitionDirectoryEntry @@ -46,7 +47,7 @@ typedef struct PartitionDirectoryEntry PartitionDesc pd; } PartitionDirectoryEntry; -static void RelationBuildPartitionDesc(Relation rel); +static void RelationBuildPartitionDesc(Relation rel, bool include_detached); /* @@ -61,13 +62,14 @@ static void RelationBuildPartitionDesc(Relation rel); * that the data doesn't become stale. */ PartitionDesc -RelationGetPartitionDesc(Relation rel) +RelationGetPartitionDesc(Relation rel, bool include_detached) { if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) return NULL; - if (unlikely(rel->rd_partdesc == NULL)) - RelationBuildPartitionDesc(rel); + if (unlikely(rel->rd_partdesc == NULL || + rel->rd_partdesc->includes_detached != include_detached)) + RelationBuildPartitionDesc(rel, include_detached); return rel->rd_partdesc; } @@ -88,7 +90,7 @@ RelationGetPartitionDesc(Relation rel) * permanently. */ static void -RelationBuildPartitionDesc(Relation rel) +RelationBuildPartitionDesc(Relation rel, bool include_detached) { PartitionDesc partdesc; PartitionBoundInfo boundinfo = NULL; @@ -110,7 +112,8 @@ RelationBuildPartitionDesc(Relation rel) * concurrently, whatever this function returns will be accurate as of * some well-defined point in time. */ - inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock); + inhoids = find_inheritance_children(RelationGetRelid(rel), include_detached, + NoLock); nparts = list_length(inhoids); /* Allocate working arrays for OIDs, leaf flags, and boundspecs. */ @@ -238,6 +241,7 @@ RelationBuildPartitionDesc(Relation rel) partdesc->boundinfo = partition_bounds_copy(boundinfo, key); partdesc->oids = (Oid *) palloc(nparts * sizeof(Oid)); partdesc->is_leaf = (bool *) palloc(nparts * sizeof(bool)); + partdesc->includes_detached = include_detached; /* * Assign OIDs from the original array into mapped indexes of the @@ -280,7 +284,7 @@ RelationBuildPartitionDesc(Relation rel) * Create a new partition directory object. */ PartitionDirectory -CreatePartitionDirectory(MemoryContext mcxt) +CreatePartitionDirectory(MemoryContext mcxt, bool include_detached) { MemoryContext oldcontext = MemoryContextSwitchTo(mcxt); PartitionDirectory pdir; @@ -295,6 +299,7 @@ CreatePartitionDirectory(MemoryContext mcxt) pdir->pdir_hash = hash_create("partition directory", 256, &ctl, HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); + pdir->include_detached = include_detached; MemoryContextSwitchTo(oldcontext); return pdir; @@ -327,7 +332,7 @@ PartitionDirectoryLookup(PartitionDirectory pdir, Relation rel) */ RelationIncrementReferenceCount(rel); pde->rel = rel; - pde->pd = RelationGetPartitionDesc(rel); + pde->pd = RelationGetPartitionDesc(rel, pdir->include_detached); Assert(pde->pd != NULL); } return pde->pd; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 53a511f1da..38ff1e0923 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1274,6 +1274,25 @@ ProcessUtilitySlow(ParseState *pstate, AlterTableStmt *atstmt = (AlterTableStmt *) parsetree; Oid relid; LOCKMODE lockmode; + ListCell *cell; + + /* + * Disallow ALTER TABLE .. DETACH CONCURRENTLY in a + * transaction block or function. (Perhaps it could be + * allowed in a procedure, but don't hold your breath.) + */ + foreach(cell, atstmt->cmds) + { + AlterTableCmd *cmd = (AlterTableCmd *) lfirst(cell); + + /* Disallow DETACH CONCURRENTLY in a transaction block */ + if (cmd->subtype == AT_DetachPartition) + { + if (((PartitionCmd *) cmd->def)->concurrent) + PreventInTransactionBlock(isTopLevel, + "ALTER TABLE ... DETACH CONCURRENTLY"); + } + } /* * Figure out lock mode, and acquire lock. This also does diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index a6388d980e..74d85b89fb 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -341,6 +341,7 @@ generate_partition_qual(Relation rel) bool isnull; List *my_qual = NIL, *result = NIL; + Oid parentrelid; Relation parent; /* Guard against stack overflow due to overly deep partition tree */ @@ -350,9 +351,19 @@ generate_partition_qual(Relation rel) if (rel->rd_partcheckvalid) return copyObject(rel->rd_partcheck); + /* + * Obtain parent relid; if it's invalid, then the partition is being + * detached + */ + parentrelid = get_partition_parent(RelationGetRelid(rel)); + if (parentrelid == InvalidOid) + { + rel->rd_partcheckvalid = true; + return NIL; + } + /* Grab at least an AccessShareLock on the parent table */ - parent = relation_open(get_partition_parent(RelationGetRelid(rel)), - AccessShareLock); + parent = relation_open(parentrelid, AccessShareLock); /* Get pg_class.relpartbound */ tuple = SearchSysCache1(RELOID, RelationGetRelid(rel)); diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index caf97563f4..883ebdc474 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2117,7 +2117,12 @@ describeOneTableDetails(const char *schemaname, printfPQExpBuffer(&buf, "SELECT inhparent::pg_catalog.regclass,\n" - " pg_catalog.pg_get_expr(c.relpartbound, c.oid)"); + " pg_catalog.pg_get_expr(c.relpartbound, c.oid),\n "); + + appendPQExpBuffer(&buf, + pset.sversion >= 140000 ? "inhdetached" : + "false as inhdetached"); + /* If verbose, also request the partition constraint definition */ if (verbose) appendPQExpBufferStr(&buf, @@ -2135,17 +2140,19 @@ describeOneTableDetails(const char *schemaname, { char *parent_name = PQgetvalue(result, 0, 0); char *partdef = PQgetvalue(result, 0, 1); + char *detached = PQgetvalue(result, 0, 2); - printfPQExpBuffer(&tmpbuf, _("Partition of: %s %s"), parent_name, - partdef); + printfPQExpBuffer(&tmpbuf, _("Partition of: %s %s%s"), parent_name, + partdef, + strcmp(detached, "t") == 0 ? " DETACHED" : ""); printTableAddFooter(&cont, tmpbuf.data); if (verbose) { char *partconstraintdef = NULL; - if (!PQgetisnull(result, 0, 2)) - partconstraintdef = PQgetvalue(result, 0, 2); + if (!PQgetisnull(result, 0, 3)) + partconstraintdef = PQgetvalue(result, 0, 3); /* If there isn't any constraint, show that explicitly */ if (partconstraintdef == NULL || partconstraintdef[0] == '\0') printfPQExpBuffer(&tmpbuf, _("No partition constraint")); @@ -3197,9 +3204,18 @@ describeOneTableDetails(const char *schemaname, } /* print child tables (with additional info if partitions) */ - if (pset.sversion >= 100000) + if (pset.sversion >= 140000) printfPQExpBuffer(&buf, - "SELECT c.oid::pg_catalog.regclass, c.relkind," + "SELECT c.oid::pg_catalog.regclass, c.relkind, inhdetached," + " pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n" + "FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n" + "WHERE c.oid = i.inhrelid AND i.inhparent = '%s'\n" + "ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT'," + " c.oid::pg_catalog.regclass::pg_catalog.text;", + oid); + else if (pset.sversion >= 100000) + printfPQExpBuffer(&buf, + "SELECT c.oid::pg_catalog.regclass, c.relkind, false AS inhdetached," " pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n" "FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n" "WHERE c.oid = i.inhrelid AND i.inhparent = '%s'\n" @@ -3208,14 +3224,14 @@ describeOneTableDetails(const char *schemaname, oid); else if (pset.sversion >= 80300) printfPQExpBuffer(&buf, - "SELECT c.oid::pg_catalog.regclass, c.relkind, NULL\n" + "SELECT c.oid::pg_catalog.regclass, c.relkind, false AS inhdetached, NULL\n" "FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n" "WHERE c.oid = i.inhrelid AND i.inhparent = '%s'\n" "ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid); else printfPQExpBuffer(&buf, - "SELECT c.oid::pg_catalog.regclass, c.relkind, NULL\n" + "SELECT c.oid::pg_catalog.regclass, c.relkind, false AS inhdetached, NULL\n" "FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n" "WHERE c.oid = i.inhrelid AND i.inhparent = '%s'\n" "ORDER BY c.relname;", @@ -3265,11 +3281,13 @@ describeOneTableDetails(const char *schemaname, else printfPQExpBuffer(&buf, "%*s %s", ctw, "", PQgetvalue(result, i, 0)); - if (!PQgetisnull(result, i, 2)) - appendPQExpBuffer(&buf, " %s", PQgetvalue(result, i, 2)); + if (!PQgetisnull(result, i, 3)) + appendPQExpBuffer(&buf, " %s", PQgetvalue(result, i, 3)); if (child_relkind == RELKIND_PARTITIONED_TABLE || child_relkind == RELKIND_PARTITIONED_INDEX) appendPQExpBufferStr(&buf, ", PARTITIONED"); + if (strcmp(PQgetvalue(result, i, 2), "t") == 0) + appendPQExpBuffer(&buf, " (DETACHED)"); if (i < tuples - 1) appendPQExpBufferChar(&buf, ','); diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.h index a0f2ec4e0b..ba2a150128 100644 --- a/src/include/catalog/pg_inherits.h +++ b/src/include/catalog/pg_inherits.h @@ -34,6 +34,7 @@ CATALOG(pg_inherits,2611,InheritsRelationId) Oid inhrelid; Oid inhparent; int32 inhseqno; + bool inhdetached; } FormData_pg_inherits; /* ---------------- @@ -49,7 +50,8 @@ DECLARE_INDEX(pg_inherits_parent_index, 2187, on pg_inherits using btree(inhpare #define InheritsParentIndexId 2187 -extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode); +extern List *find_inheritance_children(Oid parentrelId, bool include_detached, + LOCKMODE lockmode); extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **parents); extern bool has_subclass(Oid relationId); @@ -57,6 +59,7 @@ extern bool has_superclass(Oid relationId); extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId); extern void StoreSingleInheritance(Oid relationId, Oid parentOid, int32 seqNumber); -extern bool DeleteInheritsTuple(Oid inhrelid, Oid inhparent); +extern bool DeleteInheritsTuple(Oid inhrelid, Oid inhparent, bool allow_detached, + const char *childname); #endif /* PG_INHERITS_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index dc2bb40926..1a2f87f5f9 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -860,6 +860,7 @@ typedef struct PartitionCmd NodeTag type; RangeVar *name; /* name of partition to attach/detach */ PartitionBoundSpec *bound; /* FOR VALUES, if attaching */ + bool concurrent; } PartitionCmd; /**************************************************************************** @@ -1870,6 +1871,7 @@ typedef enum AlterTableType AT_GenericOptions, /* OPTIONS (...) */ AT_AttachPartition, /* ATTACH PARTITION */ AT_DetachPartition, /* DETACH PARTITION */ + AT_DetachPartitionFinalize, /* DETACH PARTITION FINALIZE */ AT_AddIdentity, /* ADD IDENTITY */ AT_SetIdentity, /* SET identity column options */ AT_DropIdentity, /* DROP IDENTITY */ diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index 8c554e1f69..c79b76edca 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -163,6 +163,7 @@ PG_KEYWORD("false", FALSE_P, RESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("family", FAMILY, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("fetch", FETCH, RESERVED_KEYWORD, AS_LABEL) PG_KEYWORD("filter", FILTER, UNRESERVED_KEYWORD, AS_LABEL) +PG_KEYWORD("finalize", FINALIZE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("first", FIRST_P, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("float", FLOAT_P, COL_NAME_KEYWORD, BARE_LABEL) PG_KEYWORD("following", FOLLOWING, UNRESERVED_KEYWORD, BARE_LABEL) diff --git a/src/include/partitioning/partdesc.h b/src/include/partitioning/partdesc.h index ff113199e5..7f03ff4271 100644 --- a/src/include/partitioning/partdesc.h +++ b/src/include/partitioning/partdesc.h @@ -21,6 +21,7 @@ typedef struct PartitionDescData { int nparts; /* Number of partitions */ + bool includes_detached; /* Does it include detached partitions */ Oid *oids; /* Array of 'nparts' elements containing * partition OIDs in order of the their bounds */ bool *is_leaf; /* Array of 'nparts' elements storing whether @@ -30,9 +31,9 @@ typedef struct PartitionDescData } PartitionDescData; -extern PartitionDesc RelationGetPartitionDesc(Relation rel); +extern PartitionDesc RelationGetPartitionDesc(Relation rel, bool include_detached); -extern PartitionDirectory CreatePartitionDirectory(MemoryContext mcxt); +extern PartitionDirectory CreatePartitionDirectory(MemoryContext mcxt, bool include_detached); extern PartitionDesc PartitionDirectoryLookup(PartitionDirectory, Relation); extern void DestroyPartitionDirectory(PartitionDirectory pdir); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 579be352c5..21487b6d44 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -133,6 +133,7 @@ extern void AtEOXact_Snapshot(bool isCommit, bool resetXmin); extern void ImportSnapshot(const char *idstr); extern bool XactHasExportedSnapshots(void); extern void DeleteAllExportedSnapshotFiles(void); +extern void WaitForOlderSnapshots(TransactionId limitXmin, bool progress); extern bool ThereAreNoPriorRegisteredSnapshots(void); extern bool TransactionIdLimitedForOldSnapshots(TransactionId recentXmin, Relation relation, diff --git a/src/test/isolation/expected/detach-partition-concurrently-1.out b/src/test/isolation/expected/detach-partition-concurrently-1.out new file mode 100644 index 0000000000..8d15beac60 --- /dev/null +++ b/src/test/isolation/expected/detach-partition-concurrently-1.out @@ -0,0 +1,301 @@ +Parsed test spec with 3 sessions + +starting permutation: s1b s1s s2detach s1s s1c s1s +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +step s1c: COMMIT; +step s2detach: <... completed> +step s1s: SELECT * FROM d_listp; +a + +1 + +starting permutation: s1b s1s s2detach s1s s3s s3i s1c s3i s2drop s1s +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +step s3s: SELECT * FROM d_listp; +a + +1 +step s3i: SELECT relpartbound IS NULL FROM pg_class where relname = 'd_listp2'; +?column? + +f +step s1c: COMMIT; +step s2detach: <... completed> +step s3i: SELECT relpartbound IS NULL FROM pg_class where relname = 'd_listp2'; +?column? + +t +step s2drop: DROP TABLE d_listp2; +step s1s: SELECT * FROM d_listp; +a + +1 + +starting permutation: s1b s1s s2detach s1ins s1s s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1ins: INSERT INTO d_listp VALUES (1); +step s1s: SELECT * FROM d_listp; +a + +1 +1 +step s1c: COMMIT; +step s2detach: <... completed> + +starting permutation: s1b s1s s1ins2 s2detach s1ins s1s s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1ins2: INSERT INTO d_listp VALUES (2); +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1ins: INSERT INTO d_listp VALUES (1); +step s1s: SELECT * FROM d_listp; +a + +1 +1 +2 +2 +step s1c: COMMIT; +step s2detach: <... completed> + +starting permutation: s1brr s1s s2detach s1ins s1s s1c +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1ins: INSERT INTO d_listp VALUES (1); +step s1s: SELECT * FROM d_listp; +a + +1 +1 +2 +step s1c: COMMIT; +step s2detach: <... completed> + +starting permutation: s1brr s1s s2detach s1s s1c +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1c: COMMIT; +step s2detach: <... completed> + +starting permutation: s1b s1ins2 s2detach s3ins2 s1c s3debugtest +step s1b: BEGIN; +step s1ins2: INSERT INTO d_listp VALUES (2); +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s3ins2: INSERT INTO d_listp VALUES (2); <waiting ...> +step s1c: COMMIT; +step s2detach: <... completed> +step s3ins2: <... completed> +step s3debugtest: SELECT tableoid::regclass FROM d_listp; SELECT * from d_listp2; +tableoid + +d_listp1 +a + +2 +2 +2 + +starting permutation: s1brr s1prep s1s s2detach s1s s1exec1 s3s s1dealloc s1c +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1prep: PREPARE f(int) AS INSERT INTO d_listp VALUES ($1); +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1exec1: EXECUTE f(1); +step s3s: SELECT * FROM d_listp; +a + +1 +step s1dealloc: DEALLOCATE f; +step s1c: COMMIT; +step s2detach: <... completed> + +starting permutation: s1brr s1prep s1exec2 s2detach s1s s1exec2 s3s s1c s1dealloc +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1prep: PREPARE f(int) AS INSERT INTO d_listp VALUES ($1); +step s1exec2: EXECUTE f(2); +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +2 +step s1exec2: EXECUTE f(2); +step s3s: SELECT * FROM d_listp; +a + +1 +2 +step s1c: COMMIT; +step s2detach: <... completed> +step s1dealloc: DEALLOCATE f; + +starting permutation: s1brr s1prep s1s s2detach s1s s1exec2 s1c s1dealloc +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1prep: PREPARE f(int) AS INSERT INTO d_listp VALUES ($1); +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1exec2: EXECUTE f(2); +step s1c: COMMIT; +step s2detach: <... completed> +step s1dealloc: DEALLOCATE f; + +starting permutation: s1brr s1prep s2detach s1s s1exec2 s1c s1dealloc +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1prep: PREPARE f(int) AS INSERT INTO d_listp VALUES ($1); +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1exec2: EXECUTE f(2); +step s1c: COMMIT; +step s2detach: <... completed> +step s1dealloc: DEALLOCATE f; + +starting permutation: s1brr s1prep1 s2detach s1s s1exec2 s1c s1dealloc +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1prep1: PREPARE f(int) AS INSERT INTO d_listp VALUES (1); +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1exec2: EXECUTE f(2); +step s1c: COMMIT; +step s2detach: <... completed> +step s1dealloc: DEALLOCATE f; + +starting permutation: s1brr s1prep2 s2detach s1s s1exec2 s1c s1dealloc +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1prep2: PREPARE f(int) AS INSERT INTO d_listp VALUES (2); +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s1exec2: EXECUTE f(2); +step s1c: COMMIT; +step s2detach: <... completed> +step s1dealloc: DEALLOCATE f; + +starting permutation: s1b s1s s2detach s3drop1 s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s3drop1: DROP TABLE d_listp; <waiting ...> +step s1c: COMMIT; +step s2detach: <... completed> +step s3drop1: <... completed> +error in steps s1c s2detach s3drop1: ERROR: partitioned table "d_listp" was removed concurrently + +starting permutation: s1b s1s s2detach s3drop2 s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s3drop2: DROP TABLE d_listp2; <waiting ...> +step s1c: COMMIT; +step s2detach: <... completed> +step s3drop2: <... completed> +error in steps s1c s2detach s3drop2: ERROR: partition "d_listp2" was removed concurrently + +starting permutation: s1b s1s s2detach s3detach s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s3detach: ALTER TABLE d_listp DETACH PARTITION d_listp2; <waiting ...> +step s1c: COMMIT; +step s2detach: <... completed> +step s3detach: <... completed> +error in steps s1c s2detach s3detach: ERROR: cannot detach partition "d_listp2" + +starting permutation: s1b s1s s2detach s3rename s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_listp; +a + +1 +2 +step s2detach: ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; <waiting ...> +step s3rename: ALTER TABLE d_listp2 RENAME TO d_listp_foobar; <waiting ...> +step s1c: COMMIT; +step s2detach: <... completed> +step s3rename: <... completed> diff --git a/src/test/isolation/expected/detach-partition-concurrently-2.out b/src/test/isolation/expected/detach-partition-concurrently-2.out new file mode 100644 index 0000000000..85be707b40 --- /dev/null +++ b/src/test/isolation/expected/detach-partition-concurrently-2.out @@ -0,0 +1,66 @@ +Parsed test spec with 3 sessions + +starting permutation: s1b s1s s2d s3i1 s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_lp_fk; +a + +1 +2 +step s2d: ALTER TABLE d_lp_fk DETACH PARTITION d_lp_fk_1 CONCURRENTLY; <waiting ...> +step s3i1: INSERT INTO d_lp_fk_r VALUES (1); +ERROR: insert or update on table "d_lp_fk_r" violates foreign key constraint "d_lp_fk_r_a_fkey" +step s1c: COMMIT; +step s2d: <... completed> + +starting permutation: s1b s1s s2d s3i2 s3i2 s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_lp_fk; +a + +1 +2 +step s2d: ALTER TABLE d_lp_fk DETACH PARTITION d_lp_fk_1 CONCURRENTLY; <waiting ...> +step s3i2: INSERT INTO d_lp_fk_r VALUES (2); +step s3i2: INSERT INTO d_lp_fk_r VALUES (2); +step s1c: COMMIT; +step s2d: <... completed> + +starting permutation: s1b s1s s3i1 s2d s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_lp_fk; +a + +1 +2 +step s3i1: INSERT INTO d_lp_fk_r VALUES (1); +step s2d: ALTER TABLE d_lp_fk DETACH PARTITION d_lp_fk_1 CONCURRENTLY; +ERROR: removing partition "d_lp_fk_1" violates foreign key constraint "d_lp_fk_r_a_fkey1" +step s1c: COMMIT; + +starting permutation: s1b s1s s3i2 s2d s1c +step s1b: BEGIN; +step s1s: SELECT * FROM d_lp_fk; +a + +1 +2 +step s3i2: INSERT INTO d_lp_fk_r VALUES (2); +step s2d: ALTER TABLE d_lp_fk DETACH PARTITION d_lp_fk_1 CONCURRENTLY; <waiting ...> +step s1c: COMMIT; +step s2d: <... completed> + +starting permutation: s1b s1s s3b s2d s3i1 s1c s3c +step s1b: BEGIN; +step s1s: SELECT * FROM d_lp_fk; +a + +1 +2 +step s3b: BEGIN; +step s2d: ALTER TABLE d_lp_fk DETACH PARTITION d_lp_fk_1 CONCURRENTLY; <waiting ...> +step s3i1: INSERT INTO d_lp_fk_r VALUES (1); +ERROR: insert or update on table "d_lp_fk_r" violates foreign key constraint "d_lp_fk_r_a_fkey" +step s1c: COMMIT; +step s2d: <... completed> +step s3c: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index f2e752c445..3c8fa4e07d 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -24,6 +24,8 @@ test: deadlock-hard test: deadlock-soft test: deadlock-soft-2 test: deadlock-parallel +test: detach-partition-concurrently-1 +test: detach-partition-concurrently-2 test: fk-contention test: fk-deadlock test: fk-deadlock2 diff --git a/src/test/isolation/specs/detach-partition-concurrently-1.spec b/src/test/isolation/specs/detach-partition-concurrently-1.spec new file mode 100644 index 0000000000..1e9bea8198 --- /dev/null +++ b/src/test/isolation/specs/detach-partition-concurrently-1.spec @@ -0,0 +1,90 @@ +# Test that detach partition concurrently makes the partition invisible at the +# correct time. + +setup +{ + DROP TABLE IF EXISTS d_listp, d_listp1, d_listp2; + CREATE TABLE d_listp (a int) PARTITION BY LIST(a); + CREATE TABLE d_listp1 PARTITION OF d_listp FOR VALUES IN (1); + CREATE TABLE d_listp2 PARTITION OF d_listp FOR VALUES IN (2); + INSERT INTO d_listp VALUES (1),(2); +} + +teardown { + DROP TABLE IF EXISTS d_listp, d_listp2, d_listp_foobar; +} + +session "s1" +step "s1b" { BEGIN; } +step "s1brr" { BEGIN ISOLATION LEVEL REPEATABLE READ; } +step "s1s" { SELECT * FROM d_listp; } +step "s1ins" { INSERT INTO d_listp VALUES (1); } +step "s1ins2" { INSERT INTO d_listp VALUES (2); } +step "s1prep" { PREPARE f(int) AS INSERT INTO d_listp VALUES ($1); } +step "s1prep1" { PREPARE f(int) AS INSERT INTO d_listp VALUES (1); } +step "s1prep2" { PREPARE f(int) AS INSERT INTO d_listp VALUES (2); } +step "s1exec1" { EXECUTE f(1); } +step "s1exec2" { EXECUTE f(2); } +step "s1dealloc" { DEALLOCATE f; } +step "s1c" { COMMIT; } + +session "s2" +step "s2detach" { ALTER TABLE d_listp DETACH PARTITION d_listp2 CONCURRENTLY; } +step "s2drop" { DROP TABLE d_listp2; } + +session "s3" +step "s3s" { SELECT * FROM d_listp; } +step "s3i" { SELECT relpartbound IS NULL FROM pg_class where relname = 'd_listp2'; } +step "s3ins2" { INSERT INTO d_listp VALUES (2); } +step "s3drop1" { DROP TABLE d_listp; } +step "s3drop2" { DROP TABLE d_listp2; } +step "s3detach" { ALTER TABLE d_listp DETACH PARTITION d_listp2; } +step "s3rename" { ALTER TABLE d_listp2 RENAME TO d_listp_foobar; } +# XXX remove this step +step "s3debugtest" { SELECT tableoid::regclass FROM d_listp; SELECT * from d_listp2; } + +# The transaction that detaches hangs until it sees any older transaction +# terminate, as does anybody else. +permutation "s1b" "s1s" "s2detach" "s1s" "s1c" "s1s" + +# relpartbound remains set until s1 commits +# XXX this could be timing dependent :-( +permutation "s1b" "s1s" "s2detach" "s1s" "s3s" "s3i" "s1c" "s3i" "s2drop" "s1s" + +# In read-committed mode, the partition disappears from view of concurrent +# transactions immediately. But if a write lock is held, then the detach +# has to wait. +permutation "s1b" "s1s" "s2detach" "s1ins" "s1s" "s1c" +permutation "s1b" "s1s" "s1ins2" "s2detach" "s1ins" "s1s" "s1c" + +# In repeatable-read mode, the partition remains visible until commit even +# if the to-be-detached partition is not locked for write. +permutation "s1brr" "s1s" "s2detach" "s1ins" "s1s" "s1c" +permutation "s1brr" "s1s" "s2detach" "s1s" "s1c" + +# Another process trying to acquire a write lock will be blocked behind the +# detacher +permutation "s1b" "s1ins2" "s2detach" "s3ins2" "s1c" "s3debugtest" + +# a prepared query is not blocked +permutation "s1brr" "s1prep" "s1s" "s2detach" "s1s" "s1exec1" "s3s" "s1dealloc" "s1c" +permutation "s1brr" "s1prep" "s1exec2" "s2detach" "s1s" "s1exec2" "s3s" "s1c" "s1dealloc" +permutation "s1brr" "s1prep" "s1s" "s2detach" "s1s" "s1exec2" "s1c" "s1dealloc" +permutation "s1brr" "s1prep" "s2detach" "s1s" "s1exec2" "s1c" "s1dealloc" +permutation "s1brr" "s1prep1" "s2detach" "s1s" "s1exec2" "s1c" "s1dealloc" +permutation "s1brr" "s1prep2" "s2detach" "s1s" "s1exec2" "s1c" "s1dealloc" + + +# Note: the following tests are not very interesting in isolationtester because +# the way this tool handles multiple sessions: it'll only run the drop/detach/ +# rename commands after s2 is blocked waiting. It would be more useful to +# run the DDL when s2 commits its first transaction instead. Maybe these should +# become TAP tests someday. + +# What happens if the partition or the parent table is dropped while waiting? +permutation "s1b" "s1s" "s2detach" "s3drop1" "s1c" +permutation "s1b" "s1s" "s2detach" "s3drop2" "s1c" +# Also try a non-concurrent detach while the other one is waiting +permutation "s1b" "s1s" "s2detach" "s3detach" "s1c" +# and some other DDL +permutation "s1b" "s1s" "s2detach" "s3rename" "s1c" diff --git a/src/test/isolation/specs/detach-partition-concurrently-2.spec b/src/test/isolation/specs/detach-partition-concurrently-2.spec new file mode 100644 index 0000000000..9281c80a69 --- /dev/null +++ b/src/test/isolation/specs/detach-partition-concurrently-2.spec @@ -0,0 +1,41 @@ +# Test that detach partition concurrently makes the partition safe +# for foreign keys that reference it. + +setup +{ + DROP TABLE IF EXISTS d_lp_fk, d_lp_fk_1, d_lp_fk_2, d_lp_fk_r; + + CREATE TABLE d_lp_fk (a int PRIMARY KEY) PARTITION BY LIST(a); + CREATE TABLE d_lp_fk_1 PARTITION OF d_lp_fk FOR VALUES IN (1); + CREATE TABLE d_lp_fk_2 PARTITION OF d_lp_fk FOR VALUES IN (2); + INSERT INTO d_lp_fk VALUES (1), (2); + + CREATE TABLE d_lp_fk_r (a int references d_lp_fk); +} + +teardown { DROP TABLE IF EXISTS d_lp_fk, d_lp_fk_1, d_lp_fk_2, d_lp_fk_r; } + +session "s1" +step "s1b" { BEGIN; } +step "s1s" { SELECT * FROM d_lp_fk; } +step "s1c" { COMMIT; } + +session "s2" +step "s2d" { ALTER TABLE d_lp_fk DETACH PARTITION d_lp_fk_1 CONCURRENTLY; } + +session "s3" +step "s3b" { BEGIN; } +step "s3i1" { INSERT INTO d_lp_fk_r VALUES (1); } +step "s3i2" { INSERT INTO d_lp_fk_r VALUES (2); } +step "s3c" { COMMIT; } + +# The transaction that detaches hangs until it sees any older transaction +# terminate. +permutation "s1b" "s1s" "s2d" "s3i1" "s1c" +permutation "s1b" "s1s" "s2d" "s3i2" "s3i2" "s1c" + +permutation "s1b" "s1s" "s3i1" "s2d" "s1c" +permutation "s1b" "s1s" "s3i2" "s2d" "s1c" + +# what if s3 has an uncommitted insertion? +permutation "s1b" "s1s" "s3b" "s2d" "s3i1" "s1c" "s3c" diff --git a/src/test/modules/delay_execution/Makefile b/src/test/modules/delay_execution/Makefile index f270aebf3a..70f24e846d 100644 --- a/src/test/modules/delay_execution/Makefile +++ b/src/test/modules/delay_execution/Makefile @@ -7,7 +7,8 @@ OBJS = \ $(WIN32RES) \ delay_execution.o -ISOLATION = partition-addition +ISOLATION = partition-addition \ + partition-removal-1 ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/src/test/modules/delay_execution/expected/partition-removal-1.out b/src/test/modules/delay_execution/expected/partition-removal-1.out new file mode 100644 index 0000000000..00b93ecf65 --- /dev/null +++ b/src/test/modules/delay_execution/expected/partition-removal-1.out @@ -0,0 +1,79 @@ +Parsed test spec with 3 sessions + +starting permutation: s3lock s1b s1exec s2remp s3unlock s1c +step s3lock: SELECT pg_advisory_lock(12543); +pg_advisory_lock + + +step s1b: BEGIN; +step s1exec: SELECT * FROM partrem WHERE a <> 1 AND a <> (SELECT 3); <waiting ...> +step s2remp: ALTER TABLE partrem DETACH PARTITION partrem2 CONCURRENTLY; <waiting ...> +step s3unlock: SELECT pg_advisory_unlock(12543); +pg_advisory_unlock + +t +step s1exec: <... completed> +a b + +2 JKL +step s1c: COMMIT; +step s2remp: <... completed> + +starting permutation: s3lock s1brr s1exec s2remp s3unlock s1c +step s3lock: SELECT pg_advisory_lock(12543); +pg_advisory_lock + + +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1exec: SELECT * FROM partrem WHERE a <> 1 AND a <> (SELECT 3); <waiting ...> +step s2remp: ALTER TABLE partrem DETACH PARTITION partrem2 CONCURRENTLY; <waiting ...> +step s3unlock: SELECT pg_advisory_unlock(12543); +pg_advisory_unlock + +t +step s1exec: <... completed> +a b + +2 JKL +step s1c: COMMIT; +step s2remp: <... completed> + +starting permutation: s3lock s1b s1exec2 s2remp s3unlock s1c +step s3lock: SELECT pg_advisory_lock(12543); +pg_advisory_lock + + +step s1b: BEGIN; +step s1exec2: SELECT * FROM partrem WHERE a <> (SELECT 2); <waiting ...> +step s2remp: ALTER TABLE partrem DETACH PARTITION partrem2 CONCURRENTLY; <waiting ...> +step s3unlock: SELECT pg_advisory_unlock(12543); +pg_advisory_unlock + +t +step s1exec2: <... completed> +a b + +1 ABC +3 DEF +step s1c: COMMIT; +step s2remp: <... completed> + +starting permutation: s3lock s1brr s1exec2 s2remp s3unlock s1c +step s3lock: SELECT pg_advisory_lock(12543); +pg_advisory_lock + + +step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s1exec2: SELECT * FROM partrem WHERE a <> (SELECT 2); <waiting ...> +step s2remp: ALTER TABLE partrem DETACH PARTITION partrem2 CONCURRENTLY; <waiting ...> +step s3unlock: SELECT pg_advisory_unlock(12543); +pg_advisory_unlock + +t +step s1exec2: <... completed> +a b + +1 ABC +3 DEF +step s1c: COMMIT; +step s2remp: <... completed> diff --git a/src/test/modules/delay_execution/specs/partition-removal-1.spec b/src/test/modules/delay_execution/specs/partition-removal-1.spec new file mode 100644 index 0000000000..e8e99f15bf --- /dev/null +++ b/src/test/modules/delay_execution/specs/partition-removal-1.spec @@ -0,0 +1,45 @@ +# Test removal of a partition with less-than-exclusive locking. + +setup +{ + CREATE TABLE partrem (a int, b text) PARTITION BY LIST(a); + CREATE TABLE partrem1 PARTITION OF partrem FOR VALUES IN (1); + CREATE TABLE partrem2 PARTITION OF partrem FOR VALUES IN (2); + CREATE TABLE partrem3 PARTITION OF partrem FOR VALUES IN (3); + INSERT INTO partrem VALUES (1, 'ABC'); + INSERT INTO partrem VALUES (2, 'JKL'); + INSERT INTO partrem VALUES (3, 'DEF'); +} + +teardown +{ + DROP TABLE IF EXISTS partrem, partrem2; +} + +# The SELECT will be planned with all three partitions shown above, +# of which we expect partrem1 to be pruned at planning and partrem3 at +# execution. Then we'll block, and by the time the query is actually +# executed, detach of partrem2 is already underway; however we expect +# its rows to still appear in the result. + +session "s1" +setup { LOAD 'delay_execution'; + SET delay_execution.post_planning_lock_id = 12543; } +step "s1b" { BEGIN; } +step "s1brr" { BEGIN ISOLATION LEVEL REPEATABLE READ; } +step "s1exec" { SELECT * FROM partrem WHERE a <> 1 AND a <> (SELECT 3); } +step "s1exec2" { SELECT * FROM partrem WHERE a <> (SELECT 2); } +step "s1c" { COMMIT; } + +session "s2" +step "s2remp" { ALTER TABLE partrem DETACH PARTITION partrem2 CONCURRENTLY; } + +session "s3" +step "s3lock" { SELECT pg_advisory_lock(12543); } +step "s3unlock" { SELECT pg_advisory_unlock(12543); } + +permutation "s3lock" "s1b" "s1exec" "s2remp" "s3unlock" "s1c" +permutation "s3lock" "s1brr" "s1exec" "s2remp" "s3unlock" "s1c" + +permutation "s3lock" "s1b" "s1exec2" "s2remp" "s3unlock" "s1c" +permutation "s3lock" "s1brr" "s1exec2" "s2remp" "s3unlock" "s1c" -- 2.20.1 --/04w6evG8XlLl3ft-- ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-05-18 02:07 Junwang Zhao <[email protected]> 0 siblings, 1 reply; 17+ messages in thread From: Junwang Zhao @ 2026-05-18 02:07 UTC (permalink / raw) To: Ashutosh Bapat <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> On Mon, May 18, 2026 at 8:22 AM Ashutosh Bapat <[email protected]> wrote: > > On Sun, May 17, 2026 at 5:12 PM Junwang Zhao <[email protected]> wrote: > > > > > > > > > > Regards > > Junwang Zhao > > > > On Mon, May 18, 2026 at 07:29 Ashutosh Bapat <[email protected]> wrote: > >> > >> On Fri, May 15, 2026 at 8:43 PM Ayush Tiwari > >> <[email protected]> wrote: > >> > > >> > Hi, > >> > > >> > > >> > On Fri, 15 May 2026 at 20:31, Junwang Zhao <[email protected]> wrote: > >> >> > >> >> Hi Ayush, > >> >> > >> >> >> > >> >> >> I also added regression coverage for both cases: > >> >> >> > >> >> >> DROP LABEL of a label used by a GRAPH_TABLE view > >> >> >> DROP PROPERTIES of a property used by a GRAPH_TABLE view > >> >> >> > >> >> >> Both now fail with the normal dependency error until the view is dropped. > >> >> >> > >> >> >> Thoughts? > >> >> > >> >> I'd suggest adding two stmts to the regression that can cover that walk of > >> >> graph_table_columns is also working. > >> >> > >> >> [local] zhjwpku@postgres:5432-52789=# ALTER PROPERTY GRAPH myshop > >> >> ALTER VERTEX TABLE customers ALTER LABEL customers DROP PROPERTIES > >> >> (name); > >> >> ALTER PROPERTY GRAPH > >> >> Time: 1.312 ms > >> >> > >> >> [local] zhjwpku@postgres:5432-52789=# ALTER PROPERTY GRAPH myshop > >> >> ALTER VERTEX TABLE products ALTER LABEL products DROP PROPERTIES > >> >> (name); > >> >> ERROR: cannot drop property name of property graph myshop because > >> >> other objects depend on it > >> >> DETAIL: view customers_us depends on property name of property graph myshop > >> >> HINT: Use DROP ... CASCADE to drop the dependent objects too. > >> >> Time: 2.231 ms > >> >> > >> > > >> > Good point, thanks. I added that coverage in the attached v3. > >> > > >> > The test now also drops customers.name first, which is allowed because the > >> > graph-level property still exists via products.name, and then verifies that > >> > dropping products.name is rejected with the dependency error from > >> > customers_us. That should cover GraphPropertyRef nodes reached through the > >> > GRAPH_TABLE COLUMNS list, in addition to the existing label and graph-pattern > >> > property cases. > >> > > >> > I re-added customers.name afterward so the existing myshop graph remains > >> > unchanged for the following tests. > >> > >> Thanks Ayush for working on this and providing the patch. Thanks > >> Junwang for reviewing it. > >> > >> I have some more comments. > >> > >> } > >> + else if (IsA(node, GraphLabelRef)) > >> + { > >> + GraphLabelRef *glr = (GraphLabelRef *) node; > >> + > >> + /* > >> + * GRAPH_TABLE label reference: depend on the label catalog entry. > >> + * No expression substructure to recurse into. > >> > >> That comment is correct, however, the case doesn't return false, > >> giving an impression that we are recursing into the substructure. > >> expression_tree_walker() then returns false. But I am seeing an > >> inconsistency in when to "return false" and when not to. For example, > >> for some primitive nodes in expression_tree_walker() like Var, this > >> function returns false. But for other primitive nodes like Param it > >> doesn't. And there's not comment explaining this difference. I guess > >> newer additions to this function are relying on expression_tree_walker > >> to return false. So I just removed the misleading comment and let the > >> two new nodes rely on expression_tree_walker(). > >> > >> + */ > >> + add_object_address(PropgraphLabelRelationId, glr->labelid, 0, > >> + context->addrs); > >> + } > >> + else if (IsA(node, GraphPropertyRef)) > >> + { > >> + GraphPropertyRef *gpr = (GraphPropertyRef *) node; > >> + > >> + /* GRAPH_TABLE property reference: depend on the property entry. */ > >> + add_object_address(PropgraphPropertyRelationId, gpr->propid, 0, > >> + context->addrs); > >> + } > >> > >> @@ -536,6 +536,22 @@ SELECT g.* FROM x1, > >> ORDER BY customer_name, product_name; > >> SELECT pg_get_viewdef('customers_us'::regclass); > >> +-- A view defined over GRAPH_TABLE should record dependencies on the labels > >> +-- and properties it references, so they cannot be dropped from under it. > >> +ALTER PROPERTY GRAPH myshop ALTER EDGE TABLE order_items DROP LABEL list_items; > >> +ALTER PROPERTY GRAPH myshop ALTER EDGE TABLE wishlist_items > >> + DROP LABEL list_items; -- error > >> +ALTER PROPERTY GRAPH myshop ALTER EDGE TABLE order_items > >> + ADD LABEL list_items PROPERTIES (order_id AS link_id, product_no); > >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers > >> + ALTER LABEL customers DROP PROPERTIES (address); -- error > >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers > >> + ALTER LABEL customers DROP PROPERTIES (name); > >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products > >> + ALTER LABEL products DROP PROPERTIES (name); -- error > >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers > >> + ALTER LABEL customers ADD PROPERTIES (name); > >> + > >> > >> Without the code changes, we do not see "cache lookup failed for label > >> " error, because there is nothing that uses this view after the drop. > >> In the attached patch, I have moved the DDL statements before > >> pg_get_viewdef() which throws "cache lookup failed" error without code > >> changes and for every DDL statement when we try it separately. > >> > >> My earlier comment or the test by Man might have misled you into > >> thinking that we need to drop properties or labels which are defined > >> multiple times so that we test that the dependency error does not > >> trigger when a property or a label is not orphaned. Sorry if that's > >> the case. I don't think that's the goal here. Further, such tests > >> require additional DDL to restore property graph state and also change > >> the view definition produced by pg_get_viewdef(). So I used DDLs that > >> drop properties or labels which are defined only once. +1 for this change, we don't need to re-add the label and property to myshop. > >> > >> I shortened the commit message by taking essential elements from your > >> commit message. > >> > >> Please review the attached patch. > > > > > > The attached patch seems not for this thread. > > Thanks for noticing. Here's attached correct patch. The patch LGTM, thanks for taking care of this. > > -- > Best Wishes, > Ashutosh Bapat -- Regards Junwang Zhao ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-05-18 06:26 Ayush Tiwari <[email protected]> parent: Junwang Zhao <[email protected]> 0 siblings, 1 reply; 17+ messages in thread From: Ayush Tiwari @ 2026-05-18 06:26 UTC (permalink / raw) To: Junwang Zhao <[email protected]>; +Cc: Ashutosh Bapat <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> Hi, On Mon, 18 May 2026 at 07:37, Junwang Zhao <[email protected]> wrote: > On Mon, May 18, 2026 at 8:22 AM Ashutosh Bapat > <[email protected]> wrote: > > > >> > > >> I shortened the commit message by taking essential elements from your > > >> commit message. > > >> > > >> Please review the attached patch. > > > > > > > > > The attached patch seems not for this thread. > > > > Thanks for noticing. Here's attached correct patch. > > The patch LGTM, thanks for taking care of this. +1, LGTM I've changed the status in CF entry status to Ready for Committer. https://commitfest.postgresql.org/patch/6760/ Regards, Ayush ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-05-21 18:37 Ashutosh Bapat <[email protected]> parent: Ayush Tiwari <[email protected]> 0 siblings, 1 reply; 17+ messages in thread From: Ashutosh Bapat @ 2026-05-21 18:37 UTC (permalink / raw) To: Ayush Tiwari <[email protected]>; +Cc: Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> On Sun, May 17, 2026 at 11:26 PM Ayush Tiwari <[email protected]> wrote: > > Hi, > > On Mon, 18 May 2026 at 07:37, Junwang Zhao <[email protected]> wrote: >> >> On Mon, May 18, 2026 at 8:22 AM Ashutosh Bapat >> <[email protected]> wrote: >> >> > >> >> > >> I shortened the commit message by taking essential elements from your >> > >> commit message. >> > >> >> > >> Please review the attached patch. >> > > >> > > >> > > The attached patch seems not for this thread. >> > >> > Thanks for noticing. Here's attached correct patch. >> >> The patch LGTM, thanks for taking care of this. > > > +1, LGTM > > I've changed the status in CF entry status to Ready for Committer. > https://commitfest.postgresql.org/patch/6760/ Thanks Ayush. While working on this, I found two other problems. Before we dive into those, I think we should commit the current patch. It need not wait for a solution to these problems. I was checking whether we should be expanding an empty label during the transformation stage instead of rewrite phase so that, in case the graph pattern is part of a view definition, the set of labels the empty label expression resolves to is stored in the catalogs. That way, we can create a dependency of view on the set of labels at the time of view. After discussing it with Peter Eisentraut, we feel that doing so will be good for future-proofing pg_dump of views containing graph patterns with empty labels. We will definitely need it before supporting all properties reference since the properties that the all properties reference expands to depends upon the set of labels in the label expression. We need to create dependencies between those properties and the view and hence need dependencies between labels (that the empty label expression resolves to) and the view. I will provide a patch for the same soon. When trying different things which could lead to invalidation of view if we don't add the dependency between labels (that the empty label expression resolves to) and the view, I found another way to invalidate the view. To reproduce it, run the following statements after running graph_table.sql tests. CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); BEGIN; ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; SELECT * FROM v_empty_label; ROLLBACK; The three ALTER TABLE statements leave the label behind since it's associated with the edge tables. The SELECT statement still throws "ERROR: property "elname" for element variable "v" not found". If we expand the empty label reference in the transformation phase and record dependencies between those labels and the view, we get a different error "ERROR: no property graph element of type "vertex" has label "l1" associated with it in property graph "g1"". The first error is a bit obscure compared to the second one. So there's relative improvement. Myself and Peter thought that the second error is an artifact of the term "vertex labels" in the standard; a term which is not properly defined in the standard. There are two solutions 1. We create vertex label and edge label as separate objects and record depdencies accordingly 2. We ignore the term "vertex/edge label" in the standard and handle labels that exist in property graphs but have no associated element of required type in the query. I think this will require some corrections in standard to standardize the outcome of such queries. I will provide patches once we decide which approach to take. -- Best Wishes, Ashutosh Bapat ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-01 11:37 Ashutosh Bapat <[email protected]> parent: Ashutosh Bapat <[email protected]> 0 siblings, 3 replies; 17+ messages in thread From: Ashutosh Bapat @ 2026-06-01 11:37 UTC (permalink / raw) To: Ayush Tiwari <[email protected]>; +Cc: Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> On Fri, May 22, 2026 at 12:07 AM Ashutosh Bapat <[email protected]> wrote: > > On Sun, May 17, 2026 at 11:26 PM Ayush Tiwari > <[email protected]> wrote: > > > > Hi, > > > > On Mon, 18 May 2026 at 07:37, Junwang Zhao <[email protected]> wrote: > >> > >> On Mon, May 18, 2026 at 8:22 AM Ashutosh Bapat > >> <[email protected]> wrote: > >> > >> > >> > >> > >> I shortened the commit message by taking essential elements from your > >> > >> commit message. > >> > >> > >> > >> Please review the attached patch. > >> > > > >> > > > >> > > The attached patch seems not for this thread. > >> > > >> > Thanks for noticing. Here's attached correct patch. > >> > >> The patch LGTM, thanks for taking care of this. > > > > > > +1, LGTM > > > > I've changed the status in CF entry status to Ready for Committer. > > https://commitfest.postgresql.org/patch/6760/ > > Thanks Ayush. > > While working on this, I found two other problems. Before we dive into > those, I think we should commit the current patch. It need not wait > for a solution to these problems. > > I was checking whether we should be expanding an empty label during > the transformation stage instead of rewrite phase so that, in case the > graph pattern is part of a view definition, the set of labels the > empty label expression resolves to is stored in the catalogs. That > way, we can create a dependency of view on the set of labels at the > time of view. After discussing it with Peter Eisentraut, we feel that > doing so will be good for future-proofing pg_dump of views containing > graph patterns with empty labels. We will definitely need it before > supporting all properties reference since the properties that the all > properties reference expands to depends upon the set of labels in the > label expression. We need to create dependencies between those > properties and the view and hence need dependencies between labels > (that the empty label expression resolves to) and the view. I will > provide a patch for the same soon. While working on this I found another issue. An empty label expression may turn into no label which does not have any syntax level support. Hence we can't dump a view containing such an empty label expression. For now I have prohibited empty label expressions which do not resolve to any label as an unsupported feature. I doubt if there will be any valid use case for such a label expression. As a result a test which tests dependency between a view and the property graph in create_property_graph.sql failed. Failure is accidental as test uses an empty property graph for this purpose. I fixed it by adding a vertex table to the property graph being referenced in the view. The commit message just mentions the label dependency asymmetry between explicitly mentioned labels and labels resulting from empty label expression. I haven't gone into the details of all-properties-reference to keep it simple. The discussion link should lead a curious reader to the complex discussion here. I did reconsider whether to fix this right now or whether we can postpone it till we support all-properties reference. If we don't fix this, a query containing empty label expression will return different result as the set of labels in the property graph changes. When we will support all properties reference, we will have to fix this and then the behaviour will change breaking backward compatibility. So I think we have to fix this now. > > When trying different things which could lead to invalidation of view > if we don't add the dependency between labels (that the empty label > expression resolves to) and the view, I found another way to > invalidate the view. > > To reproduce it, run the following statements after running > graph_table.sql tests. > CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v > WHERE v.vprop1 = 10) COLUMNS (v.elname)); > BEGIN; > ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; > ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; > ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; > SELECT * FROM v_empty_label; > ROLLBACK; > > The three ALTER TABLE statements leave the label behind since it's > associated with the edge tables. The SELECT statement still throws > "ERROR: property "elname" for element variable "v" not found". If we > expand the empty label reference in the transformation phase and > record dependencies between those labels and the view, we get a > different error "ERROR: no property graph element of type "vertex" > has label "l1" associated with it in property graph "g1"". The first > error is a bit obscure compared to the second one. So there's relative > improvement. Myself and Peter thought that the second error is an > artifact of the term "vertex labels" in the standard; a term which is > not properly defined in the standard. There are two solutions > > 1. We create vertex label and edge label as separate objects and > record depdencies accordingly > 2. We ignore the term "vertex/edge label" in the standard and handle > labels that exist in property graphs but have no associated element of > required type in the query. I think this will require some corrections > in standard to standardize the outcome of such queries. > > I will provide patches once we decide which approach to take. For now I have added a test case exhibiting this behaviour. If we commit the test, somebody who comes across this behaviour will know why the current behaviour is the way it is and what's the way forward. But if we don't commit it, which is ok, they will consider this as buggy behaviour and report it. I am fine either way. As I mentioned earlier, I am creating patches for all outstanding issues from the same branch. So the patches attached here do not start from 0001, but they apply with git am on the latest master for me without any problem. 0005 - fixes `cache lookup failed for label` 0006 - fixes empty label expression and view behaviour 0007 - issue with labels shared by vertex and edges -- Best Wishes, Ashutosh Bapat Attachments: [text/x-patch] v20260601-0007-View-referencing-labels-shared-by-vertex-a.patch (4.6K, ../../CAExHW5sghW3VjxyQYwYaW4Q76ZEcrsCwd_AYcfPHKZFMWOHKrw@mail.gmail.com/2-v20260601-0007-View-referencing-labels-shared-by-vertex-a.patch) download | inline diff: From e72095baf87d7e28c162c0835e0f5f03854eeb63 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 28 May 2026 02:47:33 +0530 Subject: [PATCH v20260601 7/7] View referencing labels shared by vertex and edge tables While at it add a test for view containing labels which are shared by both vertex and edge tables. When such a label is dropped from only vertex tables or only edge tables, the view may be rendered invalid because properties only associated with that label can not be resolved. The fix will need to wait for the SQL/PGQ standard to specify the behaviour in such a case. Author: Ashutosh Bapat <[email protected]> --- src/test/regress/expected/graph_table.out | 20 ++++++++++++++++++++ src/test/regress/sql/graph_table.sql | 13 +++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index cfee0626fdb..475ac20d376 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -973,6 +973,19 @@ ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error ERROR: cannot drop label vl2 of property graph g1 because other objects depend on it DETAIL: view v_empty_label depends on label vl2 of property graph g1 HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- l1 is shared by all vertex tables and edge tables. Dropping it from all +-- vertex tables only renders a view unusable. This is because the standard +-- differentiates between a vertex label and an edge label even though they +-- share the same name. Waiting for the standard to clarify the expected +-- behavior in this case. +CREATE VIEW v_shared_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname)); +BEGIN; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; +SELECT * FROM v_shared_label; +ERROR: no property graph element of type "vertex" has label "l1" associated with it in property graph "g1" +ROLLBACK; -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); pg_get_viewdef @@ -993,6 +1006,13 @@ SELECT pg_get_viewdef('v_empty_label'::regclass); FROM GRAPH_TABLE (g1 MATCH (v IS l1|vl1|vl2|vl3 WHERE (v.vprop1 = 10)) COLUMNS (v.elname AS elname)); (1 row) +SELECT pg_get_viewdef('v_shared_label'::regclass); + pg_get_viewdef +------------------------------------------------------------------------ + SELECT elname + + FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname AS elname)); +(1 row) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index c4cb7999857..5412f10ac3c 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -554,9 +554,22 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error +-- l1 is shared by all vertex tables and edge tables. Dropping it from all +-- vertex tables only renders a view unusable. This is because the standard +-- differentiates between a vertex label and an edge label even though they +-- share the same name. Waiting for the standard to clarify the expected +-- behavior in this case. +CREATE VIEW v_shared_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname)); +BEGIN; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; +SELECT * FROM v_shared_label; +ROLLBACK; -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); SELECT pg_get_viewdef('v_empty_label'::regclass); +SELECT pg_get_viewdef('v_shared_label'::regclass); -- test view/graph nesting -- 2.34.1 [text/x-patch] v20260601-0005-Record-dependencies-on-graph-labels-and-pr.patch (11.4K, ../../CAExHW5sghW3VjxyQYwYaW4Q76ZEcrsCwd_AYcfPHKZFMWOHKrw@mail.gmail.com/3-v20260601-0005-Record-dependencies-on-graph-labels-and-pr.patch) download | inline diff: From ba5614bcd38b96e0e455225bab3cb510ab7f151f Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Sun, 17 May 2026 12:21:13 +0530 Subject: [PATCH v20260601 5/7] Record dependencies on graph labels and properties A view definition with GRAPH_TABLE depends upon the property graph it references as well as the properties and labels referenced in it. We recorded the dependency on the property graph, but did not record dependency on labels and properties. This allowed properties or labels referenced by a view to be dropped resulting in a cache lookup error when such a view was accessed. Fix this bug by handling GraphPropertyRef and GraphLabelRef in find_expr_references_walker(). The dependency on the data type of property is not needed to be recorded separately as it is recorded indirectly via dependency on the property graph property itself. Please note that a property or a label associated with individual elements can still be dropped as long as there are other elements which are associated with that property or label since they do not lead to dropping the property or the label from the property graph altogether. Reported-by: Man Zeng <[email protected]> Author: Ayush Tiwari <[email protected]> Author: Ashutosh Bapat <[email protected]> Reviewed by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/catalog/dependency.c | 19 ++++++++++++ src/test/regress/expected/graph_table.out | 37 +++++++++++++++++------ src/test/regress/sql/graph_table.sql | 14 +++++++-- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index fdb8e67e1f5..c54774b3275 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -2165,6 +2165,25 @@ find_expr_references_walker(Node *node, add_object_address(TypeRelationId, rowexpr->row_typeid, 0, context->addrs); } + else if (IsA(node, GraphLabelRef)) + { + GraphLabelRef *glr = (GraphLabelRef *) node; + + /* GRAPH_TABLE label reference depends on the property graph label */ + add_object_address(PropgraphLabelRelationId, glr->labelid, 0, + context->addrs); + } + else if (IsA(node, GraphPropertyRef)) + { + GraphPropertyRef *gpr = (GraphPropertyRef *) node; + + /* + * GRAPH_TABLE property reference depends on the property graph + * property + */ + add_object_address(PropgraphPropertyRelationId, gpr->propid, 0, + context->addrs); + } else if (IsA(node, RowCompareExpr)) { RowCompareExpr *rcexpr = (RowCompareExpr *) node; diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index e8d49fd5cd4..a5f9f0ac90a 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -929,7 +929,7 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e 30 | 300 | 10 (2 rows) --- ruleutils reverse parsing +-- GRAPH_TABLE in views -- The query in the view definition is intentionally complex to test one view with many -- features like label disjunction, lateral references, WHERE clauses in graph -- patterns. @@ -938,16 +938,35 @@ SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) - COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g + COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; +-- Dropping properties or labels used by a view is not allowed +-- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup +-- error. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error +ERROR: cannot drop label orders of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on label orders of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers + ALTER LABEL customers DROP PROPERTIES (address); -- error +ERROR: cannot drop property address of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on property address of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products + ALTER LABEL products DROP PROPERTIES (price); -- error +ERROR: cannot drop property price of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on property price of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); - pg_get_viewdef ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- - SELECT g.customer_name, + - g.product_name, + - g.a + - FROM x1, + - GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g+ + pg_get_viewdef +---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + SELECT g.customer_name, + + g.product_name, + + g.price, + + g.a + + FROM x1, + + GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, p.price AS price, x1.a AS a)) g+ ORDER BY g.customer_name, g.product_name; (1 row) diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index e761f09e057..80576b2f9f4 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -525,7 +525,8 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s IS ptnv)-[e IS ptne]->(d IS ptnv) COLUMNS SELECT * FROM GRAPH_TABLE (g4 MATCH (s)-[e]-(d) WHERE s.id = 3 COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; --- ruleutils reverse parsing +-- GRAPH_TABLE in views + -- The query in the view definition is intentionally complex to test one view with many -- features like label disjunction, lateral references, WHERE clauses in graph -- patterns. @@ -534,8 +535,17 @@ SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) - COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g + COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; +-- Dropping properties or labels used by a view is not allowed +-- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup +-- error. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers + ALTER LABEL customers DROP PROPERTIES (address); -- error +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products + ALTER LABEL products DROP PROPERTIES (price); -- error +-- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); -- test view/graph nesting -- 2.34.1 [text/x-patch] v20260601-0006-Empty-label-expression-in-view-definition.patch (20.1K, ../../CAExHW5sghW3VjxyQYwYaW4Q76ZEcrsCwd_AYcfPHKZFMWOHKrw@mail.gmail.com/4-v20260601-0006-Empty-label-expression-in-view-definition.patch) download | inline diff: From db93ea38da965fa2503ef528520076378da70cc7 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 28 May 2026 02:45:58 +0530 Subject: [PATCH v20260601 6/7] Empty label expression in view definition A view definition depends on all the graph labels that are explicitly mentioned in the graph patterns in it so as to avoid it being rendered invalid when any of the labels is dropped. But when a view definition contains an empty label expression, we do not create any dependency between the view and the labels that the empty label expression resolves to. Resolve an empty label expression during transformation phase so that we can create dependency between those labels and the view. This will further help to avoid invalidation of a view containing all-properties references when we support it. An empty label expression may resolve to an empty set of labels if there are not labels associated with the elements matching the kind of element pattern containing the label expression. We do not have a syntax level support for a label expression containing no labels. Hence we can not dump a view containing such an empty label expression. Throw an error when a query contains such an empty label expression. There are possibly no real usecases which use such queries. Author: Ashutosh Bapat <[email protected]> Discussion: https://www.postgresql.org/message-id/CAExHW5twGP5Zuk4Zch4kz8XDrSpckWQipMs=ysAj8GmqNa2FCQ@mail.gmail.com --- src/backend/parser/parse_graphtable.c | 110 +++++++++++++++++- src/backend/rewrite/rewriteGraphTable.c | 78 ++++--------- src/include/nodes/parsenodes.h | 8 ++ .../expected/create_property_graph.out | 1 + src/test/regress/expected/graph_table.out | 29 ++++- .../regress/sql/create_property_graph.sql | 1 + src/test/regress/sql/graph_table.sql | 16 ++- 7 files changed, 178 insertions(+), 65 deletions(-) diff --git a/src/backend/parser/parse_graphtable.c b/src/backend/parser/parse_graphtable.c index 73fbfb541f7..5c9da75c8cf 100644 --- a/src/backend/parser/parse_graphtable.c +++ b/src/backend/parser/parse_graphtable.c @@ -18,6 +18,8 @@ #include "access/genam.h" #include "access/htup_details.h" #include "access/table.h" +#include "catalog/pg_propgraph_element.h" +#include "catalog/pg_propgraph_element_label.h" #include "catalog/pg_propgraph_label.h" #include "catalog/pg_propgraph_property.h" #include "miscadmin.h" @@ -151,6 +153,51 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) return NULL; } +/* + * Given the OID of a label and the kind of graph element pattern, return true if + * there exists at least one element matching the given kind associated with the + * label. Otherwise return false. + */ +static bool +label_has_elements_of_kind(Oid labelid, GraphElementPatternKind gepkind) +{ + Relation rel; + SysScanDesc scan; + ScanKeyData key[1]; + HeapTuple tup; + bool result = false; + + rel = table_open(PropgraphElementLabelRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_propgraph_element_label_pgellabelid, + BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(labelid)); + scan = systable_beginscan(rel, PropgraphElementLabelLabelIndexId, + true, NULL, 1, key); + while (!result && HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_propgraph_element_label element_label = (Form_pg_propgraph_element_label) GETSTRUCT(tup); + Oid element_oid = element_label->pgelelid; + HeapTuple element_tup = SearchSysCache1(PROPGRAPHELOID, ObjectIdGetDatum(element_oid)); + Form_pg_propgraph_element element_form; + + if (!HeapTupleIsValid(element_tup)) + elog(ERROR, "cache lookup failed for property graph element %u", element_oid); + + element_form = (Form_pg_propgraph_element) GETSTRUCT(element_tup); + + if ((element_form->pgekind == PGEKIND_VERTEX && gepkind == VERTEX_PATTERN) || + (element_form->pgekind == PGEKIND_EDGE && IS_EDGE_PATTERN(gepkind))) + result = true; + + ReleaseSysCache(element_tup); + } + + systable_endscan(scan); + table_close(rel, AccessShareLock); + return result; +} + /* * Transform a label expression. * @@ -161,14 +208,66 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) * GraphLabelRef nodes corresponding to the names of the labels appearing in the * expression. If any label name cannot be resolved to a label in the property * graph, an error is raised. + * + * An empty label expression is treated as a special case. According to section + * 9.2 "Contextual inference of a set of labels" subclause 2.a.ii of SQL/PGQ + * standard, element pattern which does not have a label expression is + * considered to have label expression equivalent to '%|!%' which is set of all + * labels which have at least one element of the given element kind associated with it. */ static Node * -transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr) +transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr, GraphElementPatternKind gepkind) { Node *result; - if (labelexpr == NULL) - return NULL; + if (!labelexpr) + { + Relation rel; + SysScanDesc scan; + ScanKeyData key[1]; + HeapTuple tup; + List *args = NIL; + + rel = table_open(PropgraphLabelRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_propgraph_label_pglpgid, + BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(gpstate->graphid)); + scan = systable_beginscan(rel, PropgraphLabelGraphNameIndexId, + true, NULL, 1, key); + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_propgraph_label label = (Form_pg_propgraph_label) GETSTRUCT(tup); + GraphLabelRef *lref; + + if (!label_has_elements_of_kind(label->oid, gepkind)) + continue; + + lref = makeNode(GraphLabelRef); + lref->labelid = label->oid; + lref->location = -1; + args = lappend(args, lref); + } + systable_endscan(scan); + table_close(rel, AccessShareLock); + + /* + * If there are no labels with elements of the given kind, the set of + * labels that this label expression resolves to is empty. There is no + * way to represent an empty set of labels as a label expression since + * we do not support label conjunction as well as negation. So we can + * not dump a view containing such a label expression. Hence prohibit + * it for now. + */ + if (!args) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("empty label expression does not resolve to any label")); + + result = (Node *) makeBoolExpr(OR_EXPR, args, -1); + return result; + + } check_stack_depth(); @@ -208,7 +307,7 @@ transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr) { Node *arg = (Node *) lfirst(lc); - arg = transformLabelExpr(gpstate, arg); + arg = transformLabelExpr(gpstate, arg, gepkind); args = lappend(args, arg); } @@ -249,7 +348,8 @@ transformGraphElementPattern(ParseState *pstate, GraphElementPattern *gep) gpstate->cur_gep = gep; - gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr); + gep->has_empty_labelexpr = !gep->labelexpr; + gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr, gep->kind); gep->whereClause = transformExpr(pstate, gep->whereClause, EXPR_KIND_WHERE); diff --git a/src/backend/rewrite/rewriteGraphTable.c b/src/backend/rewrite/rewriteGraphTable.c index 33d4e866d74..2292ca6dcee 100644 --- a/src/backend/rewrite/rewriteGraphTable.c +++ b/src/backend/rewrite/rewriteGraphTable.c @@ -58,6 +58,8 @@ struct path_factor { GraphElementPatternKind kind; const char *variable; + bool has_empty_labelexpr; /* Copied from the corresponding + * GraphElementPattern */ Node *labelexpr; Node *whereClause; int factorpos; /* Position of this path factor in the list of @@ -221,9 +223,12 @@ generate_queries_for_path_pattern(RangeTblEntry *rte, List *path_pattern) * expression itself. Hence if only one of the two element * patterns has a label expression use that expression. */ - if (!other->labelexpr) + if (other->has_empty_labelexpr) + { other->labelexpr = gep->labelexpr; - else if (gep->labelexpr && !equal(other->labelexpr, gep->labelexpr)) + other->has_empty_labelexpr = gep->has_empty_labelexpr; + } + else if (!gep->has_empty_labelexpr && !equal(other->labelexpr, gep->labelexpr)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("element patterns with same variable name \"%s\" but different label expressions are not supported", @@ -250,6 +255,7 @@ generate_queries_for_path_pattern(RangeTblEntry *rte, List *path_pattern) pf = palloc0_object(struct path_factor); pf->factorpos = factorpos++; pf->kind = gep->kind; + pf->has_empty_labelexpr = gep->has_empty_labelexpr; pf->labelexpr = gep->labelexpr; pf->variable = gep->variable; pf->whereClause = gep->whereClause; @@ -823,37 +829,9 @@ get_labels_for_expr(Oid propgraphid, Node *labelexpr) { List *label_oids; - if (!labelexpr) - { - Relation rel; - SysScanDesc scan; - ScanKeyData key[1]; - HeapTuple tup; - - /* - * According to section 9.2 "Contextual inference of a set of labels" - * subclause 2.a.ii of SQL/PGQ standard, element pattern which does - * not have a label expression is considered to have label expression - * equivalent to '%|!%' which is set of all labels. - */ - label_oids = NIL; - rel = table_open(PropgraphLabelRelationId, AccessShareLock); - ScanKeyInit(&key[0], - Anum_pg_propgraph_label_pglpgid, - BTEqualStrategyNumber, - F_OIDEQ, ObjectIdGetDatum(propgraphid)); - scan = systable_beginscan(rel, PropgraphLabelGraphNameIndexId, - true, NULL, 1, key); - while (HeapTupleIsValid(tup = systable_getnext(scan))) - { - Form_pg_propgraph_label label = (Form_pg_propgraph_label) GETSTRUCT(tup); + Assert(labelexpr); - label_oids = lappend_oid(label_oids, label->oid); - } - systable_endscan(scan); - table_close(rel, AccessShareLock); - } - else if (IsA(labelexpr, GraphLabelRef)) + if (IsA(labelexpr, GraphLabelRef)) { GraphLabelRef *glr = castNode(GraphLabelRef, labelexpr); @@ -903,7 +881,6 @@ get_path_elements_for_path_factor(Oid propgraphid, struct path_factor *pf) List *elem_oids_seen = NIL; List *pf_elem_oids = NIL; List *path_elements = NIL; - List *unresolved_labels = NIL; Relation rel; SysScanDesc scan; ScanKeyData key[1]; @@ -970,33 +947,26 @@ get_path_elements_for_path_factor(Oid propgraphid, struct path_factor *pf) { /* * We did not find any qualified element associated with this - * label. The label or its properties can not be associated with - * the given element pattern. Throw an error if the label was - * explicitly specified in the element pattern. Otherwise remember - * it for later use. + * label. Throw an error. + * + * An empty label expression is replaced by all labels that are + * associated with at least one element of the required kind. We + * should not reach here in that case. */ - if (!pf->labelexpr) - unresolved_labels = lappend_oid(unresolved_labels, labeloid); - else - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("no property graph element of type \"%s\" has label \"%s\" associated with it in property graph \"%s\"", - pf->kind == VERTEX_PATTERN ? "vertex" : "edge", - get_propgraph_label_name(labeloid), - get_rel_name(propgraphid)))); + Assert(!pf->has_empty_labelexpr); + + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("no property graph element of type \"%s\" has label \"%s\" associated with it in property graph \"%s\"", + pf->kind == VERTEX_PATTERN ? "vertex" : "edge", + get_propgraph_label_name(labeloid), + get_rel_name(propgraphid)))); } systable_endscan(scan); } table_close(rel, AccessShareLock); - - /* - * Remove the labels which were not explicitly mentioned in the label - * expression but do not have any qualified elements associated with them. - * Properties associated with such labels may not be referenced. See - * replace_property_refs_mutator() for more details. - */ - pf->labeloids = list_difference_oid(label_oids, unresolved_labels); + pf->labeloids = label_oids; return path_elements; } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 91377a6cde3..71040261375 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1048,7 +1048,15 @@ typedef struct GraphElementPattern NodeTag type; GraphElementPatternKind kind; const char *variable; + + /* + * If no label expression is specified, we will replace it with a non-NULL + * expression in transformLabelExpr(). This flag indicates whether the + * label expression was originally empty. + */ + bool has_empty_labelexpr; Node *labelexpr; + List *subexpr; Node *whereClause; List *quantifier; diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out index 65508223f54..a1a5501dd52 100644 --- a/src/test/regress/expected/create_property_graph.out +++ b/src/test/regress/expected/create_property_graph.out @@ -939,6 +939,7 @@ DETAIL: Table "v2tmp" is a temporary table. DROP TABLE g2; -- error: wrong object type ERROR: "g2" is not a table HINT: Use DROP PROPERTY GRAPH to remove a property graph. +ALTER PROPERTY GRAPH g1 ADD VERTEX TABLES (t1 KEY (a)); -- to make a valid graph query CREATE VIEW vg1 AS SELECT * FROM GRAPH_TABLE(g1 MATCH () COLUMNS (1 AS one)); DROP PROPERTY GRAPH g1; -- error ERROR: cannot drop property graph g1 because other objects depend on it diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index a5f9f0ac90a..cfee0626fdb 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -844,6 +844,8 @@ EXECUTE loopstmt; (2 rows) -- inheritance and partitioning +-- +-- Also test temporary property graphs, keywords NODE and RELATIONSHIP CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); CREATE TABLE cv2 () INHERITS (pv); @@ -873,7 +875,6 @@ SELECT * FROM GRAPH_TABLE (g3 MATCH (s IS pv)-[e IS pe]->(d IS pv) COLUMNS (s.va 30 | 300 | 10 (3 rows) --- temporary property graph CREATE TEMPORARY PROPERTY GRAPH gtmp VERTEX TABLES ( pv KEY (id) @@ -900,8 +901,12 @@ CREATE TABLE ptne1 PARTITION OF ptne FOR VALUES IN (1, 2); CREATE TABLE ptne2 PARTITION OF ptne FOR VALUES IN (3); INSERT INTO ptne VALUES (1, 1, 2, 100), (2, 2, 3, 200), (3, 3, 1, 300); CREATE PROPERTY GRAPH g4 - VERTEX TABLES (ptnv) - EDGE TABLES ( + VERTEX TABLES (ptnv); +-- empty label expression which resolves to no labels +SELECT * FROM GRAPH_TABLE (g4 MATCH (s is ptnv)-[e]-(d is ptnv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; -- error +ERROR: empty label expression does not resolve to any label +ALTER PROPERTY GRAPH g4 + ADD EDGE TABLES ( ptne SOURCE KEY (src) REFERENCES ptnv(id) DESTINATION KEY (dest) REFERENCES ptnv(id) @@ -957,6 +962,17 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ERROR: cannot drop property price of property graph myshop because other objects depend on it DETAIL: view customers_us depends on property price of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- Empty label expression creates a dependency between the view and the set of +-- labels it resolves to +CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error +ERROR: cannot drop label vl1 of property graph g1 because other objects depend on it +DETAIL: view v_empty_label depends on label vl1 of property graph g1 +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error +ERROR: cannot drop label vl2 of property graph g1 because other objects depend on it +DETAIL: view v_empty_label depends on label vl2 of property graph g1 +HINT: Use DROP ... CASCADE to drop the dependent objects too. -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); pg_get_viewdef @@ -970,6 +986,13 @@ SELECT pg_get_viewdef('customers_us'::regclass); ORDER BY g.customer_name, g.product_name; (1 row) +SELECT pg_get_viewdef('v_empty_label'::regclass); + pg_get_viewdef +---------------------------------------------------------------------------------------------------------- + SELECT elname + + FROM GRAPH_TABLE (g1 MATCH (v IS l1|vl1|vl2|vl3 WHERE (v.vprop1 = 10)) COLUMNS (v.elname AS elname)); +(1 row) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql index e8b3b3ff94c..0c3aa81e9ca 100644 --- a/src/test/regress/sql/create_property_graph.sql +++ b/src/test/regress/sql/create_property_graph.sql @@ -364,6 +364,7 @@ ALTER PROPERTY GRAPH g1 -- DROP, ALTER SET SCHEMA, ALTER PROPERTY GRAPH RENAME TO DROP TABLE g2; -- error: wrong object type +ALTER PROPERTY GRAPH g1 ADD VERTEX TABLES (t1 KEY (a)); -- to make a valid graph query CREATE VIEW vg1 AS SELECT * FROM GRAPH_TABLE(g1 MATCH () COLUMNS (1 AS one)); DROP PROPERTY GRAPH g1; -- error ALTER PROPERTY GRAPH g1 SET SCHEMA create_property_graph_tests_2; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 80576b2f9f4..c4cb7999857 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -471,6 +471,8 @@ ALTER PROPERTY GRAPH g1 ALTER EDGE TABLE e3_3 ALTER LABEL l2 ADD PROPERTIES ((en EXECUTE loopstmt; -- inheritance and partitioning +-- +-- Also test temporary property graphs, keywords NODE and RELATIONSHIP CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); CREATE TABLE cv2 () INHERITS (pv); @@ -493,7 +495,6 @@ CREATE PROPERTY GRAPH g3 DESTINATION KEY(dest) REFERENCES pv(id) ); SELECT * FROM GRAPH_TABLE (g3 MATCH (s IS pv)-[e IS pe]->(d IS pv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; --- temporary property graph CREATE TEMPORARY PROPERTY GRAPH gtmp VERTEX TABLES ( pv KEY (id) @@ -514,8 +515,11 @@ CREATE TABLE ptne1 PARTITION OF ptne FOR VALUES IN (1, 2); CREATE TABLE ptne2 PARTITION OF ptne FOR VALUES IN (3); INSERT INTO ptne VALUES (1, 1, 2, 100), (2, 2, 3, 200), (3, 3, 1, 300); CREATE PROPERTY GRAPH g4 - VERTEX TABLES (ptnv) - EDGE TABLES ( + VERTEX TABLES (ptnv); +-- empty label expression which resolves to no labels +SELECT * FROM GRAPH_TABLE (g4 MATCH (s is ptnv)-[e]-(d is ptnv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; -- error +ALTER PROPERTY GRAPH g4 + ADD EDGE TABLES ( ptne SOURCE KEY (src) REFERENCES ptnv(id) DESTINATION KEY (dest) REFERENCES ptnv(id) @@ -545,8 +549,14 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers ALTER LABEL customers DROP PROPERTIES (address); -- error ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ALTER LABEL products DROP PROPERTIES (price); -- error +-- Empty label expression creates a dependency between the view and the set of +-- labels it resolves to +CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); +SELECT pg_get_viewdef('v_empty_label'::regclass); -- test view/graph nesting -- 2.34.1 ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-04 11:21 Ewan Young <[email protected]> parent: Ashutosh Bapat <[email protected]> 2 siblings, 2 replies; 17+ messages in thread From: Ewan Young @ 2026-06-04 11:21 UTC (permalink / raw) To: Ashutosh Bapat <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> On Thu, Jun 4, 2026 at 6:45 PM Ashutosh Bapat <[email protected]> wrote: > > On Fri, May 22, 2026 at 12:07 AM Ashutosh Bapat > <[email protected]> wrote: > > > > On Sun, May 17, 2026 at 11:26 PM Ayush Tiwari > > <[email protected]> wrote: > > > > > > Hi, > > > > > > On Mon, 18 May 2026 at 07:37, Junwang Zhao <[email protected]> wrote: > > >> > > >> On Mon, May 18, 2026 at 8:22 AM Ashutosh Bapat > > >> <[email protected]> wrote: > > >> > > >> > >> > > >> > >> I shortened the commit message by taking essential elements from your > > >> > >> commit message. > > >> > >> > > >> > >> Please review the attached patch. > > >> > > > > >> > > > > >> > > The attached patch seems not for this thread. > > >> > > > >> > Thanks for noticing. Here's attached correct patch. > > >> > > >> The patch LGTM, thanks for taking care of this. > > > > > > > > > +1, LGTM > > > > > > I've changed the status in CF entry status to Ready for Committer. > > > https://commitfest.postgresql.org/patch/6760/ > > > > Thanks Ayush. > > > > While working on this, I found two other problems. Before we dive into > > those, I think we should commit the current patch. It need not wait > > for a solution to these problems. > > > > I was checking whether we should be expanding an empty label during > > the transformation stage instead of rewrite phase so that, in case the > > graph pattern is part of a view definition, the set of labels the > > empty label expression resolves to is stored in the catalogs. That > > way, we can create a dependency of view on the set of labels at the > > time of view. After discussing it with Peter Eisentraut, we feel that > > doing so will be good for future-proofing pg_dump of views containing > > graph patterns with empty labels. We will definitely need it before > > supporting all properties reference since the properties that the all > > properties reference expands to depends upon the set of labels in the > > label expression. We need to create dependencies between those > > properties and the view and hence need dependencies between labels > > (that the empty label expression resolves to) and the view. I will > > provide a patch for the same soon. > > While working on this I found another issue. An empty label expression > may turn into no label which does not have any syntax level support. > Hence we can't dump a view containing such an empty label expression. > For now I have prohibited empty label expressions which do not resolve > to any label as an unsupported feature. I doubt if there will be any > valid use case for such a label expression. > > As a result a test which tests dependency between a view and the > property graph in create_property_graph.sql failed. Failure is > accidental as test uses an empty property graph for this purpose. I > fixed it by adding a vertex table to the property graph being > referenced in the view. > > The commit message just mentions the label dependency asymmetry > between explicitly mentioned labels and labels resulting from empty > label expression. I haven't gone into the details of > all-properties-reference to keep it simple. The discussion link should > lead a curious reader to the complex discussion here. > > I did reconsider whether to fix this right now or whether we can > postpone it till we support all-properties reference. If we don't fix > this, a query containing empty label expression will return different > result as the set of labels in the property graph changes. When we > will support all properties reference, we will have to fix this and > then the behaviour will change breaking backward compatibility. So I > think we have to fix this now. > > > > > When trying different things which could lead to invalidation of view > > if we don't add the dependency between labels (that the empty label > > expression resolves to) and the view, I found another way to > > invalidate the view. > > > > To reproduce it, run the following statements after running > > graph_table.sql tests. > > CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v > > WHERE v.vprop1 = 10) COLUMNS (v.elname)); > > BEGIN; > > ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; > > ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; > > ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; > > SELECT * FROM v_empty_label; > > ROLLBACK; > > > > The three ALTER TABLE statements leave the label behind since it's > > associated with the edge tables. The SELECT statement still throws > > "ERROR: property "elname" for element variable "v" not found". If we > > expand the empty label reference in the transformation phase and > > record dependencies between those labels and the view, we get a > > different error "ERROR: no property graph element of type "vertex" > > has label "l1" associated with it in property graph "g1"". The first > > error is a bit obscure compared to the second one. So there's relative > > improvement. Myself and Peter thought that the second error is an > > artifact of the term "vertex labels" in the standard; a term which is > > not properly defined in the standard. There are two solutions > > > > 1. We create vertex label and edge label as separate objects and > > record depdencies accordingly > > 2. We ignore the term "vertex/edge label" in the standard and handle > > labels that exist in property graphs but have no associated element of > > required type in the query. I think this will require some corrections > > in standard to standardize the outcome of such queries. > > > > I will provide patches once we decide which approach to take. > > For now I have added a test case exhibiting this behaviour. If we > commit the test, somebody who comes across this behaviour will know > why the current behaviour is the way it is and what's the way forward. > But if we don't commit it, which is ok, they will consider this as > buggy behaviour and report it. I am fine either way. > > As I mentioned earlier, I am creating patches for all outstanding > issues from the same branch. So the patches attached here do not start > from 0001, but they apply with git am on the latest master for me > without any problem. > 0005 - fixes `cache lookup failed for label` > 0006 - fixes empty label expression and view behaviour > 0007 - issue with labels shared by vertex and edges > > -- > Best Wishes, > Ashutosh Bapat Hi Ashutosh, Following up here from [1], where I was redirected to this thread, thanks. I don't think these are the same bug. I applied v20260601-0005 through 0007 on top of master (90354030b8f) and my test case still fails the same way: CREATE TABLE v1 (id int); CREATE TABLE v2 (id int); CREATE TABLE x (a int); CREATE PROPERTY GRAPH g VERTEX TABLES (v1 KEY (id) LABEL l1, v2 KEY (id) LABEL l2); SELECT 1 FROM x, GRAPH_TABLE (g MATCH (s IS l1|l2 WHERE s.id = x.a) COLUMNS (s.id)); ERROR: plan should not reference subplan's variable I also tested the earlier v20260517 patches (which I understand are what the unattached 0001-0004 of your series correspond to): same result. That matches expectations, since everything posted in this thread touches DDL-time code (dependency.c, propgraphcmds.c), while this is a planner-time failure on a plain SELECT, with no views or DDL involved. The trigger is a lateral reference into the GRAPH_TABLE combined with a label disjunction resolving to two different element tables. Two labels on the same table don't trigger it, since that produces a single path query and no UNION. Root cause, as far as I can tell: replace_property_refs_mutator() increments varlevelsup for lateral Vars, which is correct when the path query becomes the subquery RTE directly. With a multi-table label disjunction, generate_union_from_pathqueries() wraps the path queries as subquery RTEs of a new UNION query -- one level deeper -- and nothing compensates, so varlevelsup ends up off by one. The attached patch (same as in [1]) runs IncrementVarSublevelsUp() over each path query in the multiple-pathquery branch of generate_union_from_pathqueries(). It also adds a SELECT from the existing customers_us view to graph_table.sql: that view's comment says it exists to test exactly this lateral + disjunction combination, but it is currently only passed to pg_get_viewdef and never executed, which is why the regression tests don't catch this. The patch is against master and passes make check. On top of your v20260601 series only the test-file hunks conflict (both append to graph_table.sql); the code hunk applies cleanly. Given that, should this be tracked separately rather than in this thread? Happy to go either way. [1] https://www.postgresql.org/message-id/[email protected]... Attachments: [application/octet-stream] v1-0001-Fix-lateral-references-in-GRAPH_TABLE-with-label-.patch (3.7K, ../../CAON2xHMWgBhiBjKz2tGk0b1nQq03Fn4ESL73VUfUtVD+fuWYDQ@mail.gmail.com/2-v1-0001-Fix-lateral-references-in-GRAPH_TABLE-with-label-.patch) download | inline diff: From 1c30243a8385d5ba3c977764d0d9ee493dcd8184 Mon Sep 17 00:00:00 2001 From: Ewan Young <[email protected]> Date: Fri, 5 Jun 2026 00:07:10 +0800 Subject: [PATCH v1] Fix lateral references in GRAPH_TABLE with label disjunction When a graph pattern's label disjunction resolves to more than one element table, the rewritten path queries are combined with a UNION query, which places them one query level deeper than the GRAPH_TABLE's own subquery level. replace_property_refs() had already adjusted the varlevelsup of lateral references on the assumption that the path query would itself become the GRAPH_TABLE subquery, so such references ended up off by one, making the planner fail with ERROR: plan should not reference subplan's variable Compensate by incrementing the level of outer references in each path query once more when wrapping them in a UNION query. Also add a regression test that actually executes the customers_us view, which was created to cover exactly this combination of lateral references and label disjunction but was previously only exercised by the ruleutils deparsing test. --- src/backend/rewrite/rewriteGraphTable.c | 10 ++++++++++ src/test/regress/expected/graph_table.out | 9 +++++++++ src/test/regress/sql/graph_table.sql | 4 ++++ 3 files changed, 23 insertions(+) diff --git a/src/backend/rewrite/rewriteGraphTable.c b/src/backend/rewrite/rewriteGraphTable.c index 33d4e866d74..3d6c85a5ea8 100644 --- a/src/backend/rewrite/rewriteGraphTable.c +++ b/src/backend/rewrite/rewriteGraphTable.c @@ -638,6 +638,16 @@ generate_union_from_pathqueries(List **pathqueries) return sampleQuery; } + /* + * Each path query will be wrapped in a subquery RTE of the UNION query + * constructed below, which puts it one query level further away from the + * query containing the GRAPH_TABLE clause than replace_property_refs() + * assumed when it adjusted the levels of lateral references. Compensate + * by incrementing varlevelsup of any outer-query references once more. + */ + foreach_node(Query, pathquery, *pathqueries) + IncrementVarSublevelsUp((Node *) pathquery, 1, 1); + sostmt = castNode(SetOperationStmt, generate_setop_from_pathqueries(*pathqueries, &rtable, NULL)); diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index cc6d80afd82..924f0cff6db 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -942,6 +942,15 @@ SELECT pg_get_viewdef('customers_us'::regclass); ORDER BY g.customer_name, g.product_name; (1 row) +-- exercises lateral references combined with label disjunction, where the +-- rewritten path queries are wrapped in an extra UNION query level +SELECT * FROM customers_us; + customer_name | product_name | a +---------------+--------------+--- + customer1 | product1 | 1 + customer1 | product2 | 1 +(2 rows) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 0e381ec72bc..f8207578d32 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -536,6 +536,10 @@ SELECT g.* FROM x1, ORDER BY customer_name, product_name; SELECT pg_get_viewdef('customers_us'::regclass); +-- exercises lateral references combined with label disjunction, where the +-- rewritten path queries are wrapped in an extra UNION query level +SELECT * FROM customers_us; + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; -- 2.47.3 ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-04 11:46 Ashutosh Bapat <[email protected]> parent: Ewan Young <[email protected]> 1 sibling, 1 reply; 17+ messages in thread From: Ashutosh Bapat @ 2026-06-04 11:46 UTC (permalink / raw) To: Ewan Young <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> Hi Ewan, On Thu, Jun 4, 2026 at 4:52 PM Ewan Young <[email protected]> wrote: > > > I don't think these are the same bug. I applied v20260601-0005 > through 0007 on top of master (90354030b8f) and my test case still > fails the same way: Sorry to redirect you to the wrong thread. Please check [1]. Hope I have given you the right one this time :) When submitting the next report, please check if the issue is already covered by an existing email thread. [1] https://www.postgresql.org/message-id/[email protected]... -- Best Wishes, Ashutosh Bapat ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-04 12:44 Ewan Young <[email protected]> parent: Ashutosh Bapat <[email protected]> 0 siblings, 0 replies; 17+ messages in thread From: Ewan Young @ 2026-06-04 12:44 UTC (permalink / raw) To: Ashutosh Bapat <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> Hi Ashutosh, On Thu, Jun 4, 2026 at 7:46 PM Ashutosh Bapat <[email protected]> wrote: > > Hi Ewan, > > On Thu, Jun 4, 2026 at 4:52 PM Ewan Young <[email protected]> wrote: > > > > > > I don't think these are the same bug. I applied v20260601-0005 > > through 0007 on top of master (90354030b8f) and my test case still > > fails the same way: > > Sorry to redirect you to the wrong thread. Please check [1]. Hope I > have given you the right one this time :) > > When submitting the next report, please check if the issue is already > covered by an existing email thread. > > [1] https://www.postgresql.org/message-id/[email protected]... No problem, and thanks for the pointer — this time it is indeed the same issue. I applied v20260602-0010 on top of master (90354030b8f) and verified that it fixes my test case as well as an edge-label variant of it; the graph_table regression test also passes. The fix is essentially the same as the one I posted (adjusting varlevelsup of the path queries when they get wrapped as subqueries of the UNION), just placed in generate_setop_from_pathqueries instead, which also looks cleaner to me. So please consider my report a duplicate of that thread. I will dig the archives more carefully before reporting next time :) Regards, Ewan Young > > -- > Best Wishes, > Ashutosh Bapat ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-04 14:07 =?ISO-8859-1?B?emVuZ21hbg==?= <[email protected]> parent: Ashutosh Bapat <[email protected]> 2 siblings, 1 reply; 17+ messages in thread From: =?ISO-8859-1?B?emVuZ21hbg==?= @ 2026-06-04 14:07 UTC (permalink / raw) To: =?ISO-8859-1?B?QXNodXRvc2ggQmFwYXQ=?= <[email protected]>; +Cc: =?ISO-8859-1?B?SnVud2FuZyBaaGFv?= <[email protected]>; =?ISO-8859-1?B?cGdzcWwtaGFja2Vycw==?= <[email protected]>; =?ISO-8859-1?B?UGV0ZXIgRWlzZW50cmF1dA==?= <[email protected]>; =?ISO-8859-1?B?QXl1c2ggVGl3YXJp?= <[email protected]> > For now I have added a test case exhibiting this behaviour. If we > commit the test, somebody who comes across this behaviour will know > why the current behaviour is the way it is and what's the way forward. > But if we don't commit it, which is ok, they will consider this as > buggy behaviour and report it. I am fine either way. > > As I mentioned earlier, I am creating patches for all outstanding > issues from the same branch. So the patches attached here do not start > from 0001, but they apply with git am on the latest master for me > without any problem. > 0005 - fixes `cache lookup failed for label` > 0006 - fixes empty label expression and view behaviour > 0007 - issue with labels shared by vertex and edges Hi Ashutosh, I applied and tested the v20260601 patches on top of master, looks good. One thing I noticed though -- RemoveRelations() in propgraphcmds.c cleans up orphaned pg_propgraph_property entries, but the condition doesn't include drop_label: ```diff diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c index cc516e27020..7c8f397afbf 100644 --- a/src/backend/commands/propgraphcmds.c +++ b/src/backend/commands/propgraphcmds.c @@ -1638,7 +1638,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) } /* Remove any orphaned pg_propgraph_property entries */ - if (stmt->drop_properties || stmt->drop_vertex_tables || stmt->drop_edge_tables) + if (stmt->drop_label || stmt->drop_properties || stmt->drop_vertex_tables || stmt->drop_edge_tables) { foreach_oid(propoid, get_graph_property_ids(pgrelid)) { ``` ```sql postgres@zxm-VMware-Virtual-Platform:~/code/postgres$ psql psql (19beta1) Type "help" for help. postgres=# CREATE PROPERTY GRAPH g1 VERTEX TABLES ( v1 LABEL vl1 PROPERTIES (a, b, c) LABEL vl2 PROPERTIES (a) LABEL vl3 PROPERTIES (a,b) ); CREATE PROPERTY GRAPH postgres=# ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; ALTER PROPERTY GRAPH postgres=# SELECT oid, pgpname FROM pg_propgraph_property WHERE pgppgid = 'g1'::regclass ORDER BY pgpname; oid | pgpname --------+--------- 111971 | a 111973 | b 111975 | c (3 rows) ``` After dropping vl1, b and c are still there even though no label references them anymore. Not sure if this is worth fixing -- thoughts? -- regards, Man Zeng ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-04 15:04 Ashutosh Bapat <[email protected]> parent: =?ISO-8859-1?B?emVuZ21hbg==?= <[email protected]> 0 siblings, 1 reply; 17+ messages in thread From: Ashutosh Bapat @ 2026-06-04 15:04 UTC (permalink / raw) To: zengman <[email protected]>; +Cc: Junwang Zhao <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]>; Ayush Tiwari <[email protected]> On Thu, Jun 4, 2026 at 7:37 PM zengman <[email protected]> wrote: > > > For now I have added a test case exhibiting this behaviour. If we > > commit the test, somebody who comes across this behaviour will know > > why the current behaviour is the way it is and what's the way forward. > > But if we don't commit it, which is ok, they will consider this as > > buggy behaviour and report it. I am fine either way. > > > > As I mentioned earlier, I am creating patches for all outstanding > > issues from the same branch. So the patches attached here do not start > > from 0001, but they apply with git am on the latest master for me > > without any problem. > > 0005 - fixes `cache lookup failed for label` > > 0006 - fixes empty label expression and view behaviour > > 0007 - issue with labels shared by vertex and edges > > Hi Ashutosh, > > I applied and tested the v20260601 patches on top of master, looks good. > > One thing I noticed though -- RemoveRelations() in propgraphcmds.c > cleans up orphaned pg_propgraph_property entries, but the condition > doesn't include drop_label: > > ```diff > diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c > index cc516e27020..7c8f397afbf 100644 > --- a/src/backend/commands/propgraphcmds.c > +++ b/src/backend/commands/propgraphcmds.c > @@ -1638,7 +1638,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) > } > > /* Remove any orphaned pg_propgraph_property entries */ > - if (stmt->drop_properties || stmt->drop_vertex_tables || stmt->drop_edge_tables) > + if (stmt->drop_label || stmt->drop_properties || stmt->drop_vertex_tables || stmt->drop_edge_tables) > { > foreach_oid(propoid, get_graph_property_ids(pgrelid)) > { > ``` > > ```sql > postgres@zxm-VMware-Virtual-Platform:~/code/postgres$ psql > psql (19beta1) > Type "help" for help. > > postgres=# CREATE PROPERTY GRAPH g1 > VERTEX TABLES ( > v1 > LABEL vl1 PROPERTIES (a, b, c) > LABEL vl2 PROPERTIES (a) LABEL vl3 PROPERTIES (a,b) > ); > CREATE PROPERTY GRAPH > postgres=# ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; > ALTER PROPERTY GRAPH > postgres=# SELECT oid, pgpname FROM pg_propgraph_property WHERE pgppgid = 'g1'::regclass ORDER BY pgpname; > oid | pgpname > --------+--------- > 111971 | a > 111973 | b > 111975 | c > (3 rows) > ``` > > After dropping vl1, b and c are still there even though no label > references them anymore. Not sure if this is worth fixing -- thoughts? b won't be dropped since vl3 references it. But c should get dropped. It will be interesting to see what happens if someone tries to add a label with property c afterwards with a different data type - it will fail, I guess. It's worth fixing for that case if nothing else. We have gathered 3 issues in this thread, which depend upon each other either because of code or the test queries. Can you please start a new thread to discuss this and provide a patch fixing it? -- Best Wishes, Ashutosh Bapat ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-05 14:37 =?ISO-8859-1?B?emVuZ21hbg==?= <[email protected]> parent: Ashutosh Bapat <[email protected]> 0 siblings, 0 replies; 17+ messages in thread From: =?ISO-8859-1?B?emVuZ21hbg==?= @ 2026-06-05 14:37 UTC (permalink / raw) To: =?ISO-8859-1?B?QXNodXRvc2ggQmFwYXQ=?= <[email protected]>; +Cc: =?ISO-8859-1?B?SnVud2FuZyBaaGFv?= <[email protected]>; =?ISO-8859-1?B?cGdzcWwtaGFja2Vycw==?= <[email protected]>; =?ISO-8859-1?B?UGV0ZXIgRWlzZW50cmF1dA==?= <[email protected]>; =?ISO-8859-1?B?QXl1c2ggVGl3YXJp?= <[email protected]> > b won't be dropped since vl3 references it. But c should get dropped. > It will be interesting to see what happens if someone tries to add a > label with property c afterwards with a different data type - it will > fail, I guess. It's worth fixing for that case if nothing else. > > We have gathered 3 issues in this thread, which depend upon each other > either because of code or the test queries. Can you please start a new > thread to discuss this and provide a patch fixing it? Hi Ashutosh, You're right, I pasted the wrong test case. I've started a new thread for this issue with the correct repro and patch. Please see: https://www.postgresql.org/message-id/tencent_76F6ACA2364EAA1E5DBD7A47%40qq.com -- regards, Man Zeng ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-08 20:15 Andres Freund <[email protected]> parent: Ewan Young <[email protected]> 1 sibling, 1 reply; 17+ messages in thread From: Andres Freund @ 2026-06-08 20:15 UTC (permalink / raw) To: Ewan Young <[email protected]>; +Cc: Ashutosh Bapat <[email protected]>; Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> Hi, On 2026-06-04 19:21:48 +0800, Ewan Young wrote: > The patch is against master and passes make check. On top of your > v20260601 series only the test-file hunks conflict (both append to > graph_table.sql); the code hunk applies cleanly. Unfortunately this fails in CI: https://github.com/postgresql-cfbot/postgresql/actions/runs/27163049887 #4 0x598104c3 in ExceptionalCondition (conditionName=0x59adcb6c "var->varlevelsup > 0 && var->varlevelsup < root->query_level", fileName=0x59adcb40 "../src/backend/optimizer/util/paramassign.c", lineNumber=126) at ../src/backend/utils/error/assert.c:65 No locals. #5 0x592911f8 in replace_outer_var (root=0x5c92f418, var=0x5c92f708) at ../src/backend/optimizer/util/paramassign.c:126 retval = <optimized out> i = <optimized out> #6 0x59258569 in replace_correlation_vars_mutator (node=0x5c92f708, root=0x5c92f418) at ../src/backend/optimizer/plan/subselect.c:2168 No locals. #7 0x59191152 in expression_tree_mutator_impl (node=<optimized out>, mutator=0x592584b0 <replace_correlation_vars_mutator>, context=0x5c92f418) at ../src/backend/nodes/nodeFuncs.c:3556 targetentry = 0x5c92f6dc newnode = 0x5c92fa60 __func__ = "expression_tree_mutator_impl" #8 0x59258541 in replace_correlation_vars_mutator (node=0x5c92f6dc, root=0x5c92f418) at ../src/backend/optimizer/plan/subselect.c:2198 No locals. For a full backtrace, see "Core backtraces" e.g. on https://github.com/postgresql-cfbot/postgresql/actions/runs/27163049887/job/80183542083 Greetings, Andres Freund ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-09 03:51 Ashutosh Bapat <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 0 replies; 17+ messages in thread From: Ashutosh Bapat @ 2026-06-09 03:51 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Ewan Young <[email protected]>; Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]>; Peter Eisentraut <[email protected]> On Tue, Jun 9, 2026 at 1:45 AM Andres Freund <[email protected]> wrote: > > Hi, > > On 2026-06-04 19:21:48 +0800, Ewan Young wrote: > > The patch is against master and passes make check. On top of your > > v20260601 series only the test-file hunks conflict (both append to > > graph_table.sql); the code hunk applies cleanly. > > Unfortunately this fails in CI: > https://github.com/postgresql-cfbot/postgresql/actions/runs/27163049887 > > #4 0x598104c3 in ExceptionalCondition (conditionName=0x59adcb6c "var->varlevelsup > 0 && var->varlevelsup < root->query_level", fileName=0x59adcb40 "../src/backend/optimizer/util/paramassign.c", lineNumber=126) at ../src/backend/utils/error/assert.c:65 > No locals. > #5 0x592911f8 in replace_outer_var (root=0x5c92f418, var=0x5c92f708) at ../src/backend/optimizer/util/paramassign.c:126 > retval = <optimized out> > i = <optimized out> > #6 0x59258569 in replace_correlation_vars_mutator (node=0x5c92f708, root=0x5c92f418) at ../src/backend/optimizer/plan/subselect.c:2168 > No locals. > #7 0x59191152 in expression_tree_mutator_impl (node=<optimized out>, mutator=0x592584b0 <replace_correlation_vars_mutator>, context=0x5c92f418) at ../src/backend/nodes/nodeFuncs.c:3556 > targetentry = 0x5c92f6dc > newnode = 0x5c92fa60 > __func__ = "expression_tree_mutator_impl" > #8 0x59258541 in replace_correlation_vars_mutator (node=0x5c92f6dc, root=0x5c92f418) at ../src/backend/optimizer/plan/subselect.c:2198 > No locals. > > For a full backtrace, see "Core backtraces" e.g. on > https://github.com/postgresql-cfbot/postgresql/actions/runs/27163049887/job/80183542083 That failure is the result of posting the wrong patch to the wrong thread. Here's the correct set of patches. -- Best Wishes, Ashutosh Bapat Attachments: [text/x-patch] v20260601-0007-View-referencing-labels-shared-by-vertex-a.patch (4.6K, ../../CAExHW5vW4ZPgEO2=R4ATgpw-8E==JNhG0jBW-mN6GkmMh=Q8ew@mail.gmail.com/2-v20260601-0007-View-referencing-labels-shared-by-vertex-a.patch) download | inline diff: From e72095baf87d7e28c162c0835e0f5f03854eeb63 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 28 May 2026 02:47:33 +0530 Subject: [PATCH v20260601 7/7] View referencing labels shared by vertex and edge tables While at it add a test for view containing labels which are shared by both vertex and edge tables. When such a label is dropped from only vertex tables or only edge tables, the view may be rendered invalid because properties only associated with that label can not be resolved. The fix will need to wait for the SQL/PGQ standard to specify the behaviour in such a case. Author: Ashutosh Bapat <[email protected]> --- src/test/regress/expected/graph_table.out | 20 ++++++++++++++++++++ src/test/regress/sql/graph_table.sql | 13 +++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index cfee0626fdb..475ac20d376 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -973,6 +973,19 @@ ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error ERROR: cannot drop label vl2 of property graph g1 because other objects depend on it DETAIL: view v_empty_label depends on label vl2 of property graph g1 HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- l1 is shared by all vertex tables and edge tables. Dropping it from all +-- vertex tables only renders a view unusable. This is because the standard +-- differentiates between a vertex label and an edge label even though they +-- share the same name. Waiting for the standard to clarify the expected +-- behavior in this case. +CREATE VIEW v_shared_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname)); +BEGIN; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; +SELECT * FROM v_shared_label; +ERROR: no property graph element of type "vertex" has label "l1" associated with it in property graph "g1" +ROLLBACK; -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); pg_get_viewdef @@ -993,6 +1006,13 @@ SELECT pg_get_viewdef('v_empty_label'::regclass); FROM GRAPH_TABLE (g1 MATCH (v IS l1|vl1|vl2|vl3 WHERE (v.vprop1 = 10)) COLUMNS (v.elname AS elname)); (1 row) +SELECT pg_get_viewdef('v_shared_label'::regclass); + pg_get_viewdef +------------------------------------------------------------------------ + SELECT elname + + FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname AS elname)); +(1 row) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index c4cb7999857..5412f10ac3c 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -554,9 +554,22 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error +-- l1 is shared by all vertex tables and edge tables. Dropping it from all +-- vertex tables only renders a view unusable. This is because the standard +-- differentiates between a vertex label and an edge label even though they +-- share the same name. Waiting for the standard to clarify the expected +-- behavior in this case. +CREATE VIEW v_shared_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname)); +BEGIN; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; +SELECT * FROM v_shared_label; +ROLLBACK; -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); SELECT pg_get_viewdef('v_empty_label'::regclass); +SELECT pg_get_viewdef('v_shared_label'::regclass); -- test view/graph nesting -- 2.34.1 [text/x-patch] v20260601-0006-Empty-label-expression-in-view-definition.patch (20.1K, ../../CAExHW5vW4ZPgEO2=R4ATgpw-8E==JNhG0jBW-mN6GkmMh=Q8ew@mail.gmail.com/3-v20260601-0006-Empty-label-expression-in-view-definition.patch) download | inline diff: From db93ea38da965fa2503ef528520076378da70cc7 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 28 May 2026 02:45:58 +0530 Subject: [PATCH v20260601 6/7] Empty label expression in view definition A view definition depends on all the graph labels that are explicitly mentioned in the graph patterns in it so as to avoid it being rendered invalid when any of the labels is dropped. But when a view definition contains an empty label expression, we do not create any dependency between the view and the labels that the empty label expression resolves to. Resolve an empty label expression during transformation phase so that we can create dependency between those labels and the view. This will further help to avoid invalidation of a view containing all-properties references when we support it. An empty label expression may resolve to an empty set of labels if there are not labels associated with the elements matching the kind of element pattern containing the label expression. We do not have a syntax level support for a label expression containing no labels. Hence we can not dump a view containing such an empty label expression. Throw an error when a query contains such an empty label expression. There are possibly no real usecases which use such queries. Author: Ashutosh Bapat <[email protected]> Discussion: https://www.postgresql.org/message-id/CAExHW5twGP5Zuk4Zch4kz8XDrSpckWQipMs=ysAj8GmqNa2FCQ@mail.gmail.com --- src/backend/parser/parse_graphtable.c | 110 +++++++++++++++++- src/backend/rewrite/rewriteGraphTable.c | 78 ++++--------- src/include/nodes/parsenodes.h | 8 ++ .../expected/create_property_graph.out | 1 + src/test/regress/expected/graph_table.out | 29 ++++- .../regress/sql/create_property_graph.sql | 1 + src/test/regress/sql/graph_table.sql | 16 ++- 7 files changed, 178 insertions(+), 65 deletions(-) diff --git a/src/backend/parser/parse_graphtable.c b/src/backend/parser/parse_graphtable.c index 73fbfb541f7..5c9da75c8cf 100644 --- a/src/backend/parser/parse_graphtable.c +++ b/src/backend/parser/parse_graphtable.c @@ -18,6 +18,8 @@ #include "access/genam.h" #include "access/htup_details.h" #include "access/table.h" +#include "catalog/pg_propgraph_element.h" +#include "catalog/pg_propgraph_element_label.h" #include "catalog/pg_propgraph_label.h" #include "catalog/pg_propgraph_property.h" #include "miscadmin.h" @@ -151,6 +153,51 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) return NULL; } +/* + * Given the OID of a label and the kind of graph element pattern, return true if + * there exists at least one element matching the given kind associated with the + * label. Otherwise return false. + */ +static bool +label_has_elements_of_kind(Oid labelid, GraphElementPatternKind gepkind) +{ + Relation rel; + SysScanDesc scan; + ScanKeyData key[1]; + HeapTuple tup; + bool result = false; + + rel = table_open(PropgraphElementLabelRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_propgraph_element_label_pgellabelid, + BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(labelid)); + scan = systable_beginscan(rel, PropgraphElementLabelLabelIndexId, + true, NULL, 1, key); + while (!result && HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_propgraph_element_label element_label = (Form_pg_propgraph_element_label) GETSTRUCT(tup); + Oid element_oid = element_label->pgelelid; + HeapTuple element_tup = SearchSysCache1(PROPGRAPHELOID, ObjectIdGetDatum(element_oid)); + Form_pg_propgraph_element element_form; + + if (!HeapTupleIsValid(element_tup)) + elog(ERROR, "cache lookup failed for property graph element %u", element_oid); + + element_form = (Form_pg_propgraph_element) GETSTRUCT(element_tup); + + if ((element_form->pgekind == PGEKIND_VERTEX && gepkind == VERTEX_PATTERN) || + (element_form->pgekind == PGEKIND_EDGE && IS_EDGE_PATTERN(gepkind))) + result = true; + + ReleaseSysCache(element_tup); + } + + systable_endscan(scan); + table_close(rel, AccessShareLock); + return result; +} + /* * Transform a label expression. * @@ -161,14 +208,66 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) * GraphLabelRef nodes corresponding to the names of the labels appearing in the * expression. If any label name cannot be resolved to a label in the property * graph, an error is raised. + * + * An empty label expression is treated as a special case. According to section + * 9.2 "Contextual inference of a set of labels" subclause 2.a.ii of SQL/PGQ + * standard, element pattern which does not have a label expression is + * considered to have label expression equivalent to '%|!%' which is set of all + * labels which have at least one element of the given element kind associated with it. */ static Node * -transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr) +transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr, GraphElementPatternKind gepkind) { Node *result; - if (labelexpr == NULL) - return NULL; + if (!labelexpr) + { + Relation rel; + SysScanDesc scan; + ScanKeyData key[1]; + HeapTuple tup; + List *args = NIL; + + rel = table_open(PropgraphLabelRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_propgraph_label_pglpgid, + BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(gpstate->graphid)); + scan = systable_beginscan(rel, PropgraphLabelGraphNameIndexId, + true, NULL, 1, key); + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_propgraph_label label = (Form_pg_propgraph_label) GETSTRUCT(tup); + GraphLabelRef *lref; + + if (!label_has_elements_of_kind(label->oid, gepkind)) + continue; + + lref = makeNode(GraphLabelRef); + lref->labelid = label->oid; + lref->location = -1; + args = lappend(args, lref); + } + systable_endscan(scan); + table_close(rel, AccessShareLock); + + /* + * If there are no labels with elements of the given kind, the set of + * labels that this label expression resolves to is empty. There is no + * way to represent an empty set of labels as a label expression since + * we do not support label conjunction as well as negation. So we can + * not dump a view containing such a label expression. Hence prohibit + * it for now. + */ + if (!args) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("empty label expression does not resolve to any label")); + + result = (Node *) makeBoolExpr(OR_EXPR, args, -1); + return result; + + } check_stack_depth(); @@ -208,7 +307,7 @@ transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr) { Node *arg = (Node *) lfirst(lc); - arg = transformLabelExpr(gpstate, arg); + arg = transformLabelExpr(gpstate, arg, gepkind); args = lappend(args, arg); } @@ -249,7 +348,8 @@ transformGraphElementPattern(ParseState *pstate, GraphElementPattern *gep) gpstate->cur_gep = gep; - gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr); + gep->has_empty_labelexpr = !gep->labelexpr; + gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr, gep->kind); gep->whereClause = transformExpr(pstate, gep->whereClause, EXPR_KIND_WHERE); diff --git a/src/backend/rewrite/rewriteGraphTable.c b/src/backend/rewrite/rewriteGraphTable.c index 33d4e866d74..2292ca6dcee 100644 --- a/src/backend/rewrite/rewriteGraphTable.c +++ b/src/backend/rewrite/rewriteGraphTable.c @@ -58,6 +58,8 @@ struct path_factor { GraphElementPatternKind kind; const char *variable; + bool has_empty_labelexpr; /* Copied from the corresponding + * GraphElementPattern */ Node *labelexpr; Node *whereClause; int factorpos; /* Position of this path factor in the list of @@ -221,9 +223,12 @@ generate_queries_for_path_pattern(RangeTblEntry *rte, List *path_pattern) * expression itself. Hence if only one of the two element * patterns has a label expression use that expression. */ - if (!other->labelexpr) + if (other->has_empty_labelexpr) + { other->labelexpr = gep->labelexpr; - else if (gep->labelexpr && !equal(other->labelexpr, gep->labelexpr)) + other->has_empty_labelexpr = gep->has_empty_labelexpr; + } + else if (!gep->has_empty_labelexpr && !equal(other->labelexpr, gep->labelexpr)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("element patterns with same variable name \"%s\" but different label expressions are not supported", @@ -250,6 +255,7 @@ generate_queries_for_path_pattern(RangeTblEntry *rte, List *path_pattern) pf = palloc0_object(struct path_factor); pf->factorpos = factorpos++; pf->kind = gep->kind; + pf->has_empty_labelexpr = gep->has_empty_labelexpr; pf->labelexpr = gep->labelexpr; pf->variable = gep->variable; pf->whereClause = gep->whereClause; @@ -823,37 +829,9 @@ get_labels_for_expr(Oid propgraphid, Node *labelexpr) { List *label_oids; - if (!labelexpr) - { - Relation rel; - SysScanDesc scan; - ScanKeyData key[1]; - HeapTuple tup; - - /* - * According to section 9.2 "Contextual inference of a set of labels" - * subclause 2.a.ii of SQL/PGQ standard, element pattern which does - * not have a label expression is considered to have label expression - * equivalent to '%|!%' which is set of all labels. - */ - label_oids = NIL; - rel = table_open(PropgraphLabelRelationId, AccessShareLock); - ScanKeyInit(&key[0], - Anum_pg_propgraph_label_pglpgid, - BTEqualStrategyNumber, - F_OIDEQ, ObjectIdGetDatum(propgraphid)); - scan = systable_beginscan(rel, PropgraphLabelGraphNameIndexId, - true, NULL, 1, key); - while (HeapTupleIsValid(tup = systable_getnext(scan))) - { - Form_pg_propgraph_label label = (Form_pg_propgraph_label) GETSTRUCT(tup); + Assert(labelexpr); - label_oids = lappend_oid(label_oids, label->oid); - } - systable_endscan(scan); - table_close(rel, AccessShareLock); - } - else if (IsA(labelexpr, GraphLabelRef)) + if (IsA(labelexpr, GraphLabelRef)) { GraphLabelRef *glr = castNode(GraphLabelRef, labelexpr); @@ -903,7 +881,6 @@ get_path_elements_for_path_factor(Oid propgraphid, struct path_factor *pf) List *elem_oids_seen = NIL; List *pf_elem_oids = NIL; List *path_elements = NIL; - List *unresolved_labels = NIL; Relation rel; SysScanDesc scan; ScanKeyData key[1]; @@ -970,33 +947,26 @@ get_path_elements_for_path_factor(Oid propgraphid, struct path_factor *pf) { /* * We did not find any qualified element associated with this - * label. The label or its properties can not be associated with - * the given element pattern. Throw an error if the label was - * explicitly specified in the element pattern. Otherwise remember - * it for later use. + * label. Throw an error. + * + * An empty label expression is replaced by all labels that are + * associated with at least one element of the required kind. We + * should not reach here in that case. */ - if (!pf->labelexpr) - unresolved_labels = lappend_oid(unresolved_labels, labeloid); - else - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("no property graph element of type \"%s\" has label \"%s\" associated with it in property graph \"%s\"", - pf->kind == VERTEX_PATTERN ? "vertex" : "edge", - get_propgraph_label_name(labeloid), - get_rel_name(propgraphid)))); + Assert(!pf->has_empty_labelexpr); + + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("no property graph element of type \"%s\" has label \"%s\" associated with it in property graph \"%s\"", + pf->kind == VERTEX_PATTERN ? "vertex" : "edge", + get_propgraph_label_name(labeloid), + get_rel_name(propgraphid)))); } systable_endscan(scan); } table_close(rel, AccessShareLock); - - /* - * Remove the labels which were not explicitly mentioned in the label - * expression but do not have any qualified elements associated with them. - * Properties associated with such labels may not be referenced. See - * replace_property_refs_mutator() for more details. - */ - pf->labeloids = list_difference_oid(label_oids, unresolved_labels); + pf->labeloids = label_oids; return path_elements; } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 91377a6cde3..71040261375 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1048,7 +1048,15 @@ typedef struct GraphElementPattern NodeTag type; GraphElementPatternKind kind; const char *variable; + + /* + * If no label expression is specified, we will replace it with a non-NULL + * expression in transformLabelExpr(). This flag indicates whether the + * label expression was originally empty. + */ + bool has_empty_labelexpr; Node *labelexpr; + List *subexpr; Node *whereClause; List *quantifier; diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out index 65508223f54..a1a5501dd52 100644 --- a/src/test/regress/expected/create_property_graph.out +++ b/src/test/regress/expected/create_property_graph.out @@ -939,6 +939,7 @@ DETAIL: Table "v2tmp" is a temporary table. DROP TABLE g2; -- error: wrong object type ERROR: "g2" is not a table HINT: Use DROP PROPERTY GRAPH to remove a property graph. +ALTER PROPERTY GRAPH g1 ADD VERTEX TABLES (t1 KEY (a)); -- to make a valid graph query CREATE VIEW vg1 AS SELECT * FROM GRAPH_TABLE(g1 MATCH () COLUMNS (1 AS one)); DROP PROPERTY GRAPH g1; -- error ERROR: cannot drop property graph g1 because other objects depend on it diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index a5f9f0ac90a..cfee0626fdb 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -844,6 +844,8 @@ EXECUTE loopstmt; (2 rows) -- inheritance and partitioning +-- +-- Also test temporary property graphs, keywords NODE and RELATIONSHIP CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); CREATE TABLE cv2 () INHERITS (pv); @@ -873,7 +875,6 @@ SELECT * FROM GRAPH_TABLE (g3 MATCH (s IS pv)-[e IS pe]->(d IS pv) COLUMNS (s.va 30 | 300 | 10 (3 rows) --- temporary property graph CREATE TEMPORARY PROPERTY GRAPH gtmp VERTEX TABLES ( pv KEY (id) @@ -900,8 +901,12 @@ CREATE TABLE ptne1 PARTITION OF ptne FOR VALUES IN (1, 2); CREATE TABLE ptne2 PARTITION OF ptne FOR VALUES IN (3); INSERT INTO ptne VALUES (1, 1, 2, 100), (2, 2, 3, 200), (3, 3, 1, 300); CREATE PROPERTY GRAPH g4 - VERTEX TABLES (ptnv) - EDGE TABLES ( + VERTEX TABLES (ptnv); +-- empty label expression which resolves to no labels +SELECT * FROM GRAPH_TABLE (g4 MATCH (s is ptnv)-[e]-(d is ptnv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; -- error +ERROR: empty label expression does not resolve to any label +ALTER PROPERTY GRAPH g4 + ADD EDGE TABLES ( ptne SOURCE KEY (src) REFERENCES ptnv(id) DESTINATION KEY (dest) REFERENCES ptnv(id) @@ -957,6 +962,17 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ERROR: cannot drop property price of property graph myshop because other objects depend on it DETAIL: view customers_us depends on property price of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- Empty label expression creates a dependency between the view and the set of +-- labels it resolves to +CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error +ERROR: cannot drop label vl1 of property graph g1 because other objects depend on it +DETAIL: view v_empty_label depends on label vl1 of property graph g1 +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error +ERROR: cannot drop label vl2 of property graph g1 because other objects depend on it +DETAIL: view v_empty_label depends on label vl2 of property graph g1 +HINT: Use DROP ... CASCADE to drop the dependent objects too. -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); pg_get_viewdef @@ -970,6 +986,13 @@ SELECT pg_get_viewdef('customers_us'::regclass); ORDER BY g.customer_name, g.product_name; (1 row) +SELECT pg_get_viewdef('v_empty_label'::regclass); + pg_get_viewdef +---------------------------------------------------------------------------------------------------------- + SELECT elname + + FROM GRAPH_TABLE (g1 MATCH (v IS l1|vl1|vl2|vl3 WHERE (v.vprop1 = 10)) COLUMNS (v.elname AS elname)); +(1 row) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql index e8b3b3ff94c..0c3aa81e9ca 100644 --- a/src/test/regress/sql/create_property_graph.sql +++ b/src/test/regress/sql/create_property_graph.sql @@ -364,6 +364,7 @@ ALTER PROPERTY GRAPH g1 -- DROP, ALTER SET SCHEMA, ALTER PROPERTY GRAPH RENAME TO DROP TABLE g2; -- error: wrong object type +ALTER PROPERTY GRAPH g1 ADD VERTEX TABLES (t1 KEY (a)); -- to make a valid graph query CREATE VIEW vg1 AS SELECT * FROM GRAPH_TABLE(g1 MATCH () COLUMNS (1 AS one)); DROP PROPERTY GRAPH g1; -- error ALTER PROPERTY GRAPH g1 SET SCHEMA create_property_graph_tests_2; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 80576b2f9f4..c4cb7999857 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -471,6 +471,8 @@ ALTER PROPERTY GRAPH g1 ALTER EDGE TABLE e3_3 ALTER LABEL l2 ADD PROPERTIES ((en EXECUTE loopstmt; -- inheritance and partitioning +-- +-- Also test temporary property graphs, keywords NODE and RELATIONSHIP CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); CREATE TABLE cv2 () INHERITS (pv); @@ -493,7 +495,6 @@ CREATE PROPERTY GRAPH g3 DESTINATION KEY(dest) REFERENCES pv(id) ); SELECT * FROM GRAPH_TABLE (g3 MATCH (s IS pv)-[e IS pe]->(d IS pv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; --- temporary property graph CREATE TEMPORARY PROPERTY GRAPH gtmp VERTEX TABLES ( pv KEY (id) @@ -514,8 +515,11 @@ CREATE TABLE ptne1 PARTITION OF ptne FOR VALUES IN (1, 2); CREATE TABLE ptne2 PARTITION OF ptne FOR VALUES IN (3); INSERT INTO ptne VALUES (1, 1, 2, 100), (2, 2, 3, 200), (3, 3, 1, 300); CREATE PROPERTY GRAPH g4 - VERTEX TABLES (ptnv) - EDGE TABLES ( + VERTEX TABLES (ptnv); +-- empty label expression which resolves to no labels +SELECT * FROM GRAPH_TABLE (g4 MATCH (s is ptnv)-[e]-(d is ptnv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; -- error +ALTER PROPERTY GRAPH g4 + ADD EDGE TABLES ( ptne SOURCE KEY (src) REFERENCES ptnv(id) DESTINATION KEY (dest) REFERENCES ptnv(id) @@ -545,8 +549,14 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers ALTER LABEL customers DROP PROPERTIES (address); -- error ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ALTER LABEL products DROP PROPERTIES (price); -- error +-- Empty label expression creates a dependency between the view and the set of +-- labels it resolves to +CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); +SELECT pg_get_viewdef('v_empty_label'::regclass); -- test view/graph nesting -- 2.34.1 [text/x-patch] v20260601-0005-Record-dependencies-on-graph-labels-and-pr.patch (11.4K, ../../CAExHW5vW4ZPgEO2=R4ATgpw-8E==JNhG0jBW-mN6GkmMh=Q8ew@mail.gmail.com/4-v20260601-0005-Record-dependencies-on-graph-labels-and-pr.patch) download | inline diff: From ba5614bcd38b96e0e455225bab3cb510ab7f151f Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Sun, 17 May 2026 12:21:13 +0530 Subject: [PATCH v20260601 5/7] Record dependencies on graph labels and properties A view definition with GRAPH_TABLE depends upon the property graph it references as well as the properties and labels referenced in it. We recorded the dependency on the property graph, but did not record dependency on labels and properties. This allowed properties or labels referenced by a view to be dropped resulting in a cache lookup error when such a view was accessed. Fix this bug by handling GraphPropertyRef and GraphLabelRef in find_expr_references_walker(). The dependency on the data type of property is not needed to be recorded separately as it is recorded indirectly via dependency on the property graph property itself. Please note that a property or a label associated with individual elements can still be dropped as long as there are other elements which are associated with that property or label since they do not lead to dropping the property or the label from the property graph altogether. Reported-by: Man Zeng <[email protected]> Author: Ayush Tiwari <[email protected]> Author: Ashutosh Bapat <[email protected]> Reviewed by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/catalog/dependency.c | 19 ++++++++++++ src/test/regress/expected/graph_table.out | 37 +++++++++++++++++------ src/test/regress/sql/graph_table.sql | 14 +++++++-- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index fdb8e67e1f5..c54774b3275 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -2165,6 +2165,25 @@ find_expr_references_walker(Node *node, add_object_address(TypeRelationId, rowexpr->row_typeid, 0, context->addrs); } + else if (IsA(node, GraphLabelRef)) + { + GraphLabelRef *glr = (GraphLabelRef *) node; + + /* GRAPH_TABLE label reference depends on the property graph label */ + add_object_address(PropgraphLabelRelationId, glr->labelid, 0, + context->addrs); + } + else if (IsA(node, GraphPropertyRef)) + { + GraphPropertyRef *gpr = (GraphPropertyRef *) node; + + /* + * GRAPH_TABLE property reference depends on the property graph + * property + */ + add_object_address(PropgraphPropertyRelationId, gpr->propid, 0, + context->addrs); + } else if (IsA(node, RowCompareExpr)) { RowCompareExpr *rcexpr = (RowCompareExpr *) node; diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index e8d49fd5cd4..a5f9f0ac90a 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -929,7 +929,7 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e 30 | 300 | 10 (2 rows) --- ruleutils reverse parsing +-- GRAPH_TABLE in views -- The query in the view definition is intentionally complex to test one view with many -- features like label disjunction, lateral references, WHERE clauses in graph -- patterns. @@ -938,16 +938,35 @@ SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) - COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g + COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; +-- Dropping properties or labels used by a view is not allowed +-- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup +-- error. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error +ERROR: cannot drop label orders of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on label orders of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers + ALTER LABEL customers DROP PROPERTIES (address); -- error +ERROR: cannot drop property address of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on property address of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products + ALTER LABEL products DROP PROPERTIES (price); -- error +ERROR: cannot drop property price of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on property price of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); - pg_get_viewdef ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- - SELECT g.customer_name, + - g.product_name, + - g.a + - FROM x1, + - GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g+ + pg_get_viewdef +---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + SELECT g.customer_name, + + g.product_name, + + g.price, + + g.a + + FROM x1, + + GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, p.price AS price, x1.a AS a)) g+ ORDER BY g.customer_name, g.product_name; (1 row) diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index e761f09e057..80576b2f9f4 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -525,7 +525,8 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s IS ptnv)-[e IS ptne]->(d IS ptnv) COLUMNS SELECT * FROM GRAPH_TABLE (g4 MATCH (s)-[e]-(d) WHERE s.id = 3 COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; --- ruleutils reverse parsing +-- GRAPH_TABLE in views + -- The query in the view definition is intentionally complex to test one view with many -- features like label disjunction, lateral references, WHERE clauses in graph -- patterns. @@ -534,8 +535,17 @@ SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) - COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g + COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; +-- Dropping properties or labels used by a view is not allowed +-- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup +-- error. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers + ALTER LABEL customers DROP PROPERTIES (address); -- error +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products + ALTER LABEL products DROP PROPERTIES (price); -- error +-- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); -- test view/graph nesting -- 2.34.1 ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-09 11:06 Peter Eisentraut <[email protected]> parent: Ashutosh Bapat <[email protected]> 2 siblings, 1 reply; 17+ messages in thread From: Peter Eisentraut @ 2026-06-09 11:06 UTC (permalink / raw) To: Ashutosh Bapat <[email protected]>; Ayush Tiwari <[email protected]>; +Cc: Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]> On 01.06.26 13:37, Ashutosh Bapat wrote: > As I mentioned earlier, I am creating patches for all outstanding > issues from the same branch. So the patches attached here do not start > from 0001, but they apply with git am on the latest master for me > without any problem. > 0005 - fixes `cache lookup failed for label` > 0006 - fixes empty label expression and view behaviour > 0007 - issue with labels shared by vertex and edges I have committed 0005, which I think was the topic of this thread. ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-09 15:37 Ashutosh Bapat <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 2 replies; 17+ messages in thread From: Ashutosh Bapat @ 2026-06-09 15:37 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]> On Tue, Jun 9, 2026 at 4:36 PM Peter Eisentraut <[email protected]> wrote: > > On 01.06.26 13:37, Ashutosh Bapat wrote: > > As I mentioned earlier, I am creating patches for all outstanding > > issues from the same branch. So the patches attached here do not start > > from 0001, but they apply with git am on the latest master for me > > without any problem. > > 0005 - fixes `cache lookup failed for label` > > 0006 - fixes empty label expression and view behaviour > > 0007 - issue with labels shared by vertex and edges > > I have committed 0005, which I think was the topic of this thread. > Thanks a lot. -- Best Wishes, Ashutosh Bapat ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-06-18 05:48 Ashutosh Bapat <[email protected]> parent: Ashutosh Bapat <[email protected]> 1 sibling, 0 replies; 17+ messages in thread From: Ashutosh Bapat @ 2026-06-18 05:48 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]> On Tue, Jun 9, 2026 at 9:07 PM Ashutosh Bapat <[email protected]> wrote: > > On Tue, Jun 9, 2026 at 4:36 PM Peter Eisentraut <[email protected]> wrote: > > > > On 01.06.26 13:37, Ashutosh Bapat wrote: > > > As I mentioned earlier, I am creating patches for all outstanding > > > issues from the same branch. So the patches attached here do not start > > > from 0001, but they apply with git am on the latest master for me > > > without any problem. > > > 0005 - fixes `cache lookup failed for label` > > > 0006 - fixes empty label expression and view behaviour > > > 0007 - issue with labels shared by vertex and edges > > > > I have committed 0005, which I think was the topic of this thread. > > > > Thanks a lot. Moving this to resolved items since the original bug, which led to PG 19 open item is fixed. -- Best Wishes, Ashutosh Bapat ^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: (SQL/PGQ) cache lookup failed for label @ 2026-07-08 11:39 Ashutosh Bapat <[email protected]> parent: Ashutosh Bapat <[email protected]> 1 sibling, 0 replies; 17+ messages in thread From: Ashutosh Bapat @ 2026-07-08 11:39 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; Junwang Zhao <[email protected]>; zengman <[email protected]>; pgsql-hackers <[email protected]> On Tue, Jun 9, 2026 at 9:07 PM Ashutosh Bapat <[email protected]> wrote: > > On Tue, Jun 9, 2026 at 4:36 PM Peter Eisentraut <[email protected]> wrote: > > > > On 01.06.26 13:37, Ashutosh Bapat wrote: > > > As I mentioned earlier, I am creating patches for all outstanding > > > issues from the same branch. So the patches attached here do not start > > > from 0001, but they apply with git am on the latest master for me > > > without any problem. > > > 0005 - fixes `cache lookup failed for label` > > > 0006 - fixes empty label expression and view behaviour > > > 0007 - issue with labels shared by vertex and edges > > > > I have committed 0005, which I think was the topic of this thread. > > > > Thanks a lot. Attaching rebased remaining two patches for the issues discussed in [1] [1] https://www.postgresql.org/message-id/[email protected]... -- Best Wishes, Ashutosh Bapat Attachments: [text/x-patch] v20260708-0003-View-referencing-labels-shared-by-vertex-a.patch (4.6K, ../../CAExHW5uNRS9tcgnHCc03rh5oQhFPz-wrC1YF+_XUhPdwK2fbXw@mail.gmail.com/2-v20260708-0003-View-referencing-labels-shared-by-vertex-a.patch) download | inline diff: From 8e630ed6b73f5a0cd4031ac9747d3ffc0623501d Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 28 May 2026 02:47:33 +0530 Subject: [PATCH v20260708 3/5] View referencing labels shared by vertex and edge tables While at it add a test for view containing labels which are shared by both vertex and edge tables. When such a label is dropped from only vertex tables or only edge tables, the view may be rendered invalid because properties only associated with that label can not be resolved. The fix will need to wait for the SQL/PGQ standard to specify the behaviour in such a case. Author: Ashutosh Bapat <[email protected]> --- src/test/regress/expected/graph_table.out | 20 ++++++++++++++++++++ src/test/regress/sql/graph_table.sql | 13 +++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index 869ef481c04..0ccebf1aea5 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -1008,6 +1008,19 @@ ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error ERROR: cannot drop label vl2 of property graph g1 because other objects depend on it DETAIL: view v_empty_label depends on label vl2 of property graph g1 HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- l1 is shared by all vertex tables and edge tables. Dropping it from all +-- vertex tables only renders a view unusable. This is because the standard +-- differentiates between a vertex label and an edge label even though they +-- share the same name. Waiting for the standard to clarify the expected +-- behavior in this case. +CREATE VIEW v_shared_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname)); +BEGIN; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; +SELECT * FROM v_shared_label; +ERROR: no property graph element of type "vertex" has label "l1" associated with it in property graph "g1" +ROLLBACK; -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); pg_get_viewdef @@ -1028,6 +1041,13 @@ SELECT pg_get_viewdef('v_empty_label'::regclass); FROM GRAPH_TABLE (g1 MATCH (v IS l1|vl1|vl2|vl3 WHERE (v.vprop1 = 10)) COLUMNS (v.elname AS elname)); (1 row) +SELECT pg_get_viewdef('v_shared_label'::regclass); + pg_get_viewdef +------------------------------------------------------------------------ + SELECT elname + + FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname AS elname)); +(1 row) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index b6a20db7a41..9e5817bb4ca 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -566,9 +566,22 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error +-- l1 is shared by all vertex tables and edge tables. Dropping it from all +-- vertex tables only renders a view unusable. This is because the standard +-- differentiates between a vertex label and an edge label even though they +-- share the same name. Waiting for the standard to clarify the expected +-- behavior in this case. +CREATE VIEW v_shared_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v IS l1) COLUMNS (v.elname)); +BEGIN; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL l1; +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v3 DROP LABEL l1; +SELECT * FROM v_shared_label; +ROLLBACK; -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); SELECT pg_get_viewdef('v_empty_label'::regclass); +SELECT pg_get_viewdef('v_shared_label'::regclass); -- test view/graph nesting -- 2.34.1 [text/x-patch] v20260708-0002-Empty-label-expression-in-view-definition.patch (19.5K, ../../CAExHW5uNRS9tcgnHCc03rh5oQhFPz-wrC1YF+_XUhPdwK2fbXw@mail.gmail.com/3-v20260708-0002-Empty-label-expression-in-view-definition.patch) download | inline diff: From 51aaa2f752f1b42d826603029000131361a6ee2e Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 28 May 2026 02:45:58 +0530 Subject: [PATCH v20260708 2/5] Empty label expression in view definition A view definition depends on all the graph labels that are explicitly mentioned in the graph patterns in it so as to avoid it being rendered invalid when any of the labels is dropped. But when a view definition contains an empty label expression, we do not create any dependency between the view and the labels that the empty label expression resolves to. Resolve an empty label expression during transformation phase so that we can create dependency between those labels and the view. This will further help to avoid invalidation of a view containing all-properties references when we support it. An empty label expression may resolve to an empty set of labels if there are not labels associated with the elements matching the kind of element pattern containing the label expression. We do not have a syntax level support for a label expression containing no labels. Hence we can not dump a view containing such an empty label expression. Throw an error when a query contains such an empty label expression. There are possibly no real usecases which use such queries. Author: Ashutosh Bapat <[email protected]> Discussion: https://www.postgresql.org/message-id/CAExHW5twGP5Zuk4Zch4kz8XDrSpckWQipMs=ysAj8GmqNa2FCQ@mail.gmail.com --- src/backend/parser/parse_graphtable.c | 110 +++++++++++++++++- src/backend/rewrite/rewriteGraphTable.c | 78 ++++--------- src/include/nodes/parsenodes.h | 8 ++ .../expected/create_property_graph.out | 1 + src/test/regress/expected/graph_table.out | 28 ++++- .../regress/sql/create_property_graph.sql | 1 + src/test/regress/sql/graph_table.sql | 15 ++- 7 files changed, 178 insertions(+), 63 deletions(-) diff --git a/src/backend/parser/parse_graphtable.c b/src/backend/parser/parse_graphtable.c index 73fbfb541f7..5c9da75c8cf 100644 --- a/src/backend/parser/parse_graphtable.c +++ b/src/backend/parser/parse_graphtable.c @@ -18,6 +18,8 @@ #include "access/genam.h" #include "access/htup_details.h" #include "access/table.h" +#include "catalog/pg_propgraph_element.h" +#include "catalog/pg_propgraph_element_label.h" #include "catalog/pg_propgraph_label.h" #include "catalog/pg_propgraph_property.h" #include "miscadmin.h" @@ -151,6 +153,51 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) return NULL; } +/* + * Given the OID of a label and the kind of graph element pattern, return true if + * there exists at least one element matching the given kind associated with the + * label. Otherwise return false. + */ +static bool +label_has_elements_of_kind(Oid labelid, GraphElementPatternKind gepkind) +{ + Relation rel; + SysScanDesc scan; + ScanKeyData key[1]; + HeapTuple tup; + bool result = false; + + rel = table_open(PropgraphElementLabelRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_propgraph_element_label_pgellabelid, + BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(labelid)); + scan = systable_beginscan(rel, PropgraphElementLabelLabelIndexId, + true, NULL, 1, key); + while (!result && HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_propgraph_element_label element_label = (Form_pg_propgraph_element_label) GETSTRUCT(tup); + Oid element_oid = element_label->pgelelid; + HeapTuple element_tup = SearchSysCache1(PROPGRAPHELOID, ObjectIdGetDatum(element_oid)); + Form_pg_propgraph_element element_form; + + if (!HeapTupleIsValid(element_tup)) + elog(ERROR, "cache lookup failed for property graph element %u", element_oid); + + element_form = (Form_pg_propgraph_element) GETSTRUCT(element_tup); + + if ((element_form->pgekind == PGEKIND_VERTEX && gepkind == VERTEX_PATTERN) || + (element_form->pgekind == PGEKIND_EDGE && IS_EDGE_PATTERN(gepkind))) + result = true; + + ReleaseSysCache(element_tup); + } + + systable_endscan(scan); + table_close(rel, AccessShareLock); + return result; +} + /* * Transform a label expression. * @@ -161,14 +208,66 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) * GraphLabelRef nodes corresponding to the names of the labels appearing in the * expression. If any label name cannot be resolved to a label in the property * graph, an error is raised. + * + * An empty label expression is treated as a special case. According to section + * 9.2 "Contextual inference of a set of labels" subclause 2.a.ii of SQL/PGQ + * standard, element pattern which does not have a label expression is + * considered to have label expression equivalent to '%|!%' which is set of all + * labels which have at least one element of the given element kind associated with it. */ static Node * -transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr) +transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr, GraphElementPatternKind gepkind) { Node *result; - if (labelexpr == NULL) - return NULL; + if (!labelexpr) + { + Relation rel; + SysScanDesc scan; + ScanKeyData key[1]; + HeapTuple tup; + List *args = NIL; + + rel = table_open(PropgraphLabelRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_propgraph_label_pglpgid, + BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(gpstate->graphid)); + scan = systable_beginscan(rel, PropgraphLabelGraphNameIndexId, + true, NULL, 1, key); + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_propgraph_label label = (Form_pg_propgraph_label) GETSTRUCT(tup); + GraphLabelRef *lref; + + if (!label_has_elements_of_kind(label->oid, gepkind)) + continue; + + lref = makeNode(GraphLabelRef); + lref->labelid = label->oid; + lref->location = -1; + args = lappend(args, lref); + } + systable_endscan(scan); + table_close(rel, AccessShareLock); + + /* + * If there are no labels with elements of the given kind, the set of + * labels that this label expression resolves to is empty. There is no + * way to represent an empty set of labels as a label expression since + * we do not support label conjunction as well as negation. So we can + * not dump a view containing such a label expression. Hence prohibit + * it for now. + */ + if (!args) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("empty label expression does not resolve to any label")); + + result = (Node *) makeBoolExpr(OR_EXPR, args, -1); + return result; + + } check_stack_depth(); @@ -208,7 +307,7 @@ transformLabelExpr(GraphTableParseState *gpstate, Node *labelexpr) { Node *arg = (Node *) lfirst(lc); - arg = transformLabelExpr(gpstate, arg); + arg = transformLabelExpr(gpstate, arg, gepkind); args = lappend(args, arg); } @@ -249,7 +348,8 @@ transformGraphElementPattern(ParseState *pstate, GraphElementPattern *gep) gpstate->cur_gep = gep; - gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr); + gep->has_empty_labelexpr = !gep->labelexpr; + gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr, gep->kind); gep->whereClause = transformExpr(pstate, gep->whereClause, EXPR_KIND_WHERE); diff --git a/src/backend/rewrite/rewriteGraphTable.c b/src/backend/rewrite/rewriteGraphTable.c index cdb1f4c0dca..a5b1be77a0a 100644 --- a/src/backend/rewrite/rewriteGraphTable.c +++ b/src/backend/rewrite/rewriteGraphTable.c @@ -58,6 +58,8 @@ struct path_factor { GraphElementPatternKind kind; const char *variable; + bool has_empty_labelexpr; /* Copied from the corresponding + * GraphElementPattern */ Node *labelexpr; Node *whereClause; int factorpos; /* Position of this path factor in the list of @@ -221,9 +223,12 @@ generate_queries_for_path_pattern(RangeTblEntry *rte, List *path_pattern) * expression itself. Hence if only one of the two element * patterns has a label expression use that expression. */ - if (!other->labelexpr) + if (other->has_empty_labelexpr) + { other->labelexpr = gep->labelexpr; - else if (gep->labelexpr && !equal(other->labelexpr, gep->labelexpr)) + other->has_empty_labelexpr = gep->has_empty_labelexpr; + } + else if (!gep->has_empty_labelexpr && !equal(other->labelexpr, gep->labelexpr)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("element patterns with same variable name \"%s\" but different label expressions are not supported", @@ -250,6 +255,7 @@ generate_queries_for_path_pattern(RangeTblEntry *rte, List *path_pattern) pf = palloc0_object(struct path_factor); pf->factorpos = factorpos++; pf->kind = gep->kind; + pf->has_empty_labelexpr = gep->has_empty_labelexpr; pf->labelexpr = gep->labelexpr; pf->variable = gep->variable; pf->whereClause = gep->whereClause; @@ -830,37 +836,9 @@ get_labels_for_expr(Oid propgraphid, Node *labelexpr) { List *label_oids; - if (!labelexpr) - { - Relation rel; - SysScanDesc scan; - ScanKeyData key[1]; - HeapTuple tup; - - /* - * According to section 9.2 "Contextual inference of a set of labels" - * subclause 2.a.ii of SQL/PGQ standard, element pattern which does - * not have a label expression is considered to have label expression - * equivalent to '%|!%' which is set of all labels. - */ - label_oids = NIL; - rel = table_open(PropgraphLabelRelationId, AccessShareLock); - ScanKeyInit(&key[0], - Anum_pg_propgraph_label_pglpgid, - BTEqualStrategyNumber, - F_OIDEQ, ObjectIdGetDatum(propgraphid)); - scan = systable_beginscan(rel, PropgraphLabelGraphNameIndexId, - true, NULL, 1, key); - while (HeapTupleIsValid(tup = systable_getnext(scan))) - { - Form_pg_propgraph_label label = (Form_pg_propgraph_label) GETSTRUCT(tup); + Assert(labelexpr); - label_oids = lappend_oid(label_oids, label->oid); - } - systable_endscan(scan); - table_close(rel, AccessShareLock); - } - else if (IsA(labelexpr, GraphLabelRef)) + if (IsA(labelexpr, GraphLabelRef)) { GraphLabelRef *glr = castNode(GraphLabelRef, labelexpr); @@ -910,7 +888,6 @@ get_path_elements_for_path_factor(Oid propgraphid, struct path_factor *pf) List *elem_oids_seen = NIL; List *pf_elem_oids = NIL; List *path_elements = NIL; - List *unresolved_labels = NIL; Relation rel; SysScanDesc scan; ScanKeyData key[1]; @@ -977,33 +954,26 @@ get_path_elements_for_path_factor(Oid propgraphid, struct path_factor *pf) { /* * We did not find any qualified element associated with this - * label. The label or its properties can not be associated with - * the given element pattern. Throw an error if the label was - * explicitly specified in the element pattern. Otherwise remember - * it for later use. + * label. Throw an error. + * + * An empty label expression is replaced by all labels that are + * associated with at least one element of the required kind. We + * should not reach here in that case. */ - if (!pf->labelexpr) - unresolved_labels = lappend_oid(unresolved_labels, labeloid); - else - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("no property graph element of type \"%s\" has label \"%s\" associated with it in property graph \"%s\"", - pf->kind == VERTEX_PATTERN ? "vertex" : "edge", - get_propgraph_label_name(labeloid), - get_rel_name(propgraphid)))); + Assert(!pf->has_empty_labelexpr); + + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("no property graph element of type \"%s\" has label \"%s\" associated with it in property graph \"%s\"", + pf->kind == VERTEX_PATTERN ? "vertex" : "edge", + get_propgraph_label_name(labeloid), + get_rel_name(propgraphid)))); } systable_endscan(scan); } table_close(rel, AccessShareLock); - - /* - * Remove the labels which were not explicitly mentioned in the label - * expression but do not have any qualified elements associated with them. - * Properties associated with such labels may not be referenced. See - * replace_property_refs_mutator() for more details. - */ - pf->labeloids = list_difference_oid(label_oids, unresolved_labels); + pf->labeloids = label_oids; return path_elements; } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index e03556399ab..d6d0f7d170d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1051,7 +1051,15 @@ typedef struct GraphElementPattern NodeTag type; GraphElementPatternKind kind; const char *variable; + + /* + * If no label expression is specified, we will replace it with a non-NULL + * expression in transformLabelExpr(). This flag indicates whether the + * label expression was originally empty. + */ + bool has_empty_labelexpr; Node *labelexpr; + List *subexpr; Node *whereClause; List *quantifier; diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out index 4a8b3d91219..3efcce046cf 100644 --- a/src/test/regress/expected/create_property_graph.out +++ b/src/test/regress/expected/create_property_graph.out @@ -974,6 +974,7 @@ DETAIL: Table "v2tmp" is a temporary table. DROP TABLE g2; -- error: wrong object type ERROR: "g2" is not a table HINT: Use DROP PROPERTY GRAPH to remove a property graph. +ALTER PROPERTY GRAPH g1 ADD VERTEX TABLES (t1 KEY (a)); -- to make a valid graph query CREATE VIEW vg1 AS SELECT * FROM GRAPH_TABLE(g1 MATCH () COLUMNS (1 AS one)); DROP PROPERTY GRAPH g1; -- error ERROR: cannot drop property graph g1 because other objects depend on it diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index a3d78a7ac43..869ef481c04 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -865,6 +865,8 @@ EXECUTE loopstmt; (2 rows) -- inheritance and partitioning +-- +-- Also test temporary property graphs, keywords NODE and RELATIONSHIP CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); CREATE TABLE cv2 () INHERITS (pv); @@ -934,8 +936,12 @@ CREATE TABLE ptne1 PARTITION OF ptne FOR VALUES IN (1, 2); CREATE TABLE ptne2 PARTITION OF ptne FOR VALUES IN (3); INSERT INTO ptne VALUES (1, 1, 2, 100), (2, 2, 3, 200), (3, 3, 1, 300); CREATE PROPERTY GRAPH g4 - VERTEX TABLES (ptnv) - EDGE TABLES ( + VERTEX TABLES (ptnv); +-- empty label expression which resolves to no labels +SELECT * FROM GRAPH_TABLE (g4 MATCH (s is ptnv)-[e]-(d is ptnv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; -- error +ERROR: empty label expression does not resolve to any label +ALTER PROPERTY GRAPH g4 + ADD EDGE TABLES ( ptne SOURCE KEY (src) REFERENCES ptnv(id) DESTINATION KEY (dest) REFERENCES ptnv(id) @@ -991,6 +997,17 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ERROR: cannot drop property price of property graph myshop because other objects depend on it DETAIL: view customers_us depends on property price of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- Empty label expression creates a dependency between the view and the set of +-- labels it resolves to +CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error +ERROR: cannot drop label vl1 of property graph g1 because other objects depend on it +DETAIL: view v_empty_label depends on label vl1 of property graph g1 +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error +ERROR: cannot drop label vl2 of property graph g1 because other objects depend on it +DETAIL: view v_empty_label depends on label vl2 of property graph g1 +HINT: Use DROP ... CASCADE to drop the dependent objects too. -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); pg_get_viewdef @@ -1004,6 +1021,13 @@ SELECT pg_get_viewdef('customers_us'::regclass); ORDER BY g.customer_name, g.product_name; (1 row) +SELECT pg_get_viewdef('v_empty_label'::regclass); + pg_get_viewdef +---------------------------------------------------------------------------------------------------------- + SELECT elname + + FROM GRAPH_TABLE (g1 MATCH (v IS l1|vl1|vl2|vl3 WHERE (v.vprop1 = 10)) COLUMNS (v.elname AS elname)); +(1 row) + -- test view/graph nesting CREATE VIEW customers_view AS SELECT customer_id, 'redacted' || customer_id AS name_redacted, address FROM customers; SELECT * FROM customers; diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql index 1667896f558..f98a9187451 100644 --- a/src/test/regress/sql/create_property_graph.sql +++ b/src/test/regress/sql/create_property_graph.sql @@ -387,6 +387,7 @@ ALTER PROPERTY GRAPH g1 -- DROP, ALTER SET SCHEMA, ALTER PROPERTY GRAPH RENAME TO DROP TABLE g2; -- error: wrong object type +ALTER PROPERTY GRAPH g1 ADD VERTEX TABLES (t1 KEY (a)); -- to make a valid graph query CREATE VIEW vg1 AS SELECT * FROM GRAPH_TABLE(g1 MATCH () COLUMNS (1 AS one)); DROP PROPERTY GRAPH g1; -- error ALTER PROPERTY GRAPH g1 SET SCHEMA create_property_graph_tests_2; diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 6aacc2d4aa5..b6a20db7a41 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -479,6 +479,8 @@ ALTER PROPERTY GRAPH g1 ALTER EDGE TABLE e3_3 ALTER LABEL l2 ADD PROPERTIES ((en EXECUTE loopstmt; -- inheritance and partitioning +-- +-- Also test temporary property graphs, keywords NODE and RELATIONSHIP CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); CREATE TABLE cv2 () INHERITS (pv); @@ -525,8 +527,11 @@ CREATE TABLE ptne1 PARTITION OF ptne FOR VALUES IN (1, 2); CREATE TABLE ptne2 PARTITION OF ptne FOR VALUES IN (3); INSERT INTO ptne VALUES (1, 1, 2, 100), (2, 2, 3, 200), (3, 3, 1, 300); CREATE PROPERTY GRAPH g4 - VERTEX TABLES (ptnv) - EDGE TABLES ( + VERTEX TABLES (ptnv); +-- empty label expression which resolves to no labels +SELECT * FROM GRAPH_TABLE (g4 MATCH (s is ptnv)-[e]-(d is ptnv) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; -- error +ALTER PROPERTY GRAPH g4 + ADD EDGE TABLES ( ptne SOURCE KEY (src) REFERENCES ptnv(id) DESTINATION KEY (dest) REFERENCES ptnv(id) @@ -556,8 +561,14 @@ ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers ALTER LABEL customers DROP PROPERTIES (address); -- error ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ALTER LABEL products DROP PROPERTIES (price); -- error +-- Empty label expression creates a dependency between the view and the set of +-- labels it resolves to +CREATE VIEW v_empty_label AS SELECT * FROM GRAPH_TABLE (g1 MATCH (v WHERE v.vprop1 = 10) COLUMNS (v.elname)); +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v1 DROP LABEL vl1; -- error +ALTER PROPERTY GRAPH g1 ALTER VERTEX TABLE v2 DROP LABEL vl2; -- error -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); +SELECT pg_get_viewdef('v_empty_label'::regclass); -- test view/graph nesting -- 2.34.1 ^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2026-07-08 11:39 UTC | newest] Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-08-03 23:21 [PATCH v5 2/2] ALTER TABLE ... DETACH CONCURRENTLY Alvaro Herrera <[email protected]> 2026-05-18 02:07 Re: (SQL/PGQ) cache lookup failed for label Junwang Zhao <[email protected]> 2026-05-18 06:26 ` Re: (SQL/PGQ) cache lookup failed for label Ayush Tiwari <[email protected]> 2026-05-21 18:37 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-06-01 11:37 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-06-04 11:21 ` Re: (SQL/PGQ) cache lookup failed for label Ewan Young <[email protected]> 2026-06-04 11:46 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-06-04 12:44 ` Re: (SQL/PGQ) cache lookup failed for label Ewan Young <[email protected]> 2026-06-08 20:15 ` Re: (SQL/PGQ) cache lookup failed for label Andres Freund <[email protected]> 2026-06-09 03:51 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-06-04 14:07 ` Re: (SQL/PGQ) cache lookup failed for label =?ISO-8859-1?B?emVuZ21hbg==?= <[email protected]> 2026-06-04 15:04 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-06-05 14:37 ` Re: (SQL/PGQ) cache lookup failed for label =?ISO-8859-1?B?emVuZ21hbg==?= <[email protected]> 2026-06-09 11:06 ` Re: (SQL/PGQ) cache lookup failed for label Peter Eisentraut <[email protected]> 2026-06-09 15:37 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-06-18 05:48 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]> 2026-07-08 11:39 ` Re: (SQL/PGQ) cache lookup failed for label Ashutosh Bapat <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox