Hello Sanja, On 04/10/2018 03:37 PM, Oleksandr Byelkin wrote:
Am 10.04.2018 um 10:58 schrieb Alexander Barkov:
Hello Sanja,
I reviewed your recent changes in "10.3-MDEV-11953" (and the attached additional patch for sql_yacc_ora.yy)
I have some proposals:
1. Can you please move huge pieces of the code from sql_yacc.yy to LEX or other relevant classes?
It makes the grammar much more readable (patches are aslo much more readable).
I'd move the relevant pieces of the code to LEX as a separate patch, even before fixing the grammar. OK
2. You're adding too many main_select_push() and pop_select(). Please move them to upper level rules (it should be possible in many cases). Impossible
It's possible. I easily removed 15 push/pop pairs. But it does not look nice either. It's easier to read when all "create" alternatives reside in the same "create" rule. Ok. I won't insist on that. Just adding "expr" with push/pop should be good enough.
Add new helper rules when needed. For example, this piece of code repeats many times:
+ { + if (Lex->main_select_push()) + MYSQL_YYABORT; + } + expr + { + Lex->pop_select(); //main select + $$= $3;
It deserved a rule, say, expr_with_select_push_pop. You can find a better name :)
OK
- Serg and I spent a lot of time working on this task: MDEV-8909 union parser cleanup (and its 13 dependency tasks, and 3 related tasks, see the "Issue links" section in MDEV).
We think that it should be the parser who disallows bad grammar, instead of post-analysis with raising errors like ER_CANT_USE_OPTION_HERE. Please keep using the same approach. The task did not made parser recognizing brackets, and I have no ideas how to return parser errors when all SELECT parsed in the same way in difference from the previous parser which could recognize only one level of SELECTs.
Can you give an example of a query that is not parsed by the current 10.3 parser, but is parsed in 10.3-MDEV-11953 ? Thanks.
Thanks!
[skip]