Hi! On Wed, Jul 29, 2020 at 2:39 PM Sergei Golubchik <serg@mariadb.org> wrote:

Hi, Oleksandr!

On Jul 29, Oleksandr Byelkin wrote:

...

revision-id: 7311586ca25 (mariadb-10.2.31-317-g7311586ca25) parent(s): a1e52e7f32c author: Oleksandr Byelkin <sanja@mariadb.com> committer: Oleksandr Byelkin <sanja@mariadb.com> timestamp: 2020-07-17 13:47:26 +0200 message:

MDEV-15703 Crash in EXECUTE IMMEDIATE 'CREATE OR REPLACE TABLE t1 (a INT DEFAULT ?)' USING DEFAULT

Part 1: make better asserts.

diff --git a/sql/field.cc b/sql/field.cc index 65bd9d22857..cf0cc655888 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -11138,6 +11138,16 @@ bool Field::save_in_field_default_value(bool view_error_processing) { THD *thd= table->in_use;

+ if ( // table is not opened properly + table == NULL || table->pos_in_table_list == NULL || + table->pos_in_table_list->select_lex == NULL || + // we are in subquery/derived/CTE + !table->pos_in_table_list->top_table()->select_lex->is_top_level_select()) + { + DBUG_ASSERT(0); // shoud not happened + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); + return false; + }

that's kind of questionable. if this condition can never be true, why do you have my_error() there? Just do asserts, like

DBUG_ASSERT(table); DBUG_ASSERT(table->pos_in_table_list); DBUG_ASSERT(table->pos_in_table_list->select_lex);

DBUG_ASSERT(table->pos_in_table_list->top_table()->select_lex->is_top_level_select());

It was the first solution to catch the problem on the low level, now we catch it earlier and in many places, but if we forget something on non-debug builds it will just work.

...

if (flags & NO_DEFAULT_VALUE_FLAG && real_type() != MYSQL_TYPE_ENUM) { @@ -11177,12 +11187,29 @@ bool Field::save_in_field_default_value(bool view_error_processing) bool Field::save_in_field_ignore_value(bool view_error_processing) { enum_sql_command com= table->in_use->lex->sql_command; + + if ( // table is not opened properly + table == NULL || table->pos_in_table_list == NULL || + table->pos_in_table_list->select_lex == NULL || + // we are in subquery/derived/CTE + !table->pos_in_table_list->top_table()->select_lex->is_top_level_select()) + { + DBUG_ASSERT(0); // shoud not happened + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); + return false; + }

ditto

the same as above

...

// All insert-like commands if (com == SQLCOM_INSERT || com == SQLCOM_REPLACE || com == SQLCOM_INSERT_SELECT || com == SQLCOM_REPLACE_SELECT || com == SQLCOM_LOAD) return save_in_field_default_value(view_error_processing); - return 0; // ignore + if (com == SQLCOM_UPDATE || com == SQLCOM_UPDATE_MULTI) + return 0; // ignore + + // unexpected command + DBUG_ASSERT(0); // shoud not happened

ditto

but why it should not happen? What if one does SELECT DEFAULT(f) FROM t?

we think that we caught all cases with: 1) calls of Item_(param|default|ignore)::val_* (it is old way, was here for long time) 2) checks Item::is_evaluable_expression() before save_in_fields() where it is possible

...

+ my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); + return false; }

diff --git a/sql/item.cc b/sql/item.cc index 9451d4203ca..382eb3ac47a 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -3930,13 +3930,19 @@ int Item_param::save_in_field(Field *field, bool no_conversions) case NULL_VALUE: return set_field_to_null_with_conversions(field, no_conversions); case DEFAULT_VALUE: - return field->save_in_field_default_value(field->table->pos_in_table_list-> + return field->save_in_field_default_value((field->table && + field->table->pos_in_table_list)?

in what case can field->table be NULL here?

probably impossible (I thought it is for fake fields in SP, but it looks like they have fake table also)

...

+ field->table->pos_in_table_list-> top_table() != - field->table->pos_in_table_list); + field->table->pos_in_table_list: + FALSE); case IGNORE_VALUE: - return field->save_in_field_ignore_value(field->table->pos_in_table_list-> + return field->save_in_field_ignore_value((field->table && + field->table->pos_in_table_list)? + field->table->pos_in_table_list-> top_table() != - field->table->pos_in_table_list); + field->table->pos_in_table_list: + FALSE); case NO_VALUE: DBUG_ASSERT(0); // Should not be possible return true;

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org