Details

    • New Feature
    • Status: Closed (View Workflow)
    • Critical
    • Resolution: Fixed
    • 11.5.1
    • Sequences
    • None

    Description

      This is a list of trivial features for Sequence (originally mentioned as part of MDEV-13005)

      • Support syntax "CREATE SEQUENCE x AS <some_int_type>';
        <some_int_type> can mean all the integers that we support
        Currently (as per documentation) maximum value that sequence can support is 9223372036854775806.
        But unsigned bigint can have maximum (unsigned) value: 18446744073709551615
        So adding support for this maximum (unsigned) value is needed.
        Also, set maximum minimum based on the datatype
      • Allow in parser very big numbers (decimal) for MAXVALUE but change these to LONGLONG_MAX-1, with a note for the user. This is needes as it's common practice to use
        MAXVALUE 9999999999999999999999999999 in Oracle examples.
      • Create information_schema.sequences that lists all sequences and their information. Should work similar to views:
        select * from information_schema.sequences where table_schema="test";
        The column names are:
        SEQUENCE_CATALOG, SEQUENCE_SCHEMA, SEQUENCE_NAME, DATA_TYPE, NUMERIC_PRECISION, NUMERIC_PRECISION_RADIX, NUMERIC_SCALE, START_VALUE, MINIMUM_VALUE, MAXIMUM_VALUE, INCREMENT, CYCLE_OPTION, DECLARED_DATA_TYPE, DECLARED_NUMERIC_PRECISION, DECLARED_NUMERIC_SCALE

      Some suggested values:
      DATA_TYPE BIGINT
      DECLARED_DATA_TYPE BIGINT
      NUMERIC_PRECISION 19
      NUMERIC_PRECISION_RADIX 10
      DECLARED_DATA_TYPE NULL
      DECLARED_NUMERIC_PRECISION NULL
      DECLARED_NUMERIC_SCALE NULL

      Attachments

        Issue Links

          Activity

            ycp Yuchen Pei added a comment -

            Initial version of the second item done: https://github.com/MariaDB/server/commit/84675ed91f4, where for consistency we

            1. truncate not only decimals, but also ulonglong, -ulonglong, LONGLONG_MAX and LONGLONG_MIN
            2. truncate not only maxvalue but also minvalue (obviously)

            Now working on the first item.

            ycp Yuchen Pei added a comment - Initial version of the second item done: https://github.com/MariaDB/server/commit/84675ed91f4 , where for consistency we 1. truncate not only decimals, but also ulonglong, -ulonglong, LONGLONG_MAX and LONGLONG_MIN 2. truncate not only maxvalue but also minvalue (obviously) Now working on the first item.
            ycp Yuchen Pei added a comment -

            Initial version of the first item, part 1: https://github.com/MariaDB/server/commit/8a96330b07a. It supports signed datatypes for sequence values (unsigned datatypes will be in a separate patch). A pending task for this patch is to choose a proper representation of the the value datatypes in the sequence table. Currently I am lazily using the number representation of the C++ enum, but need to find out whether there is a builtin sql enum already in the server code, or we have to use something else, like string (Type_handler::name()).

            ycp Yuchen Pei added a comment - Initial version of the first item, part 1: https://github.com/MariaDB/server/commit/8a96330b07a . It supports signed datatypes for sequence values (unsigned datatypes will be in a separate patch). A pending task for this patch is to choose a proper representation of the the value datatypes in the sequence table. Currently I am lazily using the number representation of the C++ enum, but need to find out whether there is a builtin sql enum already in the server code, or we have to use something else, like string (Type_handler::name()).
            ycp Yuchen Pei added a comment - - edited

            I could not find any built-in sql enums corresponding to the c++ enum_field_types, so I updated my patch to use string instead.

            However, the change from tinyint to string fails some tests. AFAIK, there are two types of failure:

            • "ER_SEQUENCE_INVALID_TABLE_STRUCTURE (4086): Sequence 'test.t1' table structure is invalid (value_type)", which is caused by the varchar field defaulting to my_charset_bin, which adds a binary flag to the field.
            • aria table removing the row in the sequence when cycling back (sequence.next testcase):

              CREATE or REPLACE SEQUENCE t1 minvalue 1 maxvalue 2 increment by -1 cache 2 cycle engine=aria;
              select next value for t1;
              select * from t1;
              CREATE or REPLACE SEQUENCE t1 minvalue 1 maxvalue 2 increment by -1 cache 2 cycle engine=aria;
              select next value for t1;
              select * from t1;
                

              The first select * from t1 (with myisam) returns the correct result:

                next_not_cached_value	minimum_value	maximum_value	start_value
                  increment	cache_size	cycle_option	cycle_count	value_type
              0	1	2	2	-1	2	1	0	bigint
                

              But the second one (with aria) returns empty:

                  next_not_cached_value	minimum_value	maximum_value	start_value
                  increment	cache_size	cycle_option	cycle_count	value_type
                

            I have therefore kept this change in a separate commit (https://github.com/MariaDB/server/commit/f9cb7cbb7dc).

            I will continue debugging these problems. Meanwhile, sanja could you take a look at the three patches and let me know what you think (e.g. whether you agree with the general approach)? After that I will continue with adding support for unsigned value types and the information schema tables.

            One concern with adding a column for value types is migration: how do we handle schema changes caused by server upgrade?

            ycp Yuchen Pei added a comment - - edited I could not find any built-in sql enums corresponding to the c++ enum_field_types , so I updated my patch to use string instead. However, the change from tinyint to string fails some tests. AFAIK, there are two types of failure: "ER_SEQUENCE_INVALID_TABLE_STRUCTURE (4086): Sequence 'test.t1' table structure is invalid (value_type)", which is caused by the varchar field defaulting to my_charset_bin , which adds a binary flag to the field. aria table removing the row in the sequence when cycling back (sequence.next testcase): CREATE or REPLACE SEQUENCE t1 minvalue 1 maxvalue 2 increment by -1 cache 2 cycle engine=aria; select next value for t1; select * from t1; CREATE or REPLACE SEQUENCE t1 minvalue 1 maxvalue 2 increment by -1 cache 2 cycle engine=aria; select next value for t1; select * from t1; The first select * from t1 (with myisam) returns the correct result: next_not_cached_value minimum_value maximum_value start_value increment cache_size cycle_option cycle_count value_type 0 1 2 2 -1 2 1 0 bigint But the second one (with aria) returns empty: next_not_cached_value minimum_value maximum_value start_value increment cache_size cycle_option cycle_count value_type I have therefore kept this change in a separate commit ( https://github.com/MariaDB/server/commit/f9cb7cbb7dc ). I will continue debugging these problems. Meanwhile, sanja could you take a look at the three patches and let me know what you think (e.g. whether you agree with the general approach)? After that I will continue with adding support for unsigned value types and the information schema tables. https://github.com/MariaDB/server/commit/84675ed91f4 https://github.com/MariaDB/server/commit/8a96330b07a (the commit message mentions changing the tinyint to something else, which I did in the next commit below, tests pass) https://github.com/MariaDB/server/commit/f9cb7cbb7dc (some tests do not pass as mentioned above, may need to add more tests for the new feature) One concern with adding a column for value types is migration: how do we handle schema changes caused by server upgrade?
            ycp Yuchen Pei added a comment -

            After discussions with sanja, I have revised the commits:

            ycp Yuchen Pei added a comment - After discussions with sanja , I have revised the commits: https://github.com/MariaDB/server/commit/84675ed91f4 (item 2) https://github.com/MariaDB/server/commit/51815a7d9ff (item 1)
            ycp Yuchen Pei added a comment -

            Following up on discussions, the parsing is fixed - new non-terminals have been added and there's no longer newly introduced conflicts. sanja igor.

            Current progress: Truncation of maxvalue/minvalue (item 2) done. Introducing types (item 1) still wip, mainly pending implementing ALTER SEQUENCE ... AS <new_type> (commit fd5a6414f43b2a3 below), but also need to fix a couple tests (see commit msg of 79d4283490cee) and maybe add more tests. information_schema.sequences (item 3) not started.

            Commits in order (will squash when no longer wip):

            ycp Yuchen Pei added a comment - Following up on discussions, the parsing is fixed - new non-terminals have been added and there's no longer newly introduced conflicts. sanja igor . Current progress: Truncation of maxvalue/minvalue (item 2) done. Introducing types (item 1) still wip, mainly pending implementing ALTER SEQUENCE ... AS <new_type> (commit fd5a6414f43b2a3 below), but also need to fix a couple tests (see commit msg of 79d4283490cee) and maybe add more tests. information_schema.sequences (item 3) not started. Commits in order (will squash when no longer wip): https://github.com/MariaDB/server/commit/f69ea459ca66aff8228a1b08c11c3ea78b3c0758 https://github.com/MariaDB/server/commit/86de48ae1f6f03527eba65ebf35293bf906cf5b0 https://github.com/MariaDB/server/commit/79d4283490cee04db6bf34e2b9c3a3bbf605ea96 https://github.com/MariaDB/server/commit/fd5a6414f43b2a33493fc44762a05a0d46e607df
            ycp Yuchen Pei added a comment -

            As an update, Item 3 is implemented, and all default and sql_sequence tests pass except compat/oracle.sequence which probably requires backward compatibility with oracle that does not support the new AS <TYPE> syntax[1]. The remaining non-trivial issue and blocker is the implementation of ALTER SEQUENCE ... AS <NEW_TYPE>.

            Commits in order (the "alter as new_type" wip commit is in a different branch [2] and has not changed much since the last update):

            [1] https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/CREATE-SEQUENCE.html
            [2] https://github.com/MariaDB/server/compare/bb-11.0-mdev-28152-alter-value-type

            ycp Yuchen Pei added a comment - As an update, Item 3 is implemented, and all default and sql_sequence tests pass except compat/oracle.sequence which probably requires backward compatibility with oracle that does not support the new AS <TYPE> syntax [1] . The remaining non-trivial issue and blocker is the implementation of ALTER SEQUENCE ... AS <NEW_TYPE> . Commits in order (the "alter as new_type" wip commit is in a different branch [2] and has not changed much since the last update): https://github.com/MariaDB/server/commit/f69ea459ca66aff8228a1b08c11c3ea78b3c0758 (truncating max/minvalue) https://github.com/MariaDB/server/commit/86de48ae1f6f03527eba65ebf35293bf906cf5b0 (signed types) https://github.com/MariaDB/server/commit/3758c111e89685f0fc87ea01c234d2cffd84977b (unsigned types) https://github.com/MariaDB/server/commit/3594bf4e1c813c1dff6449995b4eae7c244afcad (information_schema) [1] https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/CREATE-SEQUENCE.html [2] https://github.com/MariaDB/server/compare/bb-11.0-mdev-28152-alter-value-type
            ycp Yuchen Pei added a comment -

            As discussed with serg and sanja, we implement an incomplete alter feature so that it is more likely to be done during this development cycle. The missing feature is: in ALTER SEQUENCE, AS <new_type> cannot be combined with any other alter options. That is, we can do alter sequence s as int, or alter sequence s maxvalue 100 minvalue 20, but not alter sequence s as int maxvalue 100 minvalue 20.

            Patches in order:

            Passing all tests / CI is green tick. sanja can you take a look or assign to someone else if you don't have time?

            ycp Yuchen Pei added a comment - As discussed with serg and sanja , we implement an incomplete alter feature so that it is more likely to be done during this development cycle. The missing feature is: in ALTER SEQUENCE , AS <new_type> cannot be combined with any other alter options. That is, we can do alter sequence s as int , or alter sequence s maxvalue 100 minvalue 20 , but not alter sequence s as int maxvalue 100 minvalue 20 . Patches in order: https://github.com/MariaDB/server/commit/b779f00af30858a5c41378c0e72381cd3d5875a4 (truncating maxvalue/minvalue) https://github.com/MariaDB/server/commit/cbeb2bbfe8827d7b9a8790b671fbd94fb3ef92d2 (supporting AS <type>) https://github.com/MariaDB/server/commit/b0caadedace1b2056454c3e17f02270dea0eca3a (information schema table) Passing all tests / CI is green tick. sanja can you take a look or assign to someone else if you don't have time?
            ycp Yuchen Pei added a comment -

            I realised I need to update the implementation of setval() too for unsigned types. Working on it.

            Meanwhile I have addressed all the comments in the first round review at https://lists.launchpad.net/maria-developers/msg13297.html

            New patch:
            https://github.com/MariaDB/server/commit/eef42da6b59

            Diff with the previous (squashed) patch:
            https://github.com/MariaDB/server/commit/8c2738b21da

            ycp Yuchen Pei added a comment - I realised I need to update the implementation of setval() too for unsigned types. Working on it. Meanwhile I have addressed all the comments in the first round review at https://lists.launchpad.net/maria-developers/msg13297.html New patch: https://github.com/MariaDB/server/commit/eef42da6b59 Diff with the previous (squashed) patch: https://github.com/MariaDB/server/commit/8c2738b21da
            ycp Yuchen Pei added a comment - - edited

            sanja In case you haven't had a chance to look at the patches in the previous comment yet, here is the updated patch with setval() and auto_increment() updated for unsigned types:

            https://github.com/MariaDB/server/commit/30425c431c5

            The diff with the previous patch that you have reviewed:

            https://github.com/MariaDB/server/commit/bd4a2fac119

            ycp Yuchen Pei added a comment - - edited sanja In case you haven't had a chance to look at the patches in the previous comment yet, here is the updated patch with setval() and auto_increment() updated for unsigned types: https://github.com/MariaDB/server/commit/30425c431c5 The diff with the previous patch that you have reviewed: https://github.com/MariaDB/server/commit/bd4a2fac119

            Thank you looks OK now

            sanja Oleksandr Byelkin added a comment - Thank you looks OK now
            ycp Yuchen Pei added a comment - - edited

            rebased to 11.4:

            9548c168fb26083852118402bf6d809a77ae57b8 upstream/bb-11.4-mdev-28152 MDEV-28152 Features for sequences

            Locally tests pass on the sql_sequence suite.

            waiting for CI...

            ycp Yuchen Pei added a comment - - edited rebased to 11.4: 9548c168fb26083852118402bf6d809a77ae57b8 upstream/bb-11.4-mdev-28152 MDEV-28152 Features for sequences Locally tests pass on the sql_sequence suite. waiting for CI...
            ycp Yuchen Pei added a comment -

            Hi lstartseva, ptal at the following commit, thanks

            445ce402b01 upstream/bb-11.4-mdev-28152 MDEV-28152 Features for sequences

            ycp Yuchen Pei added a comment - Hi lstartseva , ptal at the following commit, thanks 445ce402b01 upstream/bb-11.4-mdev-28152 MDEV-28152 Features for sequences

            Testing is completed but MDEV-33012 needs to be fixed before pushing

            lstartseva Lena Startseva added a comment - Testing is completed but MDEV-33012 needs to be fixed before pushing
            ycp Yuchen Pei added a comment -

            Hi lstartseva, could you test the patch on top of the MDEV-33169 patch that fixes MDEV-33012?

            4d2224702fc upstream/bb-11.4-mdev-28152 MDEV-28152 Features for sequences
            09a710036b6 MDEV-33169 Reset sequence used fields after check in alter sequence
            

            ycp Yuchen Pei added a comment - Hi lstartseva , could you test the patch on top of the MDEV-33169 patch that fixes MDEV-33012 ? 4d2224702fc upstream/bb-11.4-mdev-28152 MDEV-28152 Features for sequences 09a710036b6 MDEV-33169 Reset sequence used fields after check in alter sequence

            Really, it looks like patch for MDEV-33169 fixes problems MDEV-33012. Tests passed.
            Feature is ok to push with this changes.

            lstartseva Lena Startseva added a comment - Really, it looks like patch for MDEV-33169 fixes problems MDEV-33012 . Tests passed. Feature is ok to push with this changes.
            ycp Yuchen Pei added a comment - - edited

            Thanks lstartseva. The fixversion was changed to 11.5 and the feature was not included in 11.4-preview, so we have to wait for an 11.5 branch to be created first.

            ycp Yuchen Pei added a comment - - edited Thanks lstartseva . The fixversion was changed to 11.5 and the feature was not included in 11.4-preview, so we have to wait for an 11.5 branch to be created first.
            ycp Yuchen Pei added a comment - - edited

            11.5 branch has been created with the MDEV-33169 patch included. I have verified that MDEV-33012 is fixed by it. So I have applied the patch on top as 374783c3d9a87170b313a05240dadaa813ec6034 and pushed it to 11.5.

            ycp Yuchen Pei added a comment - - edited 11.5 branch has been created with the MDEV-33169 patch included. I have verified that MDEV-33012 is fixed by it. So I have applied the patch on top as 374783c3d9a87170b313a05240dadaa813ec6034 and pushed it to 11.5.

            People

              ycp Yuchen Pei
              rucha174 Rucha Deodhar
              Votes:
              1 Vote for this issue
              Watchers:
              10 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.