Fix handling of dropping a property not associated with the given label

When dropping a property by name from a label, the code checked only
whether the property existed in the graph's property catalog.  It did
not verify that the property was actually associated with the given
label, resulting in passing InvalidOid to performDeletion().  Fix it
by explicilty checking the label property association.

While at it also rearrange the code so as to avoid multiple ereport
calls for the same error in the same block.

Author: Chao Li <lic@highgo.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/1DA5D52A-4AFA-426E-83F7-42ED974D682B%40gmail.com

diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c
index aa974ca6a4f9bb269b1893a47d434bb6175378a3..78cbf7d08d0089be29864019aa1815c712034181 100644 (file)
--- a/src/backend/commands/propgraphcmds.c
+++ b/src/backend/commands/propgraphcmds.c
@@ -1583,7 +1583,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
                Oid                     peoid;
                Oid                     pgerelid;
                Oid                     labeloid;
-               Oid                     ellabeloid;
+               Oid                     ellabeloid = InvalidOid;
 
                if (stmt->element_kind == PROPGRAPH_ELEMENT_KIND_VERTEX)
                        peoid = get_vertex_oid(pstate, pgrelid, stmt->element_alias, -1);
@@ -1594,17 +1594,11 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
                                                                   Anum_pg_propgraph_label_oid,
                                                                   ObjectIdGetDatum(pgrelid),
                                                                   CStringGetDatum(stmt->alter_label));
-               if (!labeloid)
-                       ereport(ERROR,
-                                       errcode(ERRCODE_UNDEFINED_OBJECT),
-                                       errmsg("property graph \"%s\" element \"%s\" has no label \"%s\"",
-                                                  get_rel_name(pgrelid), stmt->element_alias, stmt->alter_label),
-                                       parser_errposition(pstate, -1));
-
-               ellabeloid = GetSysCacheOid2(PROPGRAPHELEMENTLABELELEMENTLABEL,
-                                                                        Anum_pg_propgraph_element_label_oid,
-                                                                        ObjectIdGetDatum(peoid),
-                                                                        ObjectIdGetDatum(labeloid));
+               if (labeloid)
+                       ellabeloid = GetSysCacheOid2(PROPGRAPHELEMENTLABELELEMENTLABEL,
+                                                                                Anum_pg_propgraph_element_label_oid,
+                                                                                ObjectIdGetDatum(peoid),
+                                                                                ObjectIdGetDatum(labeloid));
                if (!ellabeloid)
                        ereport(ERROR,
                                        errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -1625,7 +1619,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
        {
                Oid                     peoid;
                Oid                     labeloid;
-               Oid                     ellabeloid;
+               Oid                     ellabeloid = InvalidOid;
                ObjectAddress obj;
 
                if (stmt->element_kind == PROPGRAPH_ELEMENT_KIND_VERTEX)
@@ -1637,17 +1631,11 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
                                                                   Anum_pg_propgraph_label_oid,
                                                                   ObjectIdGetDatum(pgrelid),
                                                                   CStringGetDatum(stmt->alter_label));
-               if (!labeloid)
-                       ereport(ERROR,
-                                       errcode(ERRCODE_UNDEFINED_OBJECT),
-                                       errmsg("property graph \"%s\" element \"%s\" has no label \"%s\"",
-                                                  get_rel_name(pgrelid), stmt->element_alias, stmt->alter_label),
-                                       parser_errposition(pstate, -1));
-
-               ellabeloid = GetSysCacheOid2(PROPGRAPHELEMENTLABELELEMENTLABEL,
-                                                                        Anum_pg_propgraph_element_label_oid,
-                                                                        ObjectIdGetDatum(peoid),
-                                                                        ObjectIdGetDatum(labeloid));
+               if (labeloid)
+                       ellabeloid = GetSysCacheOid2(PROPGRAPHELEMENTLABELELEMENTLABEL,
+                                                                                Anum_pg_propgraph_element_label_oid,
+                                                                                ObjectIdGetDatum(peoid),
+                                                                                ObjectIdGetDatum(labeloid));
 
                if (!ellabeloid)
                        ereport(ERROR,
@@ -1660,21 +1648,24 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt)
                {
                        char       *propname = strVal(lfirst(lc));
                        Oid                     propoid;
-                       Oid                     plpoid;
+                       Oid                     plpoid = InvalidOid;
 
                        propoid = GetSysCacheOid2(PROPGRAPHPROPNAME,
                                                                          Anum_pg_propgraph_property_oid,
                                                                          ObjectIdGetDatum(pgrelid),
                                                                          CStringGetDatum(propname));
-                       if (!propoid)
+                       if (propoid)
+                               plpoid = GetSysCacheOid2(PROPGRAPHLABELPROP,
+                                                                                Anum_pg_propgraph_label_property_oid,
+                                                                                ObjectIdGetDatum(ellabeloid),
+                                                                                ObjectIdGetDatum(propoid));
+                       if (!plpoid)
                                ereport(ERROR,
                                                errcode(ERRCODE_UNDEFINED_OBJECT),
                                                errmsg("property graph \"%s\" element \"%s\" label \"%s\" has no property \"%s\"",
                                                           get_rel_name(pgrelid), stmt->element_alias, stmt->alter_label, propname),
                                                parser_errposition(pstate, -1));
 
-                       plpoid = GetSysCacheOid2(PROPGRAPHLABELPROP, Anum_pg_propgraph_label_property_oid, ObjectIdGetDatum(ellabeloid), ObjectIdGetDatum(propoid));
-
                        ObjectAddressSet(obj, PropgraphLabelPropertyRelationId, plpoid);
                        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 f9960e5e0aedb678d2a3d3f73f8a8857cb104b64..2a52a396fb4ec58a19a26eb072936abe541f8b37 100644 (file)
--- a/src/test/regress/expected/create_property_graph.out
+++ b/src/test/regress/expected/create_property_graph.out
@@ -89,6 +89,8 @@ CREATE PROPERTY GRAPH g4
     );
 ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 ADD PROPERTIES (k * 2 AS kk);
 ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 DROP PROPERTIES (k);
+ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 DROP PROPERTIES (yy);  -- error
+ERROR:  property graph "g4" element "t2" label "t2" has no property "yy"
 CREATE TABLE t11 (a int PRIMARY KEY);
 CREATE TABLE t12 (b int PRIMARY KEY);
 CREATE TABLE t13 (

diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql
index b10d7191506b9d8b16e8971e06dd0719e2351564..3b27d4170bb6ee9cc291c8194f3a17a7440870a5 100644 (file)
--- a/src/test/regress/sql/create_property_graph.sql
+++ b/src/test/regress/sql/create_property_graph.sql
@@ -80,6 +80,7 @@ CREATE PROPERTY GRAPH g4
 
 ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 ADD PROPERTIES (k * 2 AS kk);
 ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 DROP PROPERTIES (k);
+ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 DROP PROPERTIES (yy);  -- error
 
 CREATE TABLE t11 (a int PRIMARY KEY);
 CREATE TABLE t12 (b int PRIMARY KEY);