Allow Complex Parsing of Functions #1200
Conversation
|
LOL! The CI server is too slow for the performance test timeouts. |
|
Building an upper boundary is quite a good idea. Could you please resolve the conflicts. I already merged your case when pr. My first changes for adaption the ValuesStatement are included as well. Maybe we could skip those MultiExpressionLists, because they are handles now by SimpleExpressonLists as well. |
…plexParsingClean Conflicts: src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt src/test/java/net/sf/jsqlparser/statement/select/NestedBracketsPerformanceTest.java src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java
|
Merged the latest Master Branch. |
|
So could you somehow better document, how the nesting depth is calculated or modified? |
|
BTW I check your modifications. Since complex parsing is now allowed for functions I removed FunctionWithCondParams. But now your depth of seven is not enough for multiple tests. Unfortunately I have problems for accepting one test. |
It is a simple count of the maximum Open Bracket
Out of my head, I foresee a situation where you have a deep nesting on a function (without complex expressions) and one complex expression somewhere else (without deep nesting). That would fail because Complex Parsing and Deep Nesting are now mutual exclusive. However, I kept experimenting when addressing the Please let me know your particular failing test case and I will have a look at it promptly. |
|
@wumpz: You have modified the Brackets Threshold for Complex Parsing from 7 to 10. Unfortunately, the 7 has been selected for a purpose: It is the point when the performance impact LOOKAHEAD amplifies too much. This was also the reason why I have had defined So, can we set the Threshold back to 7 again please? I am still working on a more fine granular solution, where the nesting is determined as per expression. |
|
Understand. The problem is, you are calculating the "complexity" of a statement via running through the parenthesis. So you are taking parenthesises into account that have nothing to do with complex expressions. Somehow we should count the deepness within the productions. I don't know, if that is even possible and limit there to a level of 7 or 10. |
|
It is not perfect, but it solves maybe 90% of the "real life" problems.
When my PRs are merged then I would like to count the real nesting in
the Primary Expression, same as I did for the CASE statement.
Problem is really that I have so many loose ends right now because the
acceptance is a bit slow.
…On Wed, 2021-06-02 at 05:05 -0700, Tobias wrote:
Understand. The problem is, you are calculating the "complexity" of a
statement via running through the parenthesis. So you are taking
parenthesises into account that have nothing to do with complex
expressions. Somehow we should count the deepness within the
productions. I don't know, if that is even possible and limit there
to a level of 7 or 10.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
Critics acknowledged. I try to improve. |
|
Does this case counting work for lookaheads as well? If I remember right, this case deepness was never checked, or am I wrong? |
|
On Wed, 2021-06-02 at 05:22 -0700, Tobias wrote:
Does this case counting work for lookaheads as well?
Yes.
If I remember right, this case deepness was never checked, or am I
wrong?
It was not needed for the cases but you see a similar preparation:
1) we have a global atomic LEVEL COUNT variable (this works well,
because the parser works strictly LL)
2) when entering a Primary Expression we count up
3) when leaving the Primary Expression we count down
This will give us the true nesting level.
We will stop for looking for Complex Expressions below a certain
threshold.
I am keen to implement and test and benchmark this better logic as soon
as everything else has been tied up.
After that we can also look again if JavaCC21 can help anything. or
not.
Cheers!
|
This PR will determine the nested brackets "(" ")" and configure the Parser in order to parse Complex Function Parameters like
to_char( a = b)and Named Function Parameters liketrim(BOTH ' ' from 'foo bar ')ONLY WHEN the level of nesting functions is below a threshold = 7. Although this is a configurable Parser Feature.This massively speeds up the parser. The whole
NestedBracketsPerformanceTestSuite takes less than 3 seconds now, with 50! iterations for the concat test.At the same time we can parse Complex Expression Parameters for all Functions (but only when there is no deep recursion of more than 9 levels although this detail is mood because it would not work in anyway because of the slowdown).
Fixes issues #1190 #1103
This is a CLEAN PR handcrafted against the latest Upstream MASTER, containing only the minimal changes.