Details

    • Type: Bug
    • Status: In Review
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 10.0.17
    • Fix Version/s: 10.1
    • Component/s: Dynamic Columns
    • Labels:
    • Environment:
      Linux
    • Sprint:
      10.1.8-3, 10.1.8-4

      Description

      SET @aaa = COLUMN_CREATE('price', '');
      SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) aaa;
      
      aaa     
      --------
      0       
      
      SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL)  + 1 aaa
      
      aaa     
      --------
      1       
      
      DELIMITER $$
      
      CREATE FUNCTION `test`(
          par_aaa DECIMAL(10,2)
         )
          RETURNS DECIMAL(10,2)
          DETERMINISTIC    
          BEGIN
      	RETURN 0;
          END$$
      
      DELIMITER ;
      
      SELECT test(COLUMN_GET(@aaa, 'price' AS DECIMAL)) bbb
      
      Error CODE: 1918
      Encountered illegal VALUE '' WHEN converting TO DECIMAL
      
      SELECT test('') bbb
      
      Error CODE: 1366
      Incorrect DECIMAL VALUE: '' FOR COLUMN 'par_aaa' AT ROW 441
      
      SELECT test(CAST('' AS DECIMAL(10,2))) bbb
      
      Error CODE: 1292
      Truncated incorrect DECIMAL VALUE: ''
      
      SELECT test(''+0) bbb
      
      bbb     
      --------
      0.00   
      

      More expected that result '' + 0 and CAST('' AS DECIMAL(10,2)) would be same.
      But we saw error when do CAST, and automatic convertion '' as 0 when try select or make math operation.

      SELECT @@sql_mode
      
      NO_BACKSLASH_ESCAPES,STRICT_ALL_TABLES,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
      

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              mikhail Mikhail Gavrilov added a comment -

              I am prefer that COLUMN_GET(@aaa, 'price' AS DECIMAL) considered all illegal decimal as 0

              Show
              mikhail Mikhail Gavrilov added a comment - I am prefer that COLUMN_GET(@aaa, 'price' AS DECIMAL) considered all illegal decimal as 0
              Hide
              elenst Elena Stepanova added a comment -

              Hi Mikhail,

              Could you please clarify what is the "major complaint" here?
              There are several expressions in your bug description.

              I understand you have a concern about '' + 0 vs CAST('' AS DECIMAL(10,2)) (the latter produces a warning/error, and the former does not); but it doesn't explain the presence of dynamic column operations in the description.

              At the same time if the major problem is what you mentioned in the comment, then it's not clear how it relates to all other statements.

              If you have several concerns that you wanted to group in this report, could you please state clearly which results you consider wrong, and what you expect instead?

              Thanks.

              Show
              elenst Elena Stepanova added a comment - Hi Mikhail, Could you please clarify what is the "major complaint" here? There are several expressions in your bug description. I understand you have a concern about '' + 0 vs CAST('' AS DECIMAL(10,2)) (the latter produces a warning/error, and the former does not); but it doesn't explain the presence of dynamic column operations in the description. At the same time if the major problem is what you mentioned in the comment, then it's not clear how it relates to all other statements. If you have several concerns that you wanted to group in this report, could you please state clearly which results you consider wrong, and what you expect instead? Thanks.
              Hide
              mikhail Mikhail Gavrilov added a comment -

              I'm very disappointed because I can retrieve data in the right type from dynamic column to the application server.

              SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) aaa
              

              But I can't extract the data in the correct type for user defined SQL functions.

              SELECT test(COLUMN_GET(@aaa, 'price' AS DECIMAL)) bbb
              
              Show
              mikhail Mikhail Gavrilov added a comment - I'm very disappointed because I can retrieve data in the right type from dynamic column to the application server. SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) aaa But I can't extract the data in the correct type for user defined SQL functions. SELECT test(COLUMN_GET(@aaa, 'price' AS DECIMAL)) bbb
              Hide
              elenst Elena Stepanova added a comment -

              The first one also doesn't go totally smoothly, it actually produces a warning:

              +---------+------+---------------------------------------------------------+
              | Level   | Code | Message                                                 |
              +---------+------+---------------------------------------------------------+
              | Warning | 1918 | Encountered illegal value '' when converting to DECIMAL |
              +---------+------+---------------------------------------------------------+
              

              But I agree that the behavior is inconsistent.

              Alexander Barkov,
              Could you please take a look – not from the side of dynamic columns, but from the general perspective, as the whole thing is rather confusing – and decide which behavior in which situation is expected? After that, if the problem is specific to dynamic columns, we can reassign it.

              Show
              elenst Elena Stepanova added a comment - The first one also doesn't go totally smoothly, it actually produces a warning: +---------+------+---------------------------------------------------------+ | Level | Code | Message | +---------+------+---------------------------------------------------------+ | Warning | 1918 | Encountered illegal value '' when converting to DECIMAL | +---------+------+---------------------------------------------------------+ But I agree that the behavior is inconsistent. Alexander Barkov , Could you please take a look – not from the side of dynamic columns, but from the general perspective, as the whole thing is rather confusing – and decide which behavior in which situation is expected? After that, if the problem is specific to dynamic columns, we can reassign it.
              Hide
              bar Alexander Barkov added a comment - - edited

              The problem happens because stored procedure (SP) variables, including parameters, use the Field class under the hood to store values and thus copy behavior from table fields.

              So this example query fails:

              SELECT f1(COLUMN_GET(@aaa, 'price' AS DECIMAL)) bbb;
              

              because there is a Field_new_decimal that stands behind the 'par_aaa' parameter of the function.
              When a value is stored into the Field_new_decimal instance that handles the SP variable par_aaa, warnings are translated into errors, similar to what happens in case of a normal table DECIMAL field.

              Field::store() should check field->table->s->db_plugin to distinguish between a real table field and a SP variable, to decide whether escalate a warning to an error in case of strict mode.

              However, there are some open questions:

              1. Some tests in mysql-test/t/* (e.g. sp-vars.test) rely on this behavior of SP variables.
              There is a risk that it could break existing user applications. It seems like a new sql_mode option STRICT_SP_VARIABLES is needed to avoid upgrade problems.

              2. Currently some Field_xxx:::val_xxx() can escalate warnings to errors, even if the value of the field is not going into some other field. This issue should be studying carefully.

              3. It's not clear what to do when an SP variable is assigned to a field and the other way around:

              UPDATE t1 SET field=spvar;
              SELECT field INTO spvar FROM t1;
              

              Which of them should translate warnings to errors? Perhaps only the former should.

              4. We believe that MDEV-8300 should be fixed in the same version with this issue, for consistent behavior and flexibility.

              Summary: this change would be too huge and dangerous for 10.0.
              Changing version to 10.1. But there are some chances that it will be moved to even 10.2.

              Show
              bar Alexander Barkov added a comment - - edited The problem happens because stored procedure (SP) variables, including parameters, use the Field class under the hood to store values and thus copy behavior from table fields. So this example query fails: SELECT f1(COLUMN_GET(@aaa, 'price' AS DECIMAL)) bbb; because there is a Field_new_decimal that stands behind the 'par_aaa' parameter of the function. When a value is stored into the Field_new_decimal instance that handles the SP variable par_aaa, warnings are translated into errors, similar to what happens in case of a normal table DECIMAL field. Field::store() should check field->table->s->db_plugin to distinguish between a real table field and a SP variable, to decide whether escalate a warning to an error in case of strict mode. However, there are some open questions: 1. Some tests in mysql-test/t/* (e.g. sp-vars.test) rely on this behavior of SP variables. There is a risk that it could break existing user applications. It seems like a new sql_mode option STRICT_SP_VARIABLES is needed to avoid upgrade problems. 2. Currently some Field_xxx:::val_xxx() can escalate warnings to errors, even if the value of the field is not going into some other field. This issue should be studying carefully. 3. It's not clear what to do when an SP variable is assigned to a field and the other way around: UPDATE t1 SET field=spvar; SELECT field INTO spvar FROM t1; Which of them should translate warnings to errors? Perhaps only the former should. 4. We believe that MDEV-8300 should be fixed in the same version with this issue, for consistent behavior and flexibility. Summary: this change would be too huge and dangerous for 10.0. Changing version to 10.1. But there are some chances that it will be moved to even 10.2.
              Hide
              bar Alexander Barkov added a comment -

              Bar proposed that for the explicit column conversion in this script:

              SET @aaa = COLUMN_CREATE('price', '');
              INSERT INTO t1 VALUES(COLUMN_GET(@aaa, 'price' AS DECIMAL));
              ERROR 1918 (22007): Encountered illegal value '' when converting to DECIMAL
              

              the correct behavior would be:

              • COLUMN_GET() to return 0 with a warning
              • INSERT to write 0 into the table normally, as the value being inserted is actually the COLUMN_GET result, which is 0.

              Sergei replied:

              That wouldv'e been ok if there was some "native" type of COLUMN_GET(@aaa, 'price'). Like, you can insert the value in its "native" type (relying on implicit type conversion):

              INSERT INTO t1 VALUES ('');
              

              or cast it explicitly:

              INSERT INTO t1 VALUES (CAST('' AS DECIMAL))
              

              But with dynamic columns there is no "native" type, you always must cast it to something. If strict mode won't turn these warnings into errors, there will be no way to do that. Normally you add a cast to insert with warnings and remove a cast to let strict mode turn warnings into errors (here I assume the case 3 from above is already fixed . But with dynamic columns there is no cast to remove.

              So I agree with you here, because logically there is an explicit conversion and strict mode should not turn this warning into an error. Still, if we do that, there will be no way to catch conversion warnings in dynamic columns. Unless you introduce STRICT_DYNAMIC_COLUMNS mode

              Show
              bar Alexander Barkov added a comment - Bar proposed that for the explicit column conversion in this script: SET @aaa = COLUMN_CREATE('price', ''); INSERT INTO t1 VALUES(COLUMN_GET(@aaa, 'price' AS DECIMAL)); ERROR 1918 (22007): Encountered illegal value '' when converting to DECIMAL the correct behavior would be: COLUMN_GET() to return 0 with a warning INSERT to write 0 into the table normally, as the value being inserted is actually the COLUMN_GET result, which is 0. Sergei replied: That wouldv'e been ok if there was some "native" type of COLUMN_GET(@aaa, 'price'). Like, you can insert the value in its "native" type (relying on implicit type conversion): INSERT INTO t1 VALUES (''); or cast it explicitly: INSERT INTO t1 VALUES (CAST('' AS DECIMAL)) But with dynamic columns there is no "native" type, you always must cast it to something. If strict mode won't turn these warnings into errors, there will be no way to do that. Normally you add a cast to insert with warnings and remove a cast to let strict mode turn warnings into errors (here I assume the case 3 from above is already fixed . But with dynamic columns there is no cast to remove. So I agree with you here, because logically there is an explicit conversion and strict mode should not turn this warning into an error. Still, if we do that, there will be no way to catch conversion warnings in dynamic columns. Unless you introduce STRICT_DYNAMIC_COLUMNS mode

                People

                • Assignee:
                  serg Sergei Golubchik
                  Reporter:
                  mikhail Mikhail Gavrilov
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  4 Start watching this issue

                  Dates

                  • Created:
                    Updated:

                    Agile