#1228 - ArangoDb - grammar and tests

[mlorek]

No description provided.

[KvanTTT]

Thank you for the new grammar but please factor common parts and fix formatting.

[mlorek]

php build fails with

[KvanTTT]

@kaby76 could you please take a look at PHP build? Does it make sense to restart the build?

[kaby76]

The PHP target for this grammar uses more than the available memory in PHP 8.1.12. If it doesn't work for that version of PHP, then it won't work for 8.0.x, which is being used in the CI build. In this case, php doesn't terminate. Fortunately, we have the trwdog app making sure this doesn't kill the build so much that intervention is required.

But, the target doesn't work well with PHP. I suggest you just add "aql" to the skip list for PHP. Doesn't matter where, but in alphabetic order is preferred.

Now, as far as what the parser fails on is "for.aql". This is a 2-line query. The grammar is ambiguous for expression, requiring a very, very large read ahead to determine. I think the grammar has some problems. But, this grammar should not fail like this for PHP. An issue should be created in antlr/antlr4 to determine why it's so bad.

[kaby76]

OK, the problem with this grammar is the choice between expr : ... expr (',' expr)+ ... and expr : ... expr '[' (expr_list | '*') ']' ... in which we have expr_list : expr (',' expr)*;. It cannot decide where the expr ends and where the next expr begins in expr_list. (Classic problem seen elsewhere. This is why Intellij with the Antlr Plugin becomes so essential to fix the grammar. lab.antlr.org also kind of shows the problem, but doesn't have the nice UI on the input pane like the Intellij editor does that notes which alts the AdaptivePredict() function is finding the ambiguity. @parrt ) Suggest the alt be expr '[' (expr | '*') ']'.

Change the expr rule or expr_list rule (maybe just remove?), and try seeing if that fixes the build. If not, just add aql to the skip list so it's not tested with PHP.

[kaby76]

Yes, this looks good. Does it work with "mvn clean test"? If so, then this should work for PHP--and speed up all the other targets as well.

[mlorek]

Yes, this looks good. Does it work with "mvn clean test"? If so, then this should work for PHP--and speed up all the other targets as well.

it did work locally

[kaby76]

Oh well--still doesn't work for PHP. After the build settles down, add aql to the skip-php.txt file to get past this problem for now.

[kaby76]

I added a "-d memory_limit=4G" option to the call to php on the command line, and although it runs longer, it still fails with a timeout. All tests parse fine except for.aql, and quickly. So, there is something quite wrong with the PHP target, the great joy of working with a type-less language. This needs to be raised in github.com/antlr/antlr4/issues. Working with 4.11.1 and PHP 8.1.12

[kaby76]

The PR was merged, but the build was still broken. skip-php.txt should contain aql. I'll submit a PR to fix this, and an issue in antlr/antlr4 for the problem in parsing for.aql.

[kaby76]

The out of memory condition for aql/for.aql has been observed in another grammar. antlr/antlr-php-runtime#12 (comment) Tracking it down. (We could change the grammar, but then the bug will still be there.)