On Wed, Feb 18, 2026 at 8:58 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 13.01.26 17:14, Ashutosh Bapat wrote:
> > On Tue, Jan 13, 2026 at 3:53 PM Peter Eisentraut <peter@eisentraut.org> 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
