On Tue, Mar 24, 2026 at 6:35 AM Henson Choi <assam258@gmail.com> wrote:
>
> Hi Ashutosh,
>
>> I have also added a few tests. I didn't add queries with all the
>> patterns you mentioned above. I tested a few by hand and all of them
>> worked as expected. Can you please check?
>
>
>
> I tested all the patterns and they all work correctly. No crashes,
> correct results.
>
Thanks a lot for the confirmation.
> One thing I noticed while reviewing the rewriter changes: the Assert
> at generate_queries_for_path_pattern() that checks alternating
> implicit/explicit elements doesn't actually work:
>
> #ifdef USE_ASSERT_CHECKING
> GraphElementPattern *prev_gep = NULL;
> #endif
> ...
> Assert(!prev_gep || prev_gep->implicit != gep->implicit);
>
> prev_gep is never updated in the loop -- it stays NULL throughout,
> so the Assert is always trivially true. It needs a
> "prev_gep = gep;" at the end of the loop body to actually perform
> the intended check.
>
Ah. Thanks for catching it. I think the Assertion is also not doing
what it mentions in the comment. It just checks whether the two
consecutive elements are not implicit ones; that doesn't necessarily
lead to an infinite or very long chain of implicit element patterns. I
have removed the assertion for now. We will add it if necessary, e.g.
when we start adding implicit edge patterns in which case the
possibility of long chains of implicit element patterns becomes real.
>>
>> Yes. That's a grammar issue. gram.y doesn't support it. Peter, do you
>> remember or know the reason why we don't support full edge left or
>> right? In fact, I am wondering what's the difference between full edge
>> left or right and full edge any direction?
>
>
>
> I looked into this. The lexer tokenizes "]->` as "]" + RIGHT_ARROW,
> so gram.y needs two alternatives -- just like the existing full edge
> right rule already does. The full edge left or right was simply
> missing both forms. Adding them fixes it:
>
> | '<' '-' '[' ... ']' '-' '>'
> | '<' '-' '[' ... ']' RIGHT_ARROW
>
> Per the standard, <-[]-> matches left or right direction while -[]-
> matches any direction. For simple directed graphs the results are
> the same, so EDGE_PATTERN_ANY seems like a reasonable mapping.
Our grammar is pretty tricky in this area especially because we can
treat a string of operators as a SQL operator. Peter may have
intentionally left it unsupported.
Here's the updated patchset.
--
Best Wishes,
Ashutosh Bapat
