| From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Noah Misch <noah(at)leadboat(dot)com> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Wrong query result w/ propgraph single lateral col reference |
| Date: | 2026-07-08 06:54:28 |
| Message-ID: | CAExHW5sgk_QrxY475uYqrPyW_tMripfM3Z2OoUkkmkj8E_vECg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
Peter has committed several other patches. Here are rebased patches
for this thread.
On Fri, Jul 3, 2026 at 10:11 AM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
Issue 1
> > >
> > > CREATE TABLE v (flag boolean, id int PRIMARY KEY, name text);
> > > INSERT INTO v VALUES (true,1,'a'), (false,2,'b'), (true,3,'c');
> > > CREATE PROPERTY GRAPH g VERTEX TABLES (v KEY (id) LABEL l PROPERTIES (id, flag, name));
> > >
> > > Findings:
> > >
> > > 0. A WHERE clause that is a single lateral col reference gives wrong query results:
> > >
> > > SELECT t.b, gt.name FROM (VALUES (false)) AS t(b),
> > > GRAPH_TABLE (g MATCH (x IS l) WHERE t.b COLUMNS (x.name AS name)) gt;
> > > -- got 2 rows, want 0
> > >
> > > Adding "AND true" corrects the result:
> > >
> > > SELECT t.b, gt.name FROM (VALUES (false)) AS t(b),
> > > GRAPH_TABLE (g MATCH (x IS l) WHERE t.b AND true COLUMNS (x.name AS name)) gt;
> > > -- got 0 rows, want 0
> > >
> > > 1. A WHERE clause that is a single property reference gives a spurious error:
> > >
> > > SELECT name FROM GRAPH_TABLE (g MATCH (x IS l) WHERE x.flag COLUMNS (x.name));
> > > -- ERROR: unrecognized node type: 63
> > >
> > > Adding "AND true" again corrects the result:
> > >
> > > SELECT name FROM GRAPH_TABLE (g MATCH (x IS l) WHERE x.flag AND true COLUMNS (x.name));
> > > -- got 2 rows, want 2
> > >
> > > For (0) and (1), Opus's opinion is that the right fix is:
> > >
> > > --- a/src/backend/rewrite/rewriteGraphTable.c
> > > +++ b/src/backend/rewrite/rewriteGraphTable.c
> > > @@ replace_property_refs(Oid propgraphid, Node *node, const List *mappings)
> > > context.mappings = mappings;
> > > context.propgraphid = propgraphid;
> > >
> > > - return expression_tree_mutator(node, replace_property_refs_mutator, &context);
> > > + return replace_property_refs_mutator(node, &context);
> > > }
> > >
> >
> > This matches the pattern in the other expression mutators, and also
> > fixes the problems. Thanks for the report, that was quite subtle.
>
> Fix attached here.
That's 0003 patch attached here.
Issue 2
>
> >
> > >
> > > 2. A property whose value is an untyped literal stays type unknown.
> > >
> > > CREATE PROPERTY GRAPH gu VERTEX TABLES (v KEY (id) LABEL lu PROPERTIES ('hi' AS p));
> > > SELECT p FROM GRAPH_TABLE (gu MATCH (n IS lu) COLUMNS (n.p));
> > > -- ERROR: failed to find conversion function from unknown to text
> > >
> >
> > I think property's data type should be set to text, not unknown. I
> > think we should combine the fix for this in
> > https://www.postgresql.org/message-id/CAHg+QDcyKNWyzDoKMxiZNjv7C-wAxs8y0ZoNkOV137Y+nk3UXg@mail.gmail.com.
> >
>
> Attaching the fix here. But I will also report it on the other thread
> [2] and discussion can continue there.
>
The patch in that thread was committed. Attaching fix for this issue
as 0002 patch.
Issue 3
>
> >
> > >
> > > 4. [cosmetic] Duplicate property names found only via catalog unique key
> > >
> > > CREATE PROPERTY GRAPH g3 VERTEX TABLES (v KEY (id) LABEL lu PROPERTIES (flag, flag));
> > > -- ERROR: duplicate key value violates unique constraint "pg_propgraph_property_name_index"
> > >
> > > Opus thinks this is unintentional and cites lack of CommandCounterIncrement()
> > > between property inserts.
> >
> > We get the same error even if we split the duplicate properties across
> > two commands.
> > CREATE PROPERTY GRAPH g3 VERTEX TABLES (v KEY (id) LABEL lu PROPERTIES (flag));
> > alter property graph g3 alter vertex table v alter label lu add
> > properties(flag);
> >
> > I think we need to check for duplicate records in
> > pg_propgraph_label_property catalog in insert_property_record(). The
> > function already checks for duplicate records in
> > pg_propgraph_property. Additionally we might need
> > CommandCounterIncrement().
>
When investigating this issue I found that duplicate labels also have
the same issue. Fixed both the issues in patch 0001.
I have added a noop else in insert_label_record() after ereport()
since it ties the following code block well with the if condition. But
we can remove it if it's not improving readability.
There's slight inconsistency in the error messages for duplicate
labels and properties. In case of labels, we always report "label ...
already exists". But in case of properties we report two different
errors. We report "property \"%s\" specified more than once" when
duplicate properties appear in the same command. But we report
"property \"%s\" already exists" when duplicate properties appear
across commands. I think the difference is minor enough that we can
entirely ignore it and avoid spending more code on it. If we feel
strongly about being consistent, either we can detect duplicate labels
in the same command and report "label ... specified more than once" or
we can make the same error being reported in case of properties in
both the cases.
--
Best Wishes,
Ashutosh Bapat
| Attachment | Content-Type | Size |
|---|---|---|
| v20260708-0003-replace_property_refs-ignores-the-root-of-.patch | text/x-patch | 8.3 KB |
| v20260708-0002-Resolve-unknown-type-literals-in-property-.patch | text/x-patch | 20.5 KB |
| v20260708-0001-Report-duplicate-property-and-label-names-.patch | text/x-patch | 6.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kyotaro Horiguchi | 2026-07-08 07:00:52 | Re: Mark class_descr strings for translation |
| Previous Message | solai v | 2026-07-08 06:52:28 | Re: Proposal: INSERT ... BY NAME |