Re: SQL Property Graph Queries (SQL/PGQ)

Поиск
Список
Период
Сортировка
От Ashutosh Bapat
Тема Re: SQL Property Graph Queries (SQL/PGQ)
Дата
Msg-id CAExHW5sr+dJPCFw2OqXUfPPqPks8qmsivaAYhRiBdFANcX8RHw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: SQL Property Graph Queries (SQL/PGQ)  (Peter Eisentraut <peter@eisentraut.org>)
Список pgsql-hackers
On Wed, Dec 17, 2025 at 2:28 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 17.12.25 06:32, Ashutosh Bapat wrote:
> > On Mon, Dec 15, 2025 at 6:43 PM Ashutosh Bapat
> > <ashutosh.bapat.oss@gmail.com> wrote:
> >>
> >> Rebased patches on the latest HEAD which required me to move
> >> graph_table.sql to another parallel group.
> >
> > Huh, the movement resulted in losing that test from parallel_schedule.
> > Fixed in the attached patches.
>
> A couple of items:
>
> 1) I was running some tests that involved properties with mismatching
> typmods, and I got an error message like
>
> ERROR:  property "p1" data type modifier mismatch: 14 vs. 19
>
> but the actual types were varchar(10) and varchar(15).  So to improve
> that, we need to run these through the typmod formatting routines, not
> just print the raw typmod numbers.  I actually just combined that with
> the check for the type itself.  Also, there was no test covering this,
> so I added one.  See attached patches.

+1. The error message is better.

>
> I did another investigation about whether this level of checking is
> necessary.  I think according to the letter of the SQL standard, the
> typmods must indeed match.  It seems Oracle does not check (the example
> mentioned above came from an Oracle source).  But I think it's okay to
> keep the check.  In PostgreSQL, it is much less common to write like
> varchar(1000).  And we can always consider relaxing it later.

+1.

Attached patch adds a couple more test statements.

>
> 2) I had it in my notes to consider whether we should support the colon
> syntax for label expressions.  I think we might have talked about that
> before.
>
> I'm now leaning toward not supporting it in the first iteration.  I
> don't know that we have fully explored possible conflicts with host
> variable syntax in ecpg and psql and the like.  Maybe avoid that for now.
>

I was aware of ecpg and I vaguely remember we fixed something in ECPG
to allow : in MATCH statement. Probably following changes in
psqlscan.l and pgc.l
-self                   [,()\[\].;\:\+\-\*\/\%\^\<\>\=]
+self                   [,()\[\].;\:\|\+\-\*\/\%\^\<\>\=]

Those changes add | after : (and not the : itself) so maybe they are
not about supporting : . Do you remember what those are?

However, I see that : in psql can be a problem
#create table t1 (a int, b int);
#create property graph g1 vertex tables (t1 key (a));
#select * from GRAPH_TABLE (g1 MATCH (a :t1) COLUMNS (a.a));
 a
---
(0 rows)

#\set t1 blank
#select * from GRAPH_TABLE (g1 MATCH (a :t1) COLUMNS (a.a));
ERROR:  syntax error at or near "blank"
LINE 1: select * from GRAPH_TABLE (g1 MATCH (a blank) COLUMNS (a.a))...

> There was also a bit of an inconsistency in the presentation: The
> documentation introduced the colon as seemingly the preferred syntax,
> but ruleutils.c dumped out the IS syntax.
>
> (It was also a bit curious that some test code put spaces around the
> colon, which is not idiomatic.)
>

That might have been just me trying to type one letter less and yet
keeping the query readable. A column between variable name and the
label is not always noticeable.

> Attached is a patch that shows how to revert the colon support.  It's
> pretty simple, and it would be easy to add it back in later, I think.

I agree that it's better not to support it now. It requires more work
and it's optional. When we come to support it, we will need thorough
testing.

I spotted some examples that use : in ddl.sgml.
<programlisting>
SELECT customer_name FROM GRAPH_TABLE (myshop MATCH
(c:customer)-[:has]->(o:"order" WHERE o.ordered_when = current_date)
COLUMNS (c.name AS customer_name));
</programlisting>

The query demonstrates that one can use label names in a way that will
make the pattern look like an English sentence. Replacing : with IS
defeats that purpose.

As written in that paragraph, the labels serve the purpose of exposing
the table with a different logical view (using different label and
property names). So we need that paragraph, but I think we should
change the example to use IS instead of :. Attached is suggested
minimal change, but I am not happy with it. Another possibility is we
completely remove that paragraph; I don't think we need to discuss all
possible usages the users will come up with.

The patch changes one more instance of : by IS. But that's straight forward.

In ddl.sgml I noticed a seemingly incomplete sentence
   A property graph is a way to represent database contents, instead of using
   relational structures such as tables.

Represent the contents as what? I feel the complete sentence should be
one of the following
property graph is a way to represent database contents as a graph,
instead of representing those as relational structures OR
property graph is another way to represent database contents instead
of using relational structures such as tables

But I can't figure out what was originally intended.

I have squashed 0002 from my earlier patchset and your 3 patches into 0001.

0002 has extra tests mentioned above. It also removes "TODO: dubious
error message" from a comment. I don't see anything dubious in the
error message. I think this patch is safe to be merged into 0001.

0003 is changed to ddl.sgml. Those need a bit more work as described above.

--
Best Wishes,
Ashutosh Bapat

Вложения

В списке pgsql-hackers по дате отправления: