From 106f0b5798a2b5d13b7d67c3cc678fc0cc2184c2 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 5 Jun 2018 10:25:39 +0400 Subject: MDEV-16385 ROW SP variable is allowed in unexpected context The problem described in the bug report happened because the code did not test check_cols(1) after fix_fields() in a few places. Additionally, fix_fields() could be called multiple times for SP variables, because they are all fixed at a early stage in append_for_log(). Solution: 1. Adding a few helper methods - fix_fields_if_needed() - fix_fields_if_needed_for_scalar() - fix_fields_if_needed_for_bool() - fix_fields_if_needed_for_order_by() and using it in many cases instead of fix_fields() where the "fixed" status is not definitely known to be "false". 2. Adding DBUG_ASSERT(!fixed) into Item_splocal*::fix_fields() to catch double execution. 3. Adding tests. As a good side effect, the patch removes a lot of duplicate code (~60 lines): if (!item->fixed && item->fix_fields(..) && item->check_cols(1)) return true; --- sql/set_var.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'sql/set_var.cc') diff --git a/sql/set_var.cc b/sql/set_var.cc index 7bf6b9f928d..c9dfeb3e211 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -768,8 +768,7 @@ int set_var::check(THD *thd) if (!value) return 0; - if ((!value->fixed && - value->fix_fields(thd, &value)) || value->check_cols(1)) + if (value->fix_fields_if_needed_for_scalar(thd, &value)) return -1; if (var->check_update_type(value)) { @@ -803,8 +802,7 @@ int set_var::light_check(THD *thd) if (type == OPT_GLOBAL && check_global_access(thd, SUPER_ACL)) return 1; - if (value && ((!value->fixed && value->fix_fields(thd, &value)) || - value->check_cols(1))) + if (value && value->fix_fields_if_needed_for_scalar(thd, &value)) return -1; return 0; } -- cgit v1.2.1