Re: [Maria-developers] Please review MDEV-13788 Server crash when issuing bad SQL partition syntax
Hi, Alexander! Fair enough. Ok to push On Nov 16, Alexander Barkov wrote: > On 11/16/2017 08:27 AM, Sergei Golubchik wrote: > > On Nov 15, Alexander Barkov wrote: > >> diff --git a/sql/partition_info.cc b/sql/partition_info.cc > >> index 512bf29..740e508 100644 > >> --- a/sql/partition_info.cc > >> +++ b/sql/partition_info.cc > >> @@ -2221,6 +2239,8 @@ int partition_info::fix_parser_data(THD *thd) > >> part_elem= it++; > >> List_iterator list_val_it(part_elem->list_val_list); > >> num_elements= part_elem->list_val_list.elements; > >> +if (!num_elements && error_if_requires_values()) > >> + DBUG_RETURN(true); > > > > I thought the parser was supposed to ensure that VALUES was used where > > needed, so there should be no need to do additional checks here. > > > > Why does the parser allow invalid syntax? > > Unlike, CREATE TABLE, there's nothing we can do during parsing for ALTER > TABLE. > > This syntax is generally valid, e.g. for HASH partitioning: > > ALTER TABLE t1 REORGANIZE PARTITION p1 INTO > ( > PARTITION p2, > PARTITION p3 > ); > > It's not valid only for RANGE or LIST partitioning. > > The current partition type becomes known only after parsing. > So I extended partition_info::fix_parser_data() to validate > the value list, depending on the partitioning type. > Regards, Sergei Chief Architect MariaDB and secur...@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
Re: [Maria-developers] Please review MDEV-13788 Server crash when issuing bad SQL partition syntax
Hi Sergei, On 11/16/2017 08:27 AM, Sergei Golubchik wrote: > Hi, Alexander! > > On Nov 15, Alexander Barkov wrote: >> diff --git a/sql/partition_info.cc b/sql/partition_info.cc >> index 512bf29..740e508 100644 >> --- a/sql/partition_info.cc >> +++ b/sql/partition_info.cc >> @@ -2221,6 +2239,8 @@ int partition_info::fix_parser_data(THD *thd) >> part_elem= it++; >> List_iterator list_val_it(part_elem->list_val_list); >> num_elements= part_elem->list_val_list.elements; >> +if (!num_elements && error_if_requires_values()) >> + DBUG_RETURN(true); > > I thought the parser was supposed to ensure that VALUES was used where > needed, so there should be no need to do additional checks here. > > Why does the parser allow invalid syntax? Unlike, CREATE TABLE, there's nothing we can do during parsing for ALTER TABLE. This syntax is generally valid, e.g. for HASH partitioning: ALTER TABLE t1 REORGANIZE PARTITION p1 INTO ( PARTITION p2, PARTITION p3 ); It's not valid only for RANGE or LIST partitioning. The current partition type becomes known only after parsing. So I extended partition_info::fix_parser_data() to validate the value list, depending on the partitioning type. > >> DBUG_ASSERT(part_type == RANGE_PARTITION ? >> num_elements == 1U : TRUE); >> for (j= 0; j < num_elements; j++) > > Regards, > Sergei > Chief Architect MariaDB > and secur...@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
Re: [Maria-developers] Please review MDEV-13788 Server crash when issuing bad SQL partition syntax
Hi, Alexander! On Nov 15, Alexander Barkov wrote: > diff --git a/sql/partition_info.cc b/sql/partition_info.cc > index 512bf29..740e508 100644 > --- a/sql/partition_info.cc > +++ b/sql/partition_info.cc > @@ -2221,6 +2239,8 @@ int partition_info::fix_parser_data(THD *thd) > part_elem= it++; > List_iterator list_val_it(part_elem->list_val_list); > num_elements= part_elem->list_val_list.elements; > +if (!num_elements && error_if_requires_values()) > + DBUG_RETURN(true); I thought the parser was supposed to ensure that VALUES was used where needed, so there should be no need to do additional checks here. Why does the parser allow invalid syntax? > DBUG_ASSERT(part_type == RANGE_PARTITION ? > num_elements == 1U : TRUE); > for (j= 0; j < num_elements; j++) Regards, Sergei Chief Architect MariaDB and secur...@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
