Prevent dropping the last label from a property graph element

Per SQL/PGQ standard, every graph element must have at least one
label.  When dropping a label from a graph element, ensure that there
exists at least one other label on the element.  If the label being
dropped is the only label on the element, raise an error.

We hold a ShareRowExclusiveLock when modifying a property graph.
Hence the label will not be dropped even when multiple labels are
being dropped concurrently.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Author: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Reported-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAHg+QDeP=mTHTV48R23zKMy1SBmCKZ_L7-z5zKnYyw+K0x-gCg@mail.gmail.com

diff --git a/doc/src/sgml/ref/alter_property_graph.sgml b/doc/src/sgml/ref/alter_property_graph.sgml
index f517f2b2d7a7d0b7772995033b2839eb42558afa..d2768a4644ab2ef319affacead1f4d8a5e6fd362 100644 (file)
--- a/doc/src/sgml/ref/alter_property_graph.sgml
+++ b/doc/src/sgml/ref/alter_property_graph.sgml
@@ -99,7 +99,9 @@ ALTER PROPERTY GRAPH [ IF EXISTS ] <replaceable class="parameter">name</replacea
      <term><literal>ALTER {VERTEX|NODE|EDGE|RELATIONSHIP} TABLE ... DROP LABEL</literal></term>
      <listitem>
       <para>
-       This form removes a label from an existing vertex or edge table.
+       This form removes a label from an existing vertex or edge table.  The
+       last label on an element table cannot be dropped; every vertex or edge
+       table must have at least one label.
       </para>
      </listitem>
     </varlistentry>

diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c
index cc516e270203c858f868be4d289531366fc8a86d..aa974ca6a4f9bb269b1893a47d434bb6175378a3 100644 (file)
--- a/src/backend/commands/propgraphcmds.c
+++ b/src/backend/commands/propgraphcmds.c
@@ -1294,6 +1294,11 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
        ListCell   *lc;
        ObjectAddress pgaddress;
 
+       /*
+        * ShareRowExclusiveLock is required because this command runs some
+        * graph-wide consistency checks that wouldn't work if more than one ALTER
+        * PROPERTY GRAPH could operate on the same graph at once.
+        */
        pgrelid = RangeVarGetRelidExtended(stmt->pgname,
                                                                           ShareRowExclusiveLock,
                                                                           stmt->missing_ok ? RVR_MISSING_OK : 0,
@@ -1491,8 +1496,13 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
        {
                Oid                     peoid;
                Oid                     labeloid;
-               Oid                     ellabeloid;
+               Oid                     ellabeloid = InvalidOid;
                ObjectAddress obj;
+               Relation        ellabelrel;
+               SysScanDesc ellabelscan;
+               ScanKeyData ellabelkey[1];
+               int                     nlabels;
+               HeapTuple       tuple;
 
                if (stmt->element_kind == PROPGRAPH_ELEMENT_KIND_VERTEX)
                        peoid = get_vertex_oid(pstate, pgrelid, stmt->element_alias, -1);
@@ -1510,10 +1520,34 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
                                                   get_rel_name(pgrelid), stmt->element_alias, stmt->drop_label),
                                        parser_errposition(pstate, -1));
 
-               ellabeloid = GetSysCacheOid2(PROPGRAPHELEMENTLABELELEMENTLABEL,
-                                                                        Anum_pg_propgraph_element_label_oid,
-                                                                        ObjectIdGetDatum(peoid),
-                                                                        ObjectIdGetDatum(labeloid));
+               /*
+                * Is the given label associated with the element?  Is this the only
+                * label associated with the element?  Scan the
+                * pg_propgraph_element_label table to find answers to these
+                * questions.  Stop scanning when we know both answers.
+                */
+               ellabelrel = table_open(PropgraphElementLabelRelationId, AccessShareLock);
+               ScanKeyInit(&ellabelkey[0],
+                                       Anum_pg_propgraph_element_label_pgelelid,
+                                       BTEqualStrategyNumber, F_OIDEQ,
+                                       ObjectIdGetDatum(peoid));
+               ellabelscan = systable_beginscan(ellabelrel, PropgraphElementLabelElementLabelIndexId,
+                                                                                true, NULL, 1, ellabelkey);
+               nlabels = 0;
+               while (HeapTupleIsValid(tuple = systable_getnext(ellabelscan)))
+               {
+                       Form_pg_propgraph_element_label ellabelform = (Form_pg_propgraph_element_label) GETSTRUCT(tuple);
+
+                       nlabels++;
+
+                       if (ellabelform->pgellabelid == labeloid)
+                               ellabeloid = ellabelform->oid;
+
+                       if (nlabels > 1 && ellabeloid)
+                               break;
+               }
+               systable_endscan(ellabelscan);
+               table_close(ellabelrel, AccessShareLock);
 
                if (!ellabeloid)
                        ereport(ERROR,
@@ -1522,6 +1556,17 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
                                                   get_rel_name(pgrelid), stmt->element_alias, stmt->drop_label),
                                        parser_errposition(pstate, -1));
 
+               /*
+                * Prevent dropping the last label from an element. Every element must
+                * have at least one label associated with it.
+                */
+               if (nlabels == 1)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                        errmsg("cannot drop the last label from element \"%s\"",
+                                                       stmt->element_alias),
+                                        errhint("Every element must have at least one label.")));
+
                ObjectAddressSet(obj, PropgraphElementLabelRelationId, ellabeloid);
                performDeletion(&obj, stmt->drop_behavior, 0);
 

diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out
index 2f06c7ce5a8083065dfc645963038a43f5b8161b..f9960e5e0aedb678d2a3d3f73f8a8857cb104b64 100644 (file)
--- a/src/test/regress/expected/create_property_graph.out
+++ b/src/test/regress/expected/create_property_graph.out
@@ -57,6 +57,13 @@ ALTER PROPERTY GRAPH g3
 ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l3x;  -- error
 ERROR:  property graph "g3" element "t3" has no label "t3l3x"
 ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l3;
+-- Test that the last label on an element cannot be dropped
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l2;
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error: last label
+ERROR:  cannot drop the last label from element "t3"
+HINT:  Every element must have at least one label.
+-- Restore the dropped label for further tests
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;
 ALTER PROPERTY GRAPH g3 DROP VERTEX TABLES (t2);  -- fail
 ERROR:  cannot drop vertex t2 of property graph g3 because other objects depend on it
 DETAIL:  edge e1 of property graph g3 depends on vertex t2 of property graph g3

diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql
index 85088ae632c7a09ea0c94d98ffdc0a5f47fd481b..b10d7191506b9d8b16e8971e06dd0719e2351564 100644 (file)
--- a/src/test/regress/sql/create_property_graph.sql
+++ b/src/test/regress/sql/create_property_graph.sql
@@ -52,6 +52,11 @@ ALTER PROPERTY GRAPH g3
         ADD LABEL t3l3 PROPERTIES ALL COLUMNS;
 ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l3x;  -- error
 ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l3;
+-- Test that the last label on an element cannot be dropped
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l2;
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error: last label
+-- Restore the dropped label for further tests
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;
 ALTER PROPERTY GRAPH g3 DROP VERTEX TABLES (t2);  -- fail
 ALTER PROPERTY GRAPH g3 DROP VERTEX TABLES (t2) CASCADE;
 ALTER PROPERTY GRAPH g3 DROP EDGE TABLES (e2);