Hi Ashutosh,
On Tue, Jan 28, 2025 at 6:17 PM Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
>
> On Fri, Jan 24, 2025 at 9:16 PM Junwang Zhao <zhjw...@gmail.com> wrote:
> >
> > I figured out it's because I have `-DWRITE_READ_PARSE_PLAN_TREES` this
> > option,
> > removing this option clears the warning, but I'm not sure if this is a
> > hidden issue.
> >
>
> Thanks for the hint. I found the issue and the fix appears in 0004.
> Some members of RangeTblEntry were not being handled in read/out
> functions.
>
> > >
> > > 0001-0009 patches are the same as the previous set sans mergeconflict
> > > fixes.
> > > 0010 - is test for \dGx - to make sure that the extended format output
> > > works with \dG. We may decide not to have this patch in the final
> > > commit. But no harm in being there til that point.
> >
> > I have some changes based on the latest patch set. I attached two patches
> > with
> > the following summary.
> >
> > 0001:
> > - doc: some of the query in ddl.sgml can not be executed
>
> The standard seems to require a property reference to be qualified by
> element pattern variable, even if the property is referenced within
> the same element pattern bound to the variable. Hence I accepted your
> document fixes which qualify property reference with element pattern
> variable.
>
> However, I didn't understand why you changed a LABEL name from order to
> order_.
>
> > - after path factor was introduced, some comments doesn't apply
>
> Thanks pointing those out. Accepted after rephrasing those and also
> correcting some related comments.
>
> >
> > 0002:
> > - add a get_propgraph_element_alias_name function and do some trivial
> > refactor
> >
>
> I see you have added some negative tests - object not found tests, but
> I didn't see positive tests. Hence I haven't added those changes in
> the attached patchset. But we certainly need those changes. You may
> want to submit a patch with positive tests. That code needs to be
> fixed before committing anyway.
The attached file adds the positive tests.
>
> --
> Best Wishes,
> Ashutosh Bapat
--
Regards
Junwang Zhao
From 5dfe5f0c7c7a72b7564b19c1d1e5396c66d5039f Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjw...@gmail.com>
Date: Sat, 21 Dec 2024 12:13:40 +0000
Subject: [PATCH v1] trivial refactor of property graph object address
---
src/backend/catalog/objectaddress.c | 54 +++++++++++++++----
src/backend/commands/propgraphcmds.c | 8 +--
src/backend/parser/parse_clause.c | 2 +-
src/backend/rewrite/rewriteGraphTable.c | 4 +-
src/backend/utils/adt/ruleutils.c | 8 +--
src/backend/utils/cache/lsyscache.c | 30 +++++++++--
src/include/utils/lsyscache.h | 5 +-
.../expected/create_property_graph.out | 36 +++++++++++++
src/test/regress/expected/object_address.out | 6 +++
.../regress/sql/create_property_graph.sql | 5 ++
src/test/regress/sql/object_address.sql | 3 ++
11 files changed, 134 insertions(+), 27 deletions(-)
diff --git a/src/backend/catalog/objectaddress.c
b/src/backend/catalog/objectaddress.c
index 6eaee345ad2..03ed830d5a1 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -4070,8 +4070,12 @@ getObjectDescription(const ObjectAddress *object, bool
missing_ok)
tup = SearchSysCache1(PROPGRAPHELOID,
ObjectIdGetDatum(object->objectId));
if (!HeapTupleIsValid(tup))
- elog(ERROR, "cache lookup failed for
property graph element %u",
- object->objectId);
+ {
+ if (!missing_ok)
+ elog(ERROR, "cache lookup
failed for property graph element %u",
+ object->objectId);
+ break;
+ }
pgeform = (Form_pg_propgraph_element)
GETSTRUCT(tup);
@@ -4118,7 +4122,7 @@ getObjectDescription(const ObjectAddress *object, bool
missing_ok)
pgelform = (Form_pg_propgraph_element_label)
GETSTRUCT(tuple);
- appendStringInfo(&buffer, _("label %s of "),
get_propgraph_label_name(pgelform->pgellabelid));
+ appendStringInfo(&buffer, _("label %s of "),
get_propgraph_label_name(pgelform->pgellabelid, false));
ObjectAddressSet(oa,
PropgraphElementRelationId, pgelform->pgelelid);
appendStringInfoString(&buffer,
getObjectDescription(&oa, false));
@@ -4193,7 +4197,7 @@ getObjectDescription(const ObjectAddress *object, bool
missing_ok)
plpform = (Form_pg_propgraph_label_property)
GETSTRUCT(tuple);
- appendStringInfo(&buffer, _("property %s of "),
get_propgraph_property_name(plpform->plppropid));
+ appendStringInfo(&buffer, _("property %s of "),
get_propgraph_property_name(plpform->plppropid, false));
ObjectAddressSet(oa,
PropgraphElementLabelRelationId, plpform->plpellabelid);
appendStringInfoString(&buffer,
getObjectDescription(&oa, false));
@@ -6174,16 +6178,46 @@ getObjectIdentityParts(const ObjectAddress *object,
}
case PropgraphElementRelationId:
- appendStringInfo(&buffer, "%u TODO", object->objectId);
- break;
+ {
+ char *elemname;
+
+ elemname =
get_propgraph_element_alias_name(object->objectId, missing_ok);
+ if (elemname)
+ {
+ appendStringInfoString(&buffer,
quote_identifier(elemname));
+ if (objname)
+ *objname = list_make1(elemname);
+ }
+ break;
+ }
case PropgraphLabelRelationId:
- appendStringInfo(&buffer, "%u TODO", object->objectId);
- break;
+ {
+ char *labelname;
+
+ labelname =
get_propgraph_label_name(object->objectId, missing_ok);
+ if (labelname)
+ {
+ appendStringInfoString(&buffer,
quote_identifier(labelname));
+ if (objname)
+ *objname =
list_make1(labelname);
+ }
+ break;
+ }
case PropgraphPropertyRelationId:
- appendStringInfo(&buffer, "%u TODO", object->objectId);
- break;
+ {
+ char *propname;
+
+ propname =
get_propgraph_property_name(object->objectId, missing_ok);
+ if (propname)
+ {
+ appendStringInfoString(&buffer,
quote_identifier(propname));
+ if (objname)
+ *objname = list_make1(propname);
+ }
+ break;
+ }
case PublicationRelationId:
{
diff --git a/src/backend/commands/propgraphcmds.c
b/src/backend/commands/propgraphcmds.c
index c26976b2a2a..68b81e1c807 100644
--- a/src/backend/commands/propgraphcmds.c
+++ b/src/backend/commands/propgraphcmds.c
@@ -1166,7 +1166,7 @@ check_element_properties(Oid peoid)
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
errmsg("element
\"%s\" property \"%s\" expression mismatch: %s vs. %s",
-
NameStr(elform->pgealias), get_propgraph_property_name(propoid), dpa, dpb),
+
NameStr(elform->pgealias), get_propgraph_property_name(propoid, false), dpa,
dpb),
errdetail("In a
property graph element, a property of the same name has to have the same
expression in each label."));
ReleaseSysCache(tuple3);
@@ -1283,7 +1283,7 @@ check_element_label_properties(Oid ellabeloid)
if (list_length(refprops) != list_length(myprops))
ereport(ERROR,
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("mismatching number of properties in
definition of label \"%s\"", get_propgraph_label_name(labelid)));
+ errmsg("mismatching number of properties in
definition of label \"%s\"", get_propgraph_label_name(labelid, false)));
diff1 = list_difference(myprops, refprops);
diff2 = list_difference(refprops, myprops);
@@ -1291,7 +1291,7 @@ check_element_label_properties(Oid ellabeloid)
if (diff1 || diff2)
ereport(ERROR,
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("mismatching properties names in
definition of label \"%s\"", get_propgraph_label_name(labelid)));
+ errmsg("mismatching properties names in
definition of label \"%s\"", get_propgraph_label_name(labelid, false)));
}
/*
@@ -1844,7 +1844,7 @@ get_element_label_property_names(Oid ellabeloid)
{
Form_pg_propgraph_label_property plpform =
(Form_pg_propgraph_label_property) GETSTRUCT(tuple);
- result = lappend(result,
makeString(get_propgraph_property_name(plpform->plppropid)));
+ result = lappend(result,
makeString(get_propgraph_property_name(plpform->plppropid, false)));
}
systable_endscan(scan);
diff --git a/src/backend/parser/parse_clause.c
b/src/backend/parser/parse_clause.c
index 29f518f6ada..b9c4934f393 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -959,7 +959,7 @@ transformRangeGraphTable(ParseState *pstate,
RangeGraphTable *rgt)
else
{
if (IsA(colexpr, GraphPropertyRef))
- colname =
get_propgraph_property_name(castNode(GraphPropertyRef, colexpr)->propid);
+ colname =
get_propgraph_property_name(castNode(GraphPropertyRef, colexpr)->propid, false);
else
{
ereport(ERROR,
diff --git a/src/backend/rewrite/rewriteGraphTable.c
b/src/backend/rewrite/rewriteGraphTable.c
index a33ad5ba7fd..6938d118f17 100644
--- a/src/backend/rewrite/rewriteGraphTable.c
+++ b/src/backend/rewrite/rewriteGraphTable.c
@@ -958,7 +958,7 @@ get_path_elements_for_path_factor(Oid propgraphid, struct
path_factor *pf)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("can not find label
\"%s\" in property graph \"%s\" for element type \"%s\"",
-
get_propgraph_label_name(labeloid),
+
get_propgraph_label_name(labeloid, false),
get_rel_name(propgraphid),
get_graph_elem_kind_name(pf->kind))));
}
@@ -1127,7 +1127,7 @@ replace_property_refs_mutator(Node *node, struct
replace_property_refs_context *
if (!n)
elog(ERROR, "property \"%s\" of element variable \"%s\"
not found",
- get_propgraph_property_name(gpr->propid),
mapping_factor->variable);
+ get_propgraph_property_name(gpr->propid,
false), mapping_factor->variable);
return n;
}
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 2e237023d95..ca29ce74601 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1809,7 +1809,7 @@ make_propgraphdef_labels(StringInfo buf, Oid elid, const
char *elalias, Oid elre
Form_pg_propgraph_element_label pgelform =
(Form_pg_propgraph_element_label) GETSTRUCT(tup);
const char *labelname;
- labelname = get_propgraph_label_name(pgelform->pgellabelid);
+ labelname = get_propgraph_label_name(pgelform->pgellabelid,
false);
if (strcmp(labelname, elalias) == 0)
{
@@ -1885,7 +1885,7 @@ make_propgraphdef_properties(StringInfo buf, Oid
ellabelid, Oid elrelid)
expr = stringToNode(tmp);
pfree(tmp);
- propname = get_propgraph_property_name(plpform->plppropid);
+ propname = get_propgraph_property_name(plpform->plppropid,
false);
outlist = lappend(outlist, list_make2(makeString(propname),
expr));
}
@@ -7871,7 +7871,7 @@ get_graph_label_expr(Node *label_expr, deparse_context
*context)
{
GraphLabelRef *lref = (GraphLabelRef *)
label_expr;
- appendStringInfoString(buf,
quote_identifier(get_propgraph_label_name(lref->labelid)));
+ appendStringInfoString(buf,
quote_identifier(get_propgraph_label_name(lref->labelid, false)));
break;
}
@@ -11048,7 +11048,7 @@ get_rule_expr(Node *node, deparse_context *context,
{
GraphPropertyRef *gpr = (GraphPropertyRef *)
node;
- appendStringInfo(buf, "%s.%s",
quote_identifier(gpr->elvarname),
quote_identifier(get_propgraph_property_name(gpr->propid)));
+ appendStringInfo(buf, "%s.%s",
quote_identifier(gpr->elvarname),
quote_identifier(get_propgraph_property_name(gpr->propid, false)));
break;
}
diff --git a/src/backend/utils/cache/lsyscache.c
b/src/backend/utils/cache/lsyscache.c
index b61533fd266..98833c7720b 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -32,6 +32,7 @@
#include "catalog/pg_opclass.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
+#include "catalog/pg_propgraph_element.h"
#include "catalog/pg_propgraph_label.h"
#include "catalog/pg_propgraph_property.h"
#include "catalog/pg_publication.h"
@@ -3718,7 +3719,26 @@ get_subscription_name(Oid subid, bool missing_ok)
}
char *
-get_propgraph_label_name(Oid labeloid)
+get_propgraph_element_alias_name(Oid elemoid, bool missing_ok)
+{
+ HeapTuple tuple;
+ char *elemname;
+
+ tuple = SearchSysCache1(PROPGRAPHELOID, elemoid);
+ if (!tuple)
+ {
+ if (!missing_ok)
+ elog(ERROR, "cache lookup failed for element %u",
elemoid);
+ return NULL;
+ }
+ elemname = pstrdup(NameStr(((Form_pg_propgraph_element)
GETSTRUCT(tuple))->pgealias));
+ ReleaseSysCache(tuple);
+
+ return elemname;
+}
+
+char *
+get_propgraph_label_name(Oid labeloid, bool missing_ok)
{
HeapTuple tuple;
char *labelname;
@@ -3726,7 +3746,8 @@ get_propgraph_label_name(Oid labeloid)
tuple = SearchSysCache1(PROPGRAPHLABELOID, labeloid);
if (!tuple)
{
- elog(ERROR, "cache lookup failed for label %u", labeloid);
+ if (!missing_ok)
+ elog(ERROR, "cache lookup failed for label %u",
labeloid);
return NULL;
}
labelname = pstrdup(NameStr(((Form_pg_propgraph_label)
GETSTRUCT(tuple))->pgllabel));
@@ -3736,7 +3757,7 @@ get_propgraph_label_name(Oid labeloid)
}
char *
-get_propgraph_property_name(Oid propoid)
+get_propgraph_property_name(Oid propoid, bool missing_ok)
{
HeapTuple tuple;
char *propname;
@@ -3744,7 +3765,8 @@ get_propgraph_property_name(Oid propoid)
tuple = SearchSysCache1(PROPGRAPHPROPOID, propoid);
if (!tuple)
{
- elog(ERROR, "cache lookup failed for property %u", propoid);
+ if (!missing_ok)
+ elog(ERROR, "cache lookup failed for property %u",
propoid);
return NULL;
}
propname = pstrdup(NameStr(((Form_pg_propgraph_property)
GETSTRUCT(tuple))->pgpname));
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 5d3444bec24..6f12ad0ea1d 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -206,8 +206,9 @@ extern char *get_publication_name(Oid pubid, bool
missing_ok);
extern Oid get_subscription_oid(const char *subname, bool missing_ok);
extern char *get_subscription_name(Oid subid, bool missing_ok);
-extern char *get_propgraph_label_name(Oid labeloid);
-extern char *get_propgraph_property_name(Oid propoid);
+extern char *get_propgraph_element_alias_name(Oid elemoid, bool missing_ok);
+extern char *get_propgraph_label_name(Oid labeloid, bool missing_ok);
+extern char *get_propgraph_property_name(Oid propoid, bool missing_ok);
#define type_is_array(typid) (get_element_type(typid) != InvalidOid)
/* type_is_array_domain accepts both plain arrays and domains over arrays */
diff --git a/src/test/regress/expected/create_property_graph.out
b/src/test/regress/expected/create_property_graph.out
index 101bdeaeb54..fd90a25c6bf 100644
--- a/src/test/regress/expected/create_property_graph.out
+++ b/src/test/regress/expected/create_property_graph.out
@@ -22,6 +22,42 @@ CREATE PROPERTY GRAPH g2
SOURCE KEY (a) REFERENCES t1 (a)
DESTINATION KEY (x, t) REFERENCES t3 (x, y)
);
+-- test object addresses
+SELECT pg_identify_object_as_address('pg_propgraph_element'::regclass, oid, 0)
FROM pg_propgraph_element;
+ pg_identify_object_as_address
+------------------------------------
+ ("property graph element",{t1},{})
+ ("property graph element",{t2},{})
+ ("property graph element",{t3},{})
+ ("property graph element",{e1},{})
+ ("property graph element",{e2},{})
+(5 rows)
+
+SELECT pg_identify_object_as_address('pg_propgraph_label'::regclass, oid, 0)
FROM pg_propgraph_label;
+ pg_identify_object_as_address
+------------------------------------
+ ("property graph label",{t1},{})
+ ("property graph label",{t2},{})
+ ("property graph label",{t3l1},{})
+ ("property graph label",{t3l2},{})
+ ("property graph label",{e1},{})
+ ("property graph label",{e2},{})
+(6 rows)
+
+SELECT pg_identify_object_as_address('pg_propgraph_property'::regclass, oid,
0) FROM pg_propgraph_property;
+ pg_identify_object_as_address
+------------------------------------
+ ("property graph property",{a},{})
+ ("property graph property",{b},{})
+ ("property graph property",{i},{})
+ ("property graph property",{j},{})
+ ("property graph property",{k},{})
+ ("property graph property",{x},{})
+ ("property graph property",{y},{})
+ ("property graph property",{z},{})
+ ("property graph property",{t},{})
+(9 rows)
+
-- test dependencies/object descriptions
DROP TABLE t1; -- fail
ERROR: cannot drop table t1 because other objects depend on it
diff --git a/src/test/regress/expected/object_address.out
b/src/test/regress/expected/object_address.out
index 432ba471fe3..3abb658de16 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -588,6 +588,9 @@ WITH objects (classid, objid, objsubid) AS (VALUES
('pg_event_trigger'::regclass, 0, 0), -- no event trigger
('pg_parameter_acl'::regclass, 0, 0), -- no parameter ACL
('pg_policy'::regclass, 0, 0), -- no policy
+ ('pg_propgraph_element'::regclass, 0, 0), -- no property graph element
+ ('pg_propgraph_label'::regclass, 0, 0), -- no property graph label
+ ('pg_propgraph_property'::regclass, 0, 0), -- no property graph property
('pg_publication'::regclass, 0, 0), -- no publication
('pg_publication_namespace'::regclass, 0, 0), -- no publication namespace
('pg_publication_rel'::regclass, 0, 0), -- no publication relation
@@ -644,5 +647,8 @@ ORDER BY objects.classid, objects.objid, objects.objsubid;
("(""publication relation"",,,)")|("(""publication relation"",,)")|NULL
("(""publication namespace"",,,)")|("(""publication namespace"",,)")|NULL
("(""parameter ACL"",,,)")|("(""parameter ACL"",,)")|NULL
+("(""property graph element"",,,)")|("(""property graph element"",,)")|NULL
+("(""property graph label"",,,)")|("(""property graph label"",,)")|NULL
+("(""property graph property"",,,)")|("(""property graph property"",,)")|NULL
-- restore normal output mode
\a\t
diff --git a/src/test/regress/sql/create_property_graph.sql
b/src/test/regress/sql/create_property_graph.sql
index 1e309b9f935..0081bedab4b 100644
--- a/src/test/regress/sql/create_property_graph.sql
+++ b/src/test/regress/sql/create_property_graph.sql
@@ -29,6 +29,11 @@ CREATE PROPERTY GRAPH g2
DESTINATION KEY (x, t) REFERENCES t3 (x, y)
);
+-- test object addresses
+SELECT pg_identify_object_as_address('pg_propgraph_element'::regclass, oid, 0)
FROM pg_propgraph_element;
+SELECT pg_identify_object_as_address('pg_propgraph_label'::regclass, oid, 0)
FROM pg_propgraph_label;
+SELECT pg_identify_object_as_address('pg_propgraph_property'::regclass, oid,
0) FROM pg_propgraph_property;
+
-- test dependencies/object descriptions
DROP TABLE t1; -- fail
diff --git a/src/test/regress/sql/object_address.sql
b/src/test/regress/sql/object_address.sql
index 93f9f9c704f..e6010b3bf3f 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -280,6 +280,9 @@ WITH objects (classid, objid, objsubid) AS (VALUES
('pg_event_trigger'::regclass, 0, 0), -- no event trigger
('pg_parameter_acl'::regclass, 0, 0), -- no parameter ACL
('pg_policy'::regclass, 0, 0), -- no policy
+ ('pg_propgraph_element'::regclass, 0, 0), -- no property graph element
+ ('pg_propgraph_label'::regclass, 0, 0), -- no property graph label
+ ('pg_propgraph_property'::regclass, 0, 0), -- no property graph property
('pg_publication'::regclass, 0, 0), -- no publication
('pg_publication_namespace'::regclass, 0, 0), -- no publication namespace
('pg_publication_rel'::regclass, 0, 0), -- no publication relation
--
2.39.5
