I have done a rough review of the patch set version 20251120.
- There are "No newline at end of file" warnings from git diff for a
couple of files:
contrib/pg_overexplain/sql/pg_overexplain.sql
src/test/regress/sql/graph_table_rls.sql
Please fix those in the next patch set, and maybe check your editor
settings.
- Attached are two small patches with some small fixes I found in
passing. (I think some of them were also reported by Junwang Zhao in
the meantime.)
- In the test files, especially src/test/regress/sql/graph_table.sql,
there are wildly different SQL styles (capitalization, formatting) used,
depending on who added the test case (I guess). Let's keep that more
consistent.
- In src/backend/parser/analyze.c, the extracted function
constructSetOpTargetlist() needs a detailed comment.
- I'm not so sure about the semantics chosen in the patch "Access
permissions on property graph". I think according to the SQL standard,
once you have access to the property graph, you don't need access to the
underlying tables as well. I guess you did this to align with how views
work? We might need to think about this a bit more, and document
whatever the conclusion is. But for now it's just small amount of code
affected.
I think you could collapse all the patches into one patch now. I have
reviewed all the incremental patches and they all look ok to me. I have
made some notes about which things I want to review in more detail, such
as the access control issue, but that doesn't need to be kept as a
separate patch.
When you create future patches, consider using the git format-patch -v
option.
And then you can also just gzip the patch. That should make it even
smaller than the .zip file.
From c7ccd6339bfa58e04e425925fc620d2a1ab91f5a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 25 Nov 2025 14:17:58 +0100
Subject: [PATCH 1/2] Typos and minor style fixes
---
doc/src/sgml/catalogs.sgml | 2 +-
doc/src/sgml/ref/create_property_graph.sgml | 2 +-
src/backend/commands/tablecmds.c | 4 ++--
src/backend/executor/execMain.c | 7 +++----
src/backend/optimizer/path/allpaths.c | 4 +++-
src/backend/rewrite/rewriteGraphTable.c | 1 +
src/backend/utils/adt/ruleutils.c | 2 +-
7 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 11f6a629ac7..fc73553ba5f 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6387,7 +6387,7 @@ <title><structname>pg_propgraph_element</structname>
Columns</title>
(references <link
linkend="catalog-pg-class"><structname>pg_class</structname></link>.<structfield>oid</structfield>)
</para>
<para>
- Reference to the table to contains the data for this property graph
element
+ Reference to the table that contains the data for this property graph
element
</para></entry>
</row>
diff --git a/doc/src/sgml/ref/create_property_graph.sgml
b/doc/src/sgml/ref/create_property_graph.sgml
index 7a4c9641470..92f870379fd 100644
--- a/doc/src/sgml/ref/create_property_graph.sgml
+++ b/doc/src/sgml/ref/create_property_graph.sgml
@@ -177,7 +177,7 @@ <title>Parameters</title>
the same as the element table alias. This can be specified explicitly
as <literal>DEFAULT LABEL</literal>. Alternatively, one or more freely
chosen label names can be specified. (Label names do not have to be
- unique across a property graph. It is can be useful to assign the same
+ unique across a property graph. It can be useful to assign the same
label to different elements.) Each label has a list (possibly empty) of
properties. By default, all columns of a table are automatically
exposed as properties. This can be specified explicitly as
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54700a25732..249499c697c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4201,8 +4201,8 @@ RenameConstraint(RenameStmt *stmt)
}
/*
- * Execute ALTER TABLE/INDEX/SEQUENCE/VIEW/MATERIALIZED VIEW/FOREIGN TABLE
- * RENAME/PROPERTY GRAPH
+ * Execute ALTER TABLE/INDEX/SEQUENCE/VIEW/MATERIALIZED VIEW/FOREIGN
TABLE/PROPERTY GRAPH
+ * RENAME
*/
ObjectAddress
RenameRelation(RenameStmt *stmt)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 11d0989bd50..f0f214a1c11 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -599,8 +599,7 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
/*
* Only relation RTEs and subquery RTEs that were once
relation
- * RTEs (views, property graph tables) have their
perminfoindex
- * set.
+ * RTEs (views, property graphs) have their
perminfoindex set.
*/
Assert(rte->rtekind == RTE_RELATION ||
(rte->rtekind == RTE_SUBQUERY &&
@@ -1234,8 +1233,8 @@ CheckValidRowMarkRel(Relation rel, RowMarkType markType)
/* Should not get here; rewriter should have expanded
the graph */
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot lock rows in property
graph \"%s\"",
-
RelationGetRelationName(rel))));
+ errmsg_internal("cannot lock rows in
property graph \"%s\"",
+
RelationGetRelationName(rel))));
break;
default:
ereport(ERROR,
diff --git a/src/backend/optimizer/path/allpaths.c
b/src/backend/optimizer/path/allpaths.c
index cd5fd292198..8e814b5725b 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -792,7 +792,9 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo
*rel,
* Shouldn't happen since these are replaced by
subquery RTEs when
* rewriting queries.
*/
- /* FALLTHROUGH */
+ Assert(false);
+ return;
+
case RTE_GROUP:
/* Shouldn't happen; we're only considering baserels
here. */
Assert(false);
diff --git a/src/backend/rewrite/rewriteGraphTable.c
b/src/backend/rewrite/rewriteGraphTable.c
index 4ed63840062..0490dd24874 100644
--- a/src/backend/rewrite/rewriteGraphTable.c
+++ b/src/backend/rewrite/rewriteGraphTable.c
@@ -539,6 +539,7 @@ generate_query_for_graph_path(RangeTblEntry *rte, List
*graph_path)
perminfo->selectedCols = bms_add_member(perminfo->selectedCols,
var->varattno - FirstLowInvalidHeapAttributeNumber);
}
+
return path_query;
}
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index dfe94c5036d..26845c9a7c1 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1842,7 +1842,7 @@ propdata_by_name_cmp(const ListCell *a, const ListCell *b)
/*
* Generates element table properties clause (PROPERTIES (...) or NO
- * PROPERTIES). Pass in label ODI and element table OID. Result is appended
+ * PROPERTIES). Pass in label OID and element table OID. Result is appended
* to buf.
*/
static void
--
2.52.0
From 56248072d25c82c64365c90e0f738a2ca4e92b1e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 25 Nov 2025 14:18:18 +0100
Subject: [PATCH 2/2] Remove added tokens from plpgsql
This was needed in earlier versions of the patch set, but no longer.
---
src/pl/plpgsql/src/pl_gram.y | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index df10a2e3705..17568d82554 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -247,7 +247,6 @@ static void
check_raise_parameters(PLpgSQL_stmt_raise *stmt);
%token <ival> ICONST PARAM
%token TYPECAST DOT_DOT COLON_EQUALS EQUALS_GREATER
%token LESS_EQUALS GREATER_EQUALS NOT_EQUALS
-%token BRACKET_RIGHT_ARROW LEFT_ARROW_BRACKET
MINUS_LEFT_BRACKET RIGHT_BRACKET_MINUS
/*
* Other tokens recognized by plpgsql's lexer interface layer (pl_scanner.c).
--
2.52.0
