Re: [Maria-developers] 4c263b6d30b: MDEV-20632: Recursive CTE cycle detection using CYCLE clause
Hi, Oleksandr! On Feb 26, Oleksandr Byelkin wrote:
On Wed, Feb 26, 2020 at 1:44 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sanja!
_some_ comments are below.
The main thing, I still don't understand your changes in sql_select.cc.
Why did you create separate counters and code paths with if() for CYCLE? I'd think it could be just a generalization of DISTINCT
DISTINCT makes all fields distinct, except hidden, I need also except come other (or better say by list of not hidden)
Exactly. I thought DISTINCT will be just CYCLE with an empty list of non-distinct columns. Or something like that. Not like if (cycle) { cycle code } else { distinct code }
--- a/sql/sql_cte.cc +++ b/sql/sql_cte.cc @@ -982,6 +982,38 @@ With_element::rename_columns_of_derived_unit(THD *thd,
are you sure what you're doing below can still be called "rename_columns_of_derived_unit" ?
I can rename it to prepare_olumns_of_derived_unit, is it OK?
anything you like :)
+opt_cycle: + /* empty */ + { $$= NULL; } + | + CYCLE_SYM + { + if (!Lex->curr_with_clause->with_recursive) + { + thd->parse_error(ER_SYNTAX_ERROR, $1.pos()); + } + } + '(' with_column_list ')'
Where did you see that the standard requires parentheses here?
Maybe in ORACLE docs... but without it it creates a lot of conflicts.
The standard says no parentheses. What kind of conflicts do you get? Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Am 26.02.20 um 22:15 schrieb Sergei Golubchik:
Hi, Oleksandr!
On Feb 26, Oleksandr Byelkin wrote:
On Wed, Feb 26, 2020 at 1:44 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sanja!
_some_ comments are below.
The main thing, I still don't understand your changes in sql_select.cc.
Why did you create separate counters and code paths with if() for CYCLE? I'd think it could be just a generalization of DISTINCT DISTINCT makes all fields distinct, except hidden, I need also except come other (or better say by list of not hidden) Exactly. I thought DISTINCT will be just CYCLE with an empty list of non-distinct columns. Or something like that. Not like
if (cycle) { cycle code } else { distinct code }
and how it will help if include/exclude decided on level of each field (when distinct is a global for whole )? I can of course write separate function or branch for cycle, but it will heavy duplicate code of distinct.
--- a/sql/sql_cte.cc +++ b/sql/sql_cte.cc @@ -982,6 +982,38 @@ With_element::rename_columns_of_derived_unit(THD *thd, are you sure what you're doing below can still be called "rename_columns_of_derived_unit" ? I can rename it to prepare_olumns_of_derived_unit, is it OK? anything you like :) OK
+opt_cycle: + /* empty */ + { $$= NULL; } + | + CYCLE_SYM + { + if (!Lex->curr_with_clause->with_recursive) + { + thd->parse_error(ER_SYNTAX_ERROR, $1.pos()); + } + } + '(' with_column_list ')' Where did you see that the standard requires parentheses here? Maybe in ORACLE docs... but without it it creates a lot of conflicts. The standard says no parentheses. What kind of conflicts do you get? shift-reduce conflicts
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik