← Back to team overview

maria-developers team mailing list archive

Re: Please review MDEV-8468 MDEV-8466


Hi Sergei,

On 09/13/2015 05:17 PM, Sergei Golubchik wrote:
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.

All right. We can think about it later.
Perhaps, for symmetry, we should add HANDLER FOR SQLNOTE.
I've added a new task, with a quote from this our discussion.

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

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?

Yeah, I was afraid that. I'd like to do this, but in a separate change.
I just created an MDEV for this:

MDEV-8810 Warnings for EDOM vs truncation in val_int, val_real, val_decimal

+  */
+  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.

Right, thanks for the idea. Moved. Now current_thd is called only
if a warning/note is really needed, and there's no a "thd" available
in the call context.

Btw, while moving the code, I realized that it's easy to reuse
the same code in:


and I did it. Now they are all just as small as two statements.


+  {
+    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());
+    }
+  }