Re: Wrong query result w/ propgraph single lateral col reference

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

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?