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.
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.
> 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.
Regards,
Henson
