[Maria-developers] Please review MDEV-13788 Server crash when issuing bad SQL partition syntax
Hello Sergei, Can you please review a patch for MDEV-13788, for 5.5? Thanks!
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<part_elem_value> 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 security@mariadb.org
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<part_elem_value> 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 security@mariadb.org
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<part_elem_value> 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 security@mariadb.org
participants (2)
-
Alexander Barkov -
Sergei Golubchik