From c44ddd95f466f479967bfc1c251a309ba12d621d Mon Sep 17 00:00:00 2001 From: Dave Gosselin Date: Fri, 8 May 2026 15:25:20 -0400 Subject: [PATCH] MDEV-35747: Wrong result from prepared TVC with parameter markers The setup of column type information in table_value_constr::prepare() was wrapped in an "if (!holders)" guard so that it runs only once per prepared statement. However, the guard was too wide because it bound the allocation of item holders (which should happen only once) to the collection of type information (which should happen on each execution). This leaves the TVC stuck with whatever placeholder type the parameter had at PREPARE time which may not match the type of the next substitution. A type holder has no actual type at PREPARE time. Its type only becomes known when a value is bound at EXECUTE time. So both the TVC types and the corresponding Item_type_holder instance in the SELECT item list must be computed again on every EXECUTE. This patch separates the work done once per prepared statement from the work done on every execution as implied above. --- mysql-test/main/table_value_constr.result | 38 ++++++++++ mysql-test/main/table_value_constr.test | 27 +++++++ sql/sql_class.h | 7 ++ sql/sql_tvc.cc | 89 ++++++++++++++++++++--- 4 files changed, 152 insertions(+), 9 deletions(-) diff --git a/mysql-test/main/table_value_constr.result b/mysql-test/main/table_value_constr.result index 43d1e1264a9c9..16d44e6bd8e2c 100644 --- a/mysql-test/main/table_value_constr.result +++ b/mysql-test/main/table_value_constr.result @@ -3279,3 +3279,41 @@ drop table t1,t2; # # End of 10.4 tests # +# +# MDEV-35747 Prepared statement with WITH ... AS (VALUES ... ) returns +# wrong result +# +# CTE whose body is a TVC with a parameter marker and an explicit column. +prepare stmt from 'with t(id) as (values (?)) select t.id from t'; +set @v= 1; +execute stmt using @v; +id +1 +execute stmt using @v; +id +1 +set @v= 'abc'; +execute stmt using @v; +id +abc +deallocate prepare stmt; +# Multi row VALUES with several parameter markers. +prepare stmt from 'with t(id) as (values (?), (?)) select t.id from t order by id'; +set @v1= 10, @v2= 20; +execute stmt using @v1, @v2; +id +10 +20 +execute stmt using @v1, @v2; +id +10 +20 +set @v1= 'aa', @v2= 'bb'; +execute stmt using @v1, @v2; +id +aa +bb +deallocate prepare stmt; +# +# End of 10.11 tests +# diff --git a/mysql-test/main/table_value_constr.test b/mysql-test/main/table_value_constr.test index 6fee71a5db879..70ac8adfa04cd 100644 --- a/mysql-test/main/table_value_constr.test +++ b/mysql-test/main/table_value_constr.test @@ -1824,3 +1824,30 @@ drop table t1,t2; --echo # --echo # End of 10.4 tests --echo # + +--echo # +--echo # MDEV-35747 Prepared statement with WITH ... AS (VALUES ... ) returns +--echo # wrong result +--echo # + +--echo # CTE whose body is a TVC with a parameter marker and an explicit column. +prepare stmt from 'with t(id) as (values (?)) select t.id from t'; +set @v= 1; +execute stmt using @v; +execute stmt using @v; +set @v= 'abc'; +execute stmt using @v; +deallocate prepare stmt; + +--echo # Multi row VALUES with several parameter markers. +prepare stmt from 'with t(id) as (values (?), (?)) select t.id from t order by id'; +set @v1= 10, @v2= 20; +execute stmt using @v1, @v2; +execute stmt using @v1, @v2; +set @v1= 'aa', @v2= 'bb'; +execute stmt using @v1, @v2; +deallocate prepare stmt; + +--echo # +--echo # End of 10.11 tests +--echo # diff --git a/sql/sql_class.h b/sql/sql_class.h index aa78312c3b6a5..7d7f8f5d1b596 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -8249,6 +8249,13 @@ class Type_holder: public Sql_alloc, bool aggregate_attributes(THD *thd) { static LEX_CSTRING union_name= { STRING_WITH_LEN("UNION") }; + /* + m_maybe_null is OR-accumulated and never reset here, so it is + sticky across calls. In a prepared statement context this means + that once any EXECUTE has observed a NULL for a given column, that + column stays nullable for the rest of the statement's lifetime + even if later EXECUTEs only bind non-NULL values. + */ for (uint i= 0; i < arg_count; i++) m_maybe_null|= args[i]->maybe_null(); return diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc index 3fa4675574c48..335f039dce16b 100644 --- a/sql/sql_tvc.cc +++ b/sql/sql_tvc.cc @@ -200,11 +200,15 @@ bool get_type_attributes_for_tvc(THD *thd, List_item *lst; li.rewind(); + /* + Reset arg_count on each call. The args[] buffer itself was allocated + on stmt_arena when the Type_holder[] array was first set up in + table_value_constr::prepare(), so we reuse that buffer across + executions of the prepared statement instead of allocating again on + every call. + */ for (uint pos= 0; pos < first_list_el_count; pos++) - { - if (holders[pos].alloc_arguments(thd, count_of_lists)) - DBUG_RETURN(true); - } + holders[pos].remove_arguments(); while ((lst= li++)) { @@ -269,17 +273,68 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl, if (fix_fields_for_tvc(thd, li)) DBUG_RETURN(true); + /* + Should be nullptr only during first PREPARE at which time it's + then allocated and will be set for the rest of the statement's + lifetime. + */ if (!holders) { DBUG_ASSERT(thd->stmt_arena->is_stmt_prepare_or_first_stmt_execute() || thd->stmt_arena->is_conventional()); - holders= type_holders= + /* + Build the Type_holder array in a local first. Only save it to + the persistent type_holders member once every holder has its + args[] buffer in place. If alloc_arguments() fails partway + through, then we must not leave type_holders pointing at a + partially initialized array. A later call would then take the + "already initialized" path, skip this block, and dereference a + NULL args[] inside add_argument(). + */ + Type_holder *new_holders= new (thd->active_stmt_arena_to_use()->mem_root) Type_holder[cnt]; - if (!holders || - join_type_handlers_for_tvc(thd, li, holders, cnt) || - get_type_attributes_for_tvc(thd, li, holders, - lists_of_values.elements, cnt)) + if (!new_holders) DBUG_RETURN(true); + + /* + Allocate each Type_holder's args[] buffer on stmt_arena so that + we have stable storage that can be refilled in place on every + EXECUTE instead of being reallocated each time. + */ + Query_arena *arena, backup; + arena= thd->activate_stmt_arena_if_needed(&backup); + bool args_err= false; + for (uint pos= 0; pos < cnt && !args_err; pos++) + { + if (new_holders[pos].alloc_arguments(thd, lists_of_values.elements)) + args_err= true; + } + if (arena) + thd->restore_active_arena(arena, &backup); + if (args_err) + DBUG_RETURN(true); + + holders= type_holders= new_holders; + } + + /* + Collect the column types from the list of values. This must run + on every PREPARE, not just the first one, so that values whose + type is not known at PREPARE time update their actual type at + EXECUTE time. + */ + if (join_type_handlers_for_tvc(thd, li, holders, cnt) || + get_type_attributes_for_tvc(thd, li, holders, + lists_of_values.elements, cnt)) + DBUG_RETURN(true); + + /* + If the counts don't match, then allocate some more + Item_type_holder instances. These will, on subsequent EXECUTEs, + be mutated in place (see the 'else' case following). + */ + if (sl->item_list.elements != cnt) + { List_iterator_fast it(*first_elem); Item *item; Query_arena *arena, backup; @@ -301,6 +356,22 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl, if (unlikely(thd->is_fatal_error)) DBUG_RETURN(true); // out of memory } + else + { + /* + Refresh the cached type info on the existing Item_type_holder + instances so that the types reported for the TVC reflect the values + seen on the current execution. + */ + List_iterator it(sl->item_list); + for (uint pos= 0; pos < cnt; pos++) + { + Item_type_holder *ith= static_cast(it++); + ith->set_handler(holders[pos].type_handler()); + ith->Type_std_attributes::set(holders[pos]); + ith->set_maybe_null(holders[pos].get_maybe_null()); + } + } result= tmp_result;