public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: Ashutosh Bapat <[email protected]>
To: Noah Misch <[email protected]>
Cc: [email protected]
Subject: Re: Wrong query result w/ propgraph single lateral col reference
Date: Wed, 8 Jul 2026 11:05:40 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAExHW5sgk_QrxY475uYqrPyW_tMripfM3Z2OoUkkmkj8E_vECg@mail.gmail.com>
References: <[email protected]>
	<CAExHW5sjypF-x1rcutfwxafBxw1mKt4FAQRMFTafMQaZQXAUAg@mail.gmail.com>
	<CAExHW5tdnJDhUUXZ9V8n7aBnW2hqRC0MZgsHnuZj092om3PhQg@mail.gmail.com>
	<CAExHW5sgk_QrxY475uYqrPyW_tMripfM3Z2OoUkkmkj8E_vECg@mail.gmail.com>

On 08.07.26 08:54, Ashutosh Bapat wrote:
>>>> --- 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.

committed

>>>> 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/[email protected]....
>>>
>>
>> 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.

committed

>>>> 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.

I would like this code to be organized differently.  Note that the 
existing insert_*_record functions don't do any error checking; they are 
just there to make catalog modifications.  There are various check_* 
functions that pretty much correspond to syntax rule checks, but these 
are run after the catalog changes, hence the present issue.  Maybe we 
should have a set of "pre-check" functions in addition?

view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Wrong query result w/ propgraph single lateral col reference
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox