Hello Monty, On 8/5/22 10:35 PM, Alexander Barkov wrote:
Hello Monty.
The patch looks OK for me. I have just one question below:
On 8/5/22 7:15 PM, Michael Widenius wrote:
Hi!
Just to make things easy, here is the patch: commit a556c4d972eaa09bb8ad586b9bda6cfb2a7b574a <cut>
Why do you need above changes in select_unit::send_data()?
Shouldn't it be enough just to have below changes in select_union_recursive::send_data()?
int select_union_recursive::send_data(List<Item> &values) { + Abort_on_warning_instant_set handle_abort(thd, 1); + enum_check_fields save_count_cuted_fields= thd->count_cuted_fields; + + /* For recursive CTE's give warnings for wrong field info */ + thd->count_cuted_fields= CHECK_FIELD_WARN; int rc= select_unit::send_data(values); + thd->count_cuted_fields= save_count_cuted_fields;
if (rc == 0 && write_err != HA_ERR_FOUND_DUPP_KEY && <cut>
I compiled your patch. The changes to select_unit::send_data() are not needed. We only need to fix select_union_recursive::send_data(). Also, in non-strict mode it should produce warnings on tructation, without aborting SELECT or CREATE..SELECT statements. After some experimenting I came to this patch:
diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 48d8c16db68..60aba118f04 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -297,7 +297,13 @@ bool select_unit::send_eof()
int select_union_recursive::send_data(List<Item> &values) { + Abort_on_warning_instant_set handle_abort(thd, thd->is_strict_mode()); + enum_check_fields save_count_cuted_fields= thd->count_cuted_fields; + + /* For recursive CTE's give warnings for wrong field info */ + thd->count_cuted_fields= CHECK_FIELD_WARN; int rc= select_unit::send_data(values); + thd->count_cuted_fields= save_count_cuted_fields;
if (rc == 0 && write_err != HA_ERR_FOUND_DUPP_KEY &&
In strict mode it still works fine and cte_recursive.test passes. In non-strict modes it: - now continues the statement (in your original version it aborted the statement on error). - SELECT and CREATE..SELECT produce the same result set, with the same set of warnings. - Issues truncation warnings Please consider applying this version of the patch. Note, a test for non-strict mode is additionally needed. Also, I think the patch should be further extended to handle IGNORE properly: CREATE TABLE t2 IGNORE AS WITH RECURSIVE .. SELECT ..; "Strict mode + IGNORE" should work like non-strict mode. A new test is also needed here. In order to process IGNORE easy, a "bool ignore;" member can be added to the class select_union_recursive and initialized in constructor: - either from a new constructor parameter - or using thd_arg->lex->ignore. Please investigate what is more appropriate. See classes st_copy_info, multi_update, Alter_inplace_info as examples having "ignore" as a member. Greetings.