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

TABLE_TYPE for temporary sequences is the same as for permanent ones

Details

    Description

      MDEV-12459 introduced table_type TEMPORARY for temporary tables. However, for temporary sequences I_S.TABLES shows the generic SEQUENCE, same as for normal sequences.
      They are still distinguishable via TEMPORARY column in I_S.TABLES, but it appears inconsistent, and besides there is a discussion in MDEV-12459 about removing the column.

      create sequence s;
      create temporary sequence s;
      select table_name, table_type, temporary from information_schema.tables where table_name = 's';
       
      # Cleanup
      drop sequence s;
      drop sequence s;
      

      preview-10.9-MDEV-20119-misc c906db30

      select table_name, table_type, temporary from information_schema.tables where table_name = 's';
      table_name	table_type	temporary
      s	SEQUENCE	Y
      s	SEQUENCE	N
      

      Attachments

        Issue Links

          Activity

            I have updated MDEV-12459 and I don't think this is a bug, two sequences are distinguished by the temporary column.

            anel Anel Husakovic added a comment - I have updated MDEV-12459 and I don't think this is a bug, two sequences are distinguished by the temporary column.
            elenst Elena Stepanova added a comment - - edited

            It is still a bit of a problem (although a small one, considering the low probability of using temporary sequences in real life and even lower probability of overshadowing).

            Suppose we have

            create table t (a int);
            create temporary sequence t;
            

            The old version of sys.table_exists, even though it doesn't understand sequences at all (see MDEV-28340), would still return in this case a close-to-valid TEMPORARY.

            However, the new version fails with

            preview-10.9-MDEV-20119-misc c906db3033

            mysqltest: At line 3: query 'call sys.table_exists('test','t',@a)' failed: ER_SUBQUERY_NO_1_ROW (1242): Subquery returns more than 1 row
            

            – understandably, since the check for TABLE_TYPE = 'temporary' returns nothing, and the next SELECT returns both rows.

            elenst Elena Stepanova added a comment - - edited It is still a bit of a problem (although a small one, considering the low probability of using temporary sequences in real life and even lower probability of overshadowing). Suppose we have create table t (a int ); create temporary sequence t; The old version of sys.table_exists , even though it doesn't understand sequences at all (see MDEV-28340 ), would still return in this case a close-to-valid TEMPORARY . However, the new version fails with preview-10.9-MDEV-20119-misc c906db3033 mysqltest: At line 3: query 'call sys.table_exists(' test ',' t ',@a)' failed: ER_SUBQUERY_NO_1_ROW (1242): Subquery returns more than 1 row – understandably, since the check for TABLE_TYPE = 'temporary' returns nothing, and the next SELECT returns both rows.

            I don't know the sys schema code and I don't know how did you get result from above (I understand that is from the latest patch and why did it fail but don't know how sys schema should be tested)?
            Looking on the number of MDEV's related to sys schema and my patch seems I need to learn it .
            Note our sys schema documentation is currently incomplete.

            anel Anel Husakovic added a comment - I don't know the sys schema code and I don't know how did you get result from above (I understand that is from the latest patch and why did it fail but don't know how sys schema should be tested)? Looking on the number of MDEV's related to sys schema and my patch seems I need to learn it . Note our sys schema documentation is currently incomplete.
            anel Anel Husakovic added a comment - V can you please review https://github.com/MariaDB/server/commit/676b16eedc4dd42815969ee5f74d6e54f8a276d0

            anel The commit you linked only seems to contain the test case, not the actual code changes and I can't figure out from the log which other commit I'm supposed to look at as it seems Monty reviewed the other ones. Can you please create a commit that covers the changes required by this MDEV and then send it for review?

            The test case does show that having 2 tables (or rather sequences), one shadowing the other one makes sys.table_exists return the type of the one that's visible. That's ok (I think). We need to document this behavior properly.

            https://mariadb.com/kb/en/sys-schema/ has no info at all regarding this intended behaviour. https://dev.mysql.com/doc/refman/8.0/en/sys-table-exists.html does, but MySQL does not have any temporary sequences.

            Additionally, from the test case the output doesn't match what I would expect the output of sys.table_exists to be. Like elenst mentioned, when executing the stored procedure on a temporary sequence one gets a close-to-valid TEMPORARY. I would extend the behavior for it to properly say TEMPORARY SEQUENCE when we do pass it a temporary sequence, much like you extended it to say SEQUENCE.

            greenman remember this MDEV when documenting sys_schema.

            cvicentiu Vicențiu Ciorbaru added a comment - anel The commit you linked only seems to contain the test case, not the actual code changes and I can't figure out from the log which other commit I'm supposed to look at as it seems Monty reviewed the other ones. Can you please create a commit that covers the changes required by this MDEV and then send it for review? The test case does show that having 2 tables (or rather sequences), one shadowing the other one makes sys.table_exists return the type of the one that's visible. That's ok (I think). We need to document this behavior properly. https://mariadb.com/kb/en/sys-schema/ has no info at all regarding this intended behaviour. https://dev.mysql.com/doc/refman/8.0/en/sys-table-exists.html does, but MySQL does not have any temporary sequences. Additionally, from the test case the output doesn't match what I would expect the output of sys.table_exists to be. Like elenst mentioned, when executing the stored procedure on a temporary sequence one gets a close-to-valid TEMPORARY . I would extend the behavior for it to properly say TEMPORARY SEQUENCE when we do pass it a temporary sequence, much like you extended it to say SEQUENCE . greenman remember this MDEV when documenting sys_schema.
            anel Anel Husakovic added a comment - PR 2189 created MDEV-28335

            This MDEV covers the last review comment from MDEV-12459.

            We will introduce TEMPORARY SEQUENCE as a string in I_S.tables.

            create table t1 (a int);
            create sequence s1;
            create temporary table t1 (b int);
            create temporary sequence s1;
            select table_schema, table_name, table_type, temporary
            from information_schema.tables
            where table_schema = 'test';
            table_schema	table_name	table_type	temporary
            test	s1	TEMPORARY SEQUENCE	Y
            test	t1	TEMPORARY	Y
            test	t1	BASE TABLE	N
            test	s1	SEQUENCE	N
            

            Anel, please close this when that patch is pushed.

            cvicentiu Vicențiu Ciorbaru added a comment - This MDEV covers the last review comment from MDEV-12459 . We will introduce TEMPORARY SEQUENCE as a string in I_S.tables. create table t1 (a int); create sequence s1; create temporary table t1 (b int); create temporary sequence s1; select table_schema, table_name, table_type, temporary from information_schema.tables where table_schema = 'test'; table_schema table_name table_type temporary test s1 TEMPORARY SEQUENCE Y test t1 TEMPORARY Y test t1 BASE TABLE N test s1 SEQUENCE N Anel, please close this when that patch is pushed.

            People

              anel Anel Husakovic
              elenst Elena Stepanova
              Votes:
              0 Vote for this issue
              Watchers:
              5 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.