On Wed, Feb 18, 2026 at 8:58 PM Peter Eisentraut <[email protected]> wrote:
>
> On 13.01.26 17:14, Ashutosh Bapat wrote:
> > On Tue, Jan 13, 2026 at 3:53 PM Peter Eisentraut <[email protected]>
> > wrote:
> >>
> >> I have a small fixup patch for your 20260102 patches, attached.
> >>
> >> - needs additional #include because of recent changes elsewhere
> >> - copyright year updates
> >> - various typos
> >> - some style changes related to palloc APIs
> >
> > All changes look good.
> >
> > Looks like you have reviewed patches 0002-onwards. I removed 0004
> > which was erroneously removing the | handling from ecpg lexer as you
> > pointed out earlier. Squashed all other patches along with your small
> > fixup patch. Attached is the resultant single patch.
>
> I have studied a bit the changes in parse_relation.c with the additional
> relkind checks. That didn't look clean to me. I have attached here a
> patch that does it differently, by *not* accepting RELKIND_PROPGRAPH in
> table_open(). Instead, we write our own alternative to
> parserOpenTable() to use in the parser. This ends up even giving some
> better error messages.
This looks good to me.
I wondered whether to create propgraph_open() and
validate_relation_kind() for propgraph in a separate file propgraph.c
on the lines of sequence.c. But we will still need something like
parserOpenPropGraph() to handle callback. So doesn't seem to have an
advantage of sequence_open which is not called from parser. Also
parserOpenPropGraph()'s placement seems to be ok since there is only
one caller in parse_clause.c. This looks better than the earlier
changes in parse_relation.c
> The only other change is that in
> AcquireRewriteLocks() we need to use relation_open() instead of
> table_open(), but that seems harmless and even intellectually better,
> because at that point we don't care about giving anyone a user-facing
> error message about wrong relkinds, we just want to lock the ones we
> have.
+1.
> (Alternatively, we could make a separate switch case for
> RTE_GRAPH_TABLE.)
>
> What do you think?
LGTM.
I am actually wondering whether the comment in parserOpenPropGraph()
is required. In case we want to keep it, the attached patch has a typo
fix. It also has some more improvements.
>
> Also, see this patch:
>
> https://www.postgresql.org/message-id/6d3fef19-a420-4e11-8235-8ea534bf2080%40eisentraut.org
>
> If this is accepted, it would make the change in the patch here even
> smaller.
+1. I think we should commit this, rebase SQL/PGQ patches and then
apply this change.
>
> And then there is this patch:
>
> https://www.postgresql.org/message-id/4bcd65fb-2497-484c-bb41-83cb435eb64d%40eisentraut.org
>
> which also grew out of this analysis.
Reviewed both the patches.
--
Best Wishes,
Ashutosh Bapat
diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c
index fe330ae862a..7479254ce21 100644
--- a/src/backend/access/table/table.c
+++ b/src/backend/access/table/table.c
@@ -131,7 +131,7 @@ table_close(Relation relation, LOCKMODE lockmode)
/* ----------------
* validate_relation_kind - check the relation's kind
*
- * Make sure relkind is not index or composite type
+ * Make sure relkind is not an index, a composite type or a property graph.
* ----------------
*/
static inline void
@@ -139,7 +139,8 @@ validate_relation_kind(Relation r)
{
if (r->rd_rel->relkind == RELKIND_INDEX ||
r->rd_rel->relkind == RELKIND_PARTITIONED_INDEX ||
- r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
+ r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE ||
+ r->rd_rel->relkind == RELKIND_PROPGRAPH)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot open relation \"%s\"",
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index dea5315bed1..25588acc4d7 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -17,6 +17,7 @@
#include "access/htup_details.h"
#include "access/nbtree.h"
+#include "access/relation.h"
#include "access/table.h"
#include "access/tsmapi.h"
#include "catalog/catalog.h"
@@ -901,6 +902,33 @@ transformRangeTableFunc(ParseState *pstate, RangeTableFunc *rtf)
tf, rtf->alias, is_lateral, true);
}
+/*
+ * Similar to parserOpenTable() but for property graphs.
+ */
+static Relation
+parserOpenPropGraph(ParseState *pstate, const RangeVar *relation, int lockmode)
+{
+ Relation rel;
+ ParseCallbackState pcbstate;
+
+ setup_parser_errposition_callback(&pcbstate, pstate, relation->location);
+
+ rel = relation_openrv(relation, lockmode);
+
+ /*
+ * In parserOpenTable(), the relkind check is done inside table_openrv*.
+ * Since we don't have anything like propgraph_open, we do it here.
+ */
+ if (rel->rd_rel->relkind != RELKIND_PROPGRAPH)
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a property graph",
+ RelationGetRelationName(rel)));
+
+ cancel_parser_errposition_callback(&pcbstate);
+ return rel;
+}
+
/*
* transformRangeGraphTable -- transform a GRAPH_TABLE clause
*/
@@ -917,13 +945,7 @@ transformRangeGraphTable(ParseState *pstate, RangeGraphTable *rgt)
int resno = 0;
bool saved_hasSublinks;
- rel = parserOpenTable(pstate, rgt->graph_name, AccessShareLock);
- if (rel->rd_rel->relkind != RELKIND_PROPGRAPH)
- ereport(ERROR,
- errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a property graph",
- RelationGetRelationName(rel)),
- parser_errposition(pstate, rgt->graph_name->location));
+ rel = parserOpenPropGraph(pstate, rgt->graph_name, AccessShareLock);
graphid = RelationGetRelid(rel);
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index cd360917653..87ca5e000fb 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1512,20 +1512,6 @@ addRangeTableEntry(ParseState *pstate,
* to a rel in a statement, we must open the rel with the proper lockmode.
*/
rel = parserOpenTable(pstate, relation, lockmode);
-
- /*
- * validate_relation_kind() already catches indexes and composite types,
- * but we use table_openrv_extended() elsewhere for open a property graph
- * reference, which is not allowed here. Use similar error message as
- * validate_relation_kind().
- */
- if (rel->rd_rel->relkind == RELKIND_PROPGRAPH)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot open relation \"%s\"",
- RelationGetRelationName(rel)),
- errdetail_relkind_not_supported(rel->rd_rel->relkind)));
-
rte->relid = RelationGetRelid(rel);
rte->inh = inh;
rte->relkind = rel->rd_rel->relkind;
@@ -1609,19 +1595,6 @@ addRangeTableEntryForRelation(ParseState *pstate,
lockmode == RowExclusiveLock);
Assert(CheckRelationLockedByMe(rel, lockmode, true));
- /*
- * validate_relation_kind() already catches indexes and composite types,
- * but we use table_openrv_extended() elsewhere for open a property graph
- * reference, which is not allowed here. Use similar error message as
- * validate_relation_kind().
- */
- if (rel->rd_rel->relkind == RELKIND_PROPGRAPH)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot open relation \"%s\"",
- RelationGetRelationName(rel)),
- errdetail_relkind_not_supported(rel->rd_rel->relkind)));
-
rte->rtekind = RTE_RELATION;
rte->alias = alias;
rte->relid = RelationGetRelid(rel);
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 2520cefae9d..01cc9691d4e 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -197,7 +197,7 @@ AcquireRewriteLocks(Query *parsetree,
else
lockmode = rte->rellockmode;
- rel = table_open(rte->relid, lockmode);
+ rel = relation_open(rte->relid, lockmode);
/*
* While we have the relation open, update the RTE's relkind,
@@ -205,7 +205,7 @@ AcquireRewriteLocks(Query *parsetree,
*/
rte->relkind = rel->rd_rel->relkind;
- table_close(rel, NoLock);
+ relation_close(rel, NoLock);
break;
case RTE_JOIN:
diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out
index f90dc29535f..547a6b50916 100644
--- a/src/test/regress/expected/graph_table.out
+++ b/src/test/regress/expected/graph_table.out
@@ -100,12 +100,16 @@ ERROR: element pattern quantifier not supported yet
-- a property graph can be referenced only from within GRAPH_TABLE clause.
SELECT * FROM myshop; -- error
ERROR: cannot open relation "myshop"
+LINE 1: SELECT * FROM myshop;
+ ^
DETAIL: This operation is not supported for property graphs.
COPY myshop TO stdout; -- error
ERROR: cannot open relation "myshop"
DETAIL: This operation is not supported for property graphs.
INSERT INTO myshop VALUES (1); -- error
ERROR: cannot open relation "myshop"
+LINE 1: INSERT INTO myshop VALUES (1);
+ ^
DETAIL: This operation is not supported for property graphs.
INSERT INTO products VALUES
(1, 'product1', 10),
