maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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
> Author: Monty <monty@xxxxxxxxxxx>
> Date:   Fri Aug 5 17:57:27 2022 +0300
>
>      MDEV-12325 Unexpected data type and truncation when using CTE
>
>      When creating a recursive CTE, the column types are taken from the
>      non recursive part of the CTE (this is according to the SQL standard).
>
>      This patch adds code to abort the CTE if the calculated values in the
>      recursive part does not fit in the fields in the created temporary table.
>
>      The new code only affects recursive CTE, so it should not cause any notable
>      problems for old applications.
<cut>
> diff --git a/sql/sql_union.cc b/sql/sql_union.cc
> index 48d8c16db68..d271a919a4e 100644
> --- a/sql/sql_union.cc
> +++ b/sql/sql_union.cc
> @@ -111,6 +111,8 @@ int select_unit::send_data(List<Item> &values)
>   {
>     int rc;
>     int not_reported_error= 0;
> +  enum_check_fields save_count_cuted_fields= thd->count_cuted_fields;
> +
>     if (unit->offset_limit_cnt)
>     {                        // using limit offset,count
>       unit->offset_limit_cnt--;
> @@ -118,8 +120,10 @@ int select_unit::send_data(List<Item> &values)
>     }
>     if (thd->killed == ABORT_QUERY)
>       return 0;
> +
>     if (table->no_rows_with_nulls)
>       table->null_catch_flags= CHECK_ROW_FOR_NULLS_TO_REJECT;
> +
>     if (intersect_mark)
>     {
>       fill_record(thd, table, table->field + 1, values, TRUE, FALSE);
> @@ -127,6 +131,8 @@ int select_unit::send_data(List<Item> &values)
>     }
>     else
>       fill_record(thd, table, table->field, values, TRUE, FALSE);
> +  thd->count_cuted_fields= save_count_cuted_fields;
> +
>     if (unlikely(thd->is_error()))
>     {
>       rc= 1;
> @@ -297,7 +303,13 @@ bool select_unit::send_eof()


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>