Wrong query result w/ propgraph single lateral col reference
I had Opus 4.8 look for user-visible defects in SQL/PGQ (commit 2f094e7).
Setup:
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);
}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
3. [cosmetic] Invalid DROP PROPERTIES diagnosed via internal error message
CREATE PROPERTY GRAPH g2 VERTEX TABLES (
v KEY (id) LABEL la PROPERTIES (flag) LABEL lb PROPERTIES (name));
ALTER PROPERTY GRAPH g2 ALTER VERTEX TABLE v ALTER LABEL la DROP PROPERTIES (name);
-- ERROR: could not find tuple for label property 0
Opus says propoid is found graph wide, but the label property oid is not
checked before performDeletion(), so the intended "label has no property"
error never fires.
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.
On Tue, Jun 30, 2026 at 11:00 PM Noah Misch <noah@leadboat.com> wrote:
I had Opus 4.8 look for user-visible defects in SQL/PGQ (commit 2f094e7).
Setup: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 0Adding "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 01. 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: 63Adding "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 2For (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.
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
/messages/by-id/CAHg+QDcyKNWyzDoKMxiZNjv7C-wAxs8y0ZoNkOV137Y+nk3UXg@mail.gmail.com.
3. [cosmetic] Invalid DROP PROPERTIES diagnosed via internal error message
CREATE PROPERTY GRAPH g2 VERTEX TABLES (
v KEY (id) LABEL la PROPERTIES (flag) LABEL lb PROPERTIES (name));
ALTER PROPERTY GRAPH g2 ALTER VERTEX TABLE v ALTER LABEL la DROP PROPERTIES (name);
-- ERROR: could not find tuple for label property 0Opus says propoid is found graph wide, but the label property oid is not
checked before performDeletion(), so the intended "label has no property"
error never fires.
This has been already reported and being discussed in [1]/messages/by-id/1DA5D52A-4AFA-426E-83F7-42ED974D682B@gmail.com
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().
[1]: /messages/by-id/1DA5D52A-4AFA-426E-83F7-42ED974D682B@gmail.com
--
Best Wishes,
Ashutosh Bapat
On Wed, Jul 1, 2026 at 10:26 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
On Tue, Jun 30, 2026 at 11:00 PM Noah Misch <noah@leadboat.com> wrote:
I had Opus 4.8 look for user-visible defects in SQL/PGQ (commit 2f094e7).
Setup: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 0Adding "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 01. 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: 63Adding "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 2For (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.
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 textI think property's data type should be set to text, not unknown. I
think we should combine the fix for this in
/messages/by-id/CAHg+QDcyKNWyzDoKMxiZNjv7C-wAxs8y0ZoNkOV137Y+nk3UXg@mail.gmail.com.
Attaching the fix here. But I will also report it on the other thread
[2]: /messages/by-id/CAHg+QDcyKNWyzDoKMxiZNjv7C-wAxs8y0ZoNkOV137Y+nk3UXg@mail.gmail.com.
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().
Fixed this as well.
I generate all the patches for SQL/PGQ bug fixes from the same branch.
These patches modify the same test and code files and thus conflict
with each other. Since we are talking about multiple issues here, I am
attaching all the patches that may be required to be applied before
applying the fixes being discussed here. I will highlight the patches
that actually fix the issues discussed here and the conflicting
patches. Ofc, the patches can be applied without conflict in the order
in which they are numbered. Each patch also has a link to the thread
which discusses the corresponding issue.
0011 fixes issues 0 and 1. The conflict is in changes to
graph_table.sql. You will need 0005 onwards patches to apply this
patch.
0005 fixes issue 2
0004 fixes issue 3
0003 fixes issue 4. Patches 0001 and 0002 are required to apply this
patch. They fix issues being discussed in [1]/messages/by-id/CAHg+QDeP=mTHTV48R23zKMy1SBmCKZ_L7-z5zKnYyw+K0x-gCg@mail.gmail.com. 0002 adds a test file
which 0003 uses.
[1]: /messages/by-id/CAHg+QDeP=mTHTV48R23zKMy1SBmCKZ_L7-z5zKnYyw+K0x-gCg@mail.gmail.com
[2]: /messages/by-id/CAHg+QDcyKNWyzDoKMxiZNjv7C-wAxs8y0ZoNkOV137Y+nk3UXg@mail.gmail.com.
--
Best Wishes,
Ashutosh Bapat
Attachments:
v20260703-0011-replace_property_refs-ignores-the-root-of-.patchtext/x-patch; charset=US-ASCII; name=v20260703-0011-replace_property_refs-ignores-the-root-of-.patchDownload+41-16
v20260703-0010-Using-GRANT-.-TABLE-command-on-a-property-.patchtext/x-patch; charset=US-ASCII; name=v20260703-0010-Using-GRANT-.-TABLE-command-on-a-property-.patchDownload+32-2
v20260703-0009-Prohibit-Locking-Clauses-on-GRAPH_TABLE.patchtext/x-patch; charset=US-ASCII; name=v20260703-0009-Prohibit-Locking-Clauses-on-GRAPH_TABLE.patchDownload+31-2
v20260703-0007-Empty-label-expression-in-view-definition.patchtext/x-patch; charset=US-ASCII; name=v20260703-0007-Empty-label-expression-in-view-definition.patchDownload+178-64
v20260703-0008-View-referencing-labels-shared-by-vertex-a.patchtext/x-patch; charset=US-ASCII; name=v20260703-0008-View-referencing-labels-shared-by-vertex-a.patchDownload+33-1
v20260703-0005-Resolve-unknown-type-literals-in-property-.patchtext/x-patch; charset=US-ASCII; name=v20260703-0005-Resolve-unknown-type-literals-in-property-.patchDownload+52-9
v20260703-0006-pg_propgraph_property-entries-orphaned-by-.patchtext/x-patch; charset=US-ASCII; name=v20260703-0006-pg_propgraph_property-entries-orphaned-by-.patchDownload+20-14
v20260703-0002-Test-concurrent-modifications-to-a-propert.patchtext/x-patch; charset=US-ASCII; name=v20260703-0002-Test-concurrent-modifications-to-a-propert.patchDownload+186-1
v20260703-0004-Dropping-a-property-not-associated-with-th.patchtext/x-patch; charset=US-ASCII; name=v20260703-0004-Dropping-a-property-not-associated-with-th.patchDownload+22-29
v20260703-0003-Report-duplicate-property-and-label-names-.patchtext/x-patch; charset=US-ASCII; name=v20260703-0003-Report-duplicate-property-and-label-names-.patchDownload+140-4
v20260703-0001-Prevent-dropping-the-last-label-from-a-pro.patchtext/x-patch; charset=US-ASCII; name=v20260703-0001-Prevent-dropping-the-last-label-from-a-pro.patchDownload+62-7
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.bapat.oss@gmail.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 0Adding "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 01. 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: 63Adding "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 2For (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 textI think property's data type should be set to text, not unknown. I
think we should combine the fix for this in
/messages/by-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
Attachments:
v20260708-0003-replace_property_refs-ignores-the-root-of-.patchtext/x-patch; charset=US-ASCII; name=v20260708-0003-replace_property_refs-ignores-the-root-of-.patchDownload+41-16
v20260708-0002-Resolve-unknown-type-literals-in-property-.patchtext/x-patch; charset=US-ASCII; name=v20260708-0002-Resolve-unknown-type-literals-in-property-.patchDownload+36-9
v20260708-0001-Report-duplicate-property-and-label-names-.patchtext/x-patch; charset=US-ASCII; name=v20260708-0001-Report-duplicate-property-and-label-names-.patchDownload+60-4
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 textI think property's data type should be set to text, not unknown. I
think we should combine the fix for this in
/messages/by-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?
On Wed, Jul 8, 2026 at 2:35 PM Peter Eisentraut <peter@eisentraut.org> wrote:
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?
I chose to perform the checks in insert_*_record() functions to keep
them at a central place instead of dispersing all over like the
check_* functions. The latter need to peform their checks considering
the overall shape of the property graph, however, the specification
checks are fairly local - like duplicate property or label names
specified in the same command or duplicate labels being inserted -
they won't usually need checks across labels or elements for example.
I am afraid we might have to sprinkle pre_check_* functions at
multiple places - thus leading to a risk of missing places as this
code evolves.
Do you expect insert_element_record() to perform sanity checks of
labels or insert_label_record() to perform sanity checks on
properties? Or do you expect a hierarchy of pre_check_ functions
cascading from elements to labels to properties?
--
Best Wishes,
Ashutosh Bapat
On Wed, Jul 8, 2026 at 6:59 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
On Wed, Jul 8, 2026 at 2:35 PM Peter Eisentraut <peter@eisentraut.org> wrote:
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?
If we consolidate all the pre-checks together similar to post checks,
we duplicate the catalog lookups. But a future code may miss calling
pre-checks before calling insert_*_record(). I tried to write one
precheck function for insert_label_record and insert_property_record
respectively and ended up calling it close to the insert_*_record
itself. The effort is attached as
v20260716-0001-pre-check-Report-duplicate-property-and-label-names-.patch.nocibot
I don't like the result.
Do you expect insert_element_record() to perform sanity checks of
labels or insert_label_record() to perform sanity checks on
properties? Or do you expect a hierarchy of pre_check_ functions
cascading from elements to labels to properties?
If I understand your intention to keep functions inserting records
into catalogs simple, Attached
v20260716-0001-simple-insert-Report-duplicate-property-and-label-names.patch.nocibot
is what I could come up with. It adds one insert_*_record() function
for every property graph catalog, and then an add_* functions which
call the insert_*_record functions after necessary pre-checks. I like
this version better. It doesn't collect all the checks like check_*
functions but create a hierarchy of checks and catalog inserts keeping
the latter simple. I have also rearranged the code so that all the
catalog insert functions are together and their wrappers are together
respectively.
--
Best Wishes,
Ashutosh Bapat