Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-32999

ERROR 138 instead of warning, and potential error masking

Details

    Description

      INSTALL PLUGIN Spider SONAME 'ha_spider.so';
      CREATE TABLE t ENGINE=Spider COMMENT='WRAPPER "mysql"';
      

      Leads to, on base:

      11.3.0 126157061b4376496c034a809ea4943e863d1465 (Debug)

      11.3.0-dbg> INSTALL PLUGIN Spider SONAME 'ha_spider.so';
      Query OK, 0 rows affected, 1 warning (0.285 sec)
       
      11.3.0-dbg> CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",TABLE "t1"';
      ERROR 1429 (HY000): Unable to connect to foreign data source: localhost
      

      Versus the patch:

      11.4.0 f93c20081a8a505ac502850ec02630f95673dfba (Optimized)

      11.4.0-opt> INSTALL PLUGIN Spider SONAME 'ha_spider.so';
      Query OK, 0 rows affected, 1 warning (0.284 sec)
       
      11.4.0-opt> CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",TABLE "t1"';
      ERROR 138 (0A000): Spider table params in COMMENT or CONNECTION strings have been deprecated and will be removed in a future release. Please use table options instead.
      

      Two issues:
      1) It seems that other errors are masked in favor of ERROR 138
      2) The ERROR should have been a warning only

      Attachments

        Issue Links

          Activity

            ycp Yuchen Pei added a comment - - edited

            This issue has nothing to do with spider, as spider is simply using
            the warning API provided by the server layer.

            What happens is the value of THD::abort_on_warning during table
            creation at the server layer. When it is true, warnings are converted
            to errors, and if the first warning happens before the first error,
            then the warning will "mask" errors.

            When one executes a CREATE TABLE without table structure, i.e. without
            defining columns of the table like in the case in the previous
            comment, this causes create_table_mode to be
            C_ASSISTED_DISCOVERY (-3), which causes thd->abort_on_warning
            to be true. Thus the warning becomes an error and no further error is
            displayed.

            When one executes a (more common) CREATE TABLE with table
            structure e.g. CREATE TABLE ts1 (c int)..., the
            create_table_mode is C_ORDINARY_CREATE (0), which causes
            mysql_create_frm_image() to be called and sets
            thd->abort_on_warning to false, and errors (if any) are printed
            instead of warnings.

            See the following example in
            storage/spider/mysql-test/spider/feature/r/engine_defined_attributes.result:

            # MDEV-27860 SIGSEGV in spider_parse_connect_info on CREATE TABLE
            CREATE TABLE tbl_a (c INT) ENGINE=SPIDER COMMENT="TABLE 'unknown_table'"
            PARTITION BY LIST COLUMNS (c) (
            PARTITION p DEFAULT COMMENT="srv 'unknown_server'" ENGINE=SPIDER
            );
            ERROR HY000: The foreign server name you are trying to reference does not exist. Data source error:  unknown_server

            So this is either intended behaviour or a server layer bug. I am
            leaning towards the former.

            ycp Yuchen Pei added a comment - - edited This issue has nothing to do with spider, as spider is simply using the warning API provided by the server layer. What happens is the value of THD::abort_on_warning during table creation at the server layer. When it is true, warnings are converted to errors, and if the first warning happens before the first error, then the warning will "mask" errors. When one executes a CREATE TABLE without table structure, i.e. without defining columns of the table like in the case in the previous comment, this causes create_table_mode to be C_ASSISTED_DISCOVERY (-3), which causes thd->abort_on_warning to be true. Thus the warning becomes an error and no further error is displayed. When one executes a (more common) CREATE TABLE with table structure e.g. CREATE TABLE ts1 (c int)... , the create_table_mode is C_ORDINARY_CREATE (0), which causes mysql_create_frm_image() to be called and sets thd->abort_on_warning to false, and errors (if any) are printed instead of warnings. See the following example in storage/spider/mysql-test/spider/feature/r/engine_defined_attributes.result: # MDEV-27860 SIGSEGV in spider_parse_connect_info on CREATE TABLE CREATE TABLE tbl_a (c INT ) ENGINE=SPIDER COMMENT= "TABLE 'unknown_table'" PARTITION BY LIST COLUMNS (c) ( PARTITION p DEFAULT COMMENT= "srv 'unknown_server'" ENGINE=SPIDER ); ERROR HY000: The foreign server name you are trying to reference does not exist. Data source error: unknown_server So this is either intended behaviour or a server layer bug. I am leaning towards the former.
            Roel Roel Van de Paar added a comment - - edited

            IMHO this is a regression. Here are my reasons:
            1. "COMMENT or CONNECTION strings have been deprecated" is a warning, and should ever only be warning-level message only.
            It is a new warning introduced in MDEV-28861, which is turned into an error due to other existing server behavior.
            2. Importantly, the error reported is not the correct reason for the failure. IOW, the actual reason is not the use of COMMENT but being unable to connect to the remote host. This may be confusing for users. Another example of a similar situation in #3 below.
            3. It looks like most Spider-related error resolution is lost. We have the example above (ERROR 1429 masked), and another example (ERROR 12702):

            INSTALL PLUGIN Spider SONAME 'ha_spider.so';
            CREATE USER spider@localhost IDENTIFIED BY 'pwd';
            GRANT ALL ON test.* TO spider@localhost;
            CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "../socket.sock",DATABASE 'test',USER 'spider',PASSWORD 'pwd');
            CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",SERVER "srv",TABLE "t1"';
            

            At the command line will produce:

            11.4.0 f93c20081a8a505ac502850ec02630f95673dfba (Optimized)

            11.4.0-opt>CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",SERVER "srv",TABLE "t1"';
            ERROR 138 (0A000): Spider table params in COMMENT or CONNECTION strings have been deprecated and will be removed in a future release. Please use table options instead.
            

            Versus

            11.3.0 126157061b4376496c034a809ea4943e863d1465 (Optimized)

            11.3.0-opt>CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",SERVER "srv",TABLE "t1"';
            ERROR 12702 (HY000): Remote table 'test.t1' is not found
            

            Is this case too the failure reason is not the ERROR 138 COMMENT/CONNECTION depreciation, and ERROR 12702 is the correct reason for failure.
            4. Users are used to the old syntax. Handling multiple levels of errors (one of which is not necessary at this point in time) can be confusing and off-putting.

            Roel Roel Van de Paar added a comment - - edited IMHO this is a regression. Here are my reasons: 1. "COMMENT or CONNECTION strings have been deprecated" is a warning, and should ever only be warning-level message only. It is a new warning introduced in MDEV-28861 , which is turned into an error due to other existing server behavior. 2. Importantly, the error reported is not the correct reason for the failure. IOW, the actual reason is not the use of COMMENT but being unable to connect to the remote host. This may be confusing for users. Another example of a similar situation in #3 below. 3. It looks like most Spider-related error resolution is lost. We have the example above (ERROR 1429 masked), and another example (ERROR 12702): INSTALL PLUGIN Spider SONAME 'ha_spider.so' ; CREATE USER spider@localhost IDENTIFIED BY 'pwd' ; GRANT ALL ON test.* TO spider@localhost; CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "../socket.sock" , DATABASE 'test' , USER 'spider' , PASSWORD 'pwd' ); CREATE TABLE ts1 ENGINE=Spider COMMENT= 'WRAPPER "mysql",SERVER "srv",TABLE "t1"' ; At the command line will produce: 11.4.0 f93c20081a8a505ac502850ec02630f95673dfba (Optimized) 11.4.0-opt>CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",SERVER "srv",TABLE "t1"'; ERROR 138 (0A000): Spider table params in COMMENT or CONNECTION strings have been deprecated and will be removed in a future release. Please use table options instead. Versus 11.3.0 126157061b4376496c034a809ea4943e863d1465 (Optimized) 11.3.0-opt>CREATE TABLE ts1 ENGINE=Spider COMMENT='WRAPPER "mysql",SERVER "srv",TABLE "t1"'; ERROR 12702 (HY000): Remote table 'test.t1' is not found Is this case too the failure reason is not the ERROR 138 COMMENT/CONNECTION depreciation, and ERROR 12702 is the correct reason for failure. 4. Users are used to the old syntax. Handling multiple levels of errors (one of which is not necessary at this point in time) can be confusing and off-putting.
            ycp Yuchen Pei added a comment -

            Hi holyfoot, ptal thanks

            upstream/bb-11.4-mdev-32999 5a5f92bf44bf6963ea9e71630eaa64b28ed9f941
            MDEV-32999 Spider: only warn of connection string deprecation when creating a table
            

            This commit will be squashed into that of MDEV-28861 to push. See below for the squashed commit.

            upstream/bb-11.4-mdev-28861 73e4e4feb085547108c8f682d7cc864b6734a4e7
            MDEV-28861 Deprecate spider table options by comment/connection
             
            Also deprecating table params not implemented in MDEV-28856.
            

            ycp Yuchen Pei added a comment - Hi holyfoot , ptal thanks upstream/bb-11.4-mdev-32999 5a5f92bf44bf6963ea9e71630eaa64b28ed9f941 MDEV-32999 Spider: only warn of connection string deprecation when creating a table This commit will be squashed into that of MDEV-28861 to push. See below for the squashed commit. upstream/bb-11.4-mdev-28861 73e4e4feb085547108c8f682d7cc864b6734a4e7 MDEV-28861 Deprecate spider table options by comment/connection   Also deprecating table params not implemented in MDEV-28856.

            ok to push.

            holyfoot Alexey Botchkov added a comment - ok to push.
            ycp Yuchen Pei added a comment -

            Thanks for the review.

            Roel: no problem. Can you test the squashed MDEV-28861 patch that contains the patch in this ticket? It is 73e4e4feb085547108c8f682d7cc864b6734a4e7 referred to in my previous comment.

            ycp Yuchen Pei added a comment - Thanks for the review. Roel : no problem. Can you test the squashed MDEV-28861 patch that contains the patch in this ticket? It is 73e4e4feb085547108c8f682d7cc864b6734a4e7 referred to in my previous comment.

            Confirmed resolved nicely:
            1. No more ERROR for COMMENT deprec.
            2. Deprec. WARNING when COMMENT is used.
            Thank you again ycp.

            Roel Roel Van de Paar added a comment - Confirmed resolved nicely: 1. No more ERROR for COMMENT deprec. 2. Deprec. WARNING when COMMENT is used. Thank you again ycp .
            ycp Yuchen Pei added a comment -

            Thanks Roel, pushed 20741b92370ee48843d6101b6d6eaecab58ba41a to 11.4

            ycp Yuchen Pei added a comment - Thanks Roel , pushed 20741b92370ee48843d6101b6d6eaecab58ba41a to 11.4

            People

              ycp Yuchen Pei
              Roel Roel Van de Paar
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.