maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

On Jul 21, Alexander Barkov wrote:
> 
> I have one question about "HANDLER FOR SQLWARNING".
> It does not catch messages of level "Note".
> Looks like a bug. Please confirm, I'll file a report if so.

Hmm, I don't know.

According to the standard, SQLWARNING should match sqlstates in the
category W, while SQLEXCEPTION should be for sqlstates in the category X.

But we don't do that, 22007 is an exception (category X), but we treat
it as a warning. So, the standard doesn't help to answer your question.

We seem to use the logic where a level "error" means an SQL exception
and a level "warning" means an SQL warning. Using this logic, "notes"
are neither exceptions nor warnings, and neither SQLEXCEPTION nor
SQLWARNING should match them.

I'd say it's not a bug.

The patch itself looks great, these changes in behavior are very welcomed.
Just one small comment regarding THD::check_edom_and_truncation(), see
below.

> diff --git a/sql/sql_class.h b/sql/sql_class.h
> index a8d8444..c19174b 100644
> --- a/sql/sql_class.h
> +++ b/sql/sql_class.h
> @@ -3270,6 +3270,53 @@ class THD :public Statement,
>              (!transaction.stmt.modified_non_trans_table ||
>               (variables.sql_mode & MODE_STRICT_ALL_TABLES)));
>    }
> +  /**
> +    Check string-to-number conversion and produce a warning if
> +    - could not convert any digits (EDOM-alike error)
> +    - found garbage at the end of the string
> +    See also Field_num::check_edom_and_truncation() for a similar function.
> +
> +    @param thd        - the thread
> +    @param type       - name of the data type (e.g. "INTEGER", "DECIMAL", "DOUBLE")
> +    @param edom       - of a EDOM-alike error happened
> +    @param cs         - character set of the original string
> +    @param str        - the original string
> +    @param end_of_num - the position where the string-to-number routine stopped.
> +    @param end        - the end of the string
> +
> +    Unlike Field_num::check_edom_and_truncation(), this function does not
> +    distinguish between EDOM and truncation and reports the same warning for
> +    both cases. Perhaps we should eventually print different warnings, to make
> +    the explicit CAST work closer to the implicit cast in Field_xxx::store().

Why haven't you done it now? It causes too many changes in tests?

> +  */
> +  void check_edom_and_truncation(const char *type, bool edom,
> +                                 CHARSET_INFO *cs,
> +                                 const char *str,
> +                                 const char *end_of_num,
> +                                 const char *end)

Move this out of THD please. You don't need to call current_thd
every time for it, but only when a warning should be issued.

> +  {
> +    DBUG_ASSERT(str <= end_of_num);
> +    DBUG_ASSERT(end_of_num <= end);
> +    if (edom ||
> +        (end_of_num < end && !check_if_only_end_space(cs, end_of_num, end)))
> +    {
> +      /*
> +        We can use err.ptr() here as ErrConvString is guranteed to put an
> +        end \0 here.
> +      */
> +      push_warning_printf(this, Sql_condition::WARN_LEVEL_WARN,
> +                          ER_TRUNCATED_WRONG_VALUE,
> +                          ER(ER_TRUNCATED_WRONG_VALUE), type,
> +                          ErrConvString(str, end - str, cs).ptr());
> +    }
> +    else if (end_of_num < end)
> +    {
> +      push_warning_printf(this, Sql_condition::WARN_LEVEL_NOTE,
> +                          ER_TRUNCATED_WRONG_VALUE,
> +                          ER(ER_TRUNCATED_WRONG_VALUE), type,
> +                          ErrConvString(str, end - str, cs).ptr());
> +    }
> +  }

Regards,
Sergei