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

Default TIMESTAMP clause for SELECT from versioned

Details

    Description

      Currently when there is no unit specifier in AS OF query like:

      select * from t1 for system_time as of @p1;
      

      it will do some heuristics to detect whether it should treat @p1 is trx_id or timestamp.

      Such semantic complexity should be removed in favour of default TIMESTAMP because:

      1. rare practical use and doubtful advantage;
      2. it conflicts with MDEV-16226.

      Attachments

        Issue Links

          Activity

            midenok Aleksey Midenkov created issue -

            bar please be aware.

            midenok Aleksey Midenkov added a comment - bar please be aware.
            midenok Aleksey Midenkov made changes -
            Field Original Value New Value
            Description Currently when there is no unit specifier in AS OF query like:
            ```sql
            select * from t1 for system_time as of @p1;
            ```
            it will do some heuristics to detect whether it should treat `@p1` is trx_id or timestamp. There is no need in such heuristics as overwhelming majority will mean timestamp.

            Such needless semantic complexity should be removed in favour of default TIMESTAMP.
            Currently when there is no unit specifier in AS OF query like:
            {code:sql}
            select * from t1 for system_time as of @p1;
            {code}
            it will do some heuristics to detect whether it should treat {{@p1}} is trx_id or timestamp. There is no need in such heuristics as overwhelming majority will mean timestamp.

            Such needless semantic complexity should be removed in favour of default TIMESTAMP.

            midenok, thanks for the link.
            I agree that in case of user variables it's better to go with TIMESTAMP by default,
            as user variables do not have a strict data type.
            The question is though what to do in case of an indirect variable use, e.g.:
            select * from t1 for system_time as of LEAST(@p1,@p2);

            Note, I recently sent a patch fixing MDEV-16094 and MDEV-16100 to Sergei's review:
            https://lists.launchpad.net/maria-developers/msg11272.html

            Let's return to this MDEV after those two.

            bar Alexander Barkov added a comment - midenok , thanks for the link. I agree that in case of user variables it's better to go with TIMESTAMP by default, as user variables do not have a strict data type. The question is though what to do in case of an indirect variable use, e.g.: select * from t1 for system_time as of LEAST(@p1,@p2); Note, I recently sent a patch fixing MDEV-16094 and MDEV-16100 to Sergei's review: https://lists.launchpad.net/maria-developers/msg11272.html Let's return to this MDEV after those two.
            bar Alexander Barkov made changes -
            bar Alexander Barkov made changes -
            midenok Aleksey Midenkov made changes -
            Status Open [ 1 ] In Progress [ 3 ]

            midenok, I suggest you wait for my patch for MDEV-16094 and MDEV-16100 to get pushed first.

            bar Alexander Barkov added a comment - midenok , I suggest you wait for my patch for MDEV-16094 and MDEV-16100 to get pushed first.
            midenok Aleksey Midenkov made changes -
            Status In Progress [ 3 ] Stalled [ 10000 ]

            Alexey, the patch for MDEV-16094 and MDEV-16100 is now in 10.3.
            It added a new virtual method Item::type_handler_for_system_time().
            The easiest way to make user variables always resolve as TIMESTAMP is to override type_handler_for_system_time() in Item_func_user_var.
            Something like this should work:

              const Type_handler *type_handler_for_system_time() const
              {
                return &type_handler_timestamp2;
              }
            

            bar Alexander Barkov added a comment - Alexey, the patch for MDEV-16094 and MDEV-16100 is now in 10.3. It added a new virtual method Item::type_handler_for_system_time(). The easiest way to make user variables always resolve as TIMESTAMP is to override type_handler_for_system_time() in Item_func_user_var. Something like this should work: const Type_handler *type_handler_for_system_time() const { return &type_handler_timestamp2; }
            midenok Aleksey Midenkov made changes -
            Comment [ [~bar] I don't think changes from MDEV-16100 are needed. TIMESTAMP should be **always** ]

            bar I don't think changes from MDEV-16100 are needed. TIMESTAMP should be always default. That's too much logic about unit resolution for almost no win in usability.

            midenok Aleksey Midenkov added a comment - bar I don't think changes from MDEV-16100 are needed. TIMESTAMP should be always default. That's too much logic about unit resolution for almost no win in usability.
            bar Alexander Barkov added a comment - - edited

            Changes from MDEV-16100 that added new methods in Type_handler are needed anyway, to disallow various non-relevant data types in FOR SYSTEM_TIME, such as GEOMETRY, ROW, ENUM/SET. And we'll be adding new data types soon (e.g. INET4, INET6, UUID), which are also non-relevant in timestamp context.

            I think it's fine to make undefined units to resolve to timestamp by default.
            But in this case the following expressions in FOR SYSTEM_TIME should probably return an error when used without explicit unit:

            • Expressions that have the BIT data type (i.e. table fields, SP variables, hybrid functions like COALESCE)
            • BIT-alike constants B'01011101'
            • HEX hybrid constants 0x616263
              because the user is obviously doing something wrong. These expressions can very rarely represent a valid TIMESTAMP.

            So they should require either an explicit unit, or even an explicit CAST.

            bar Alexander Barkov added a comment - - edited Changes from MDEV-16100 that added new methods in Type_handler are needed anyway, to disallow various non-relevant data types in FOR SYSTEM_TIME, such as GEOMETRY, ROW, ENUM/SET. And we'll be adding new data types soon (e.g. INET4, INET6, UUID), which are also non-relevant in timestamp context. I think it's fine to make undefined units to resolve to timestamp by default. But in this case the following expressions in FOR SYSTEM_TIME should probably return an error when used without explicit unit: Expressions that have the BIT data type (i.e. table fields, SP variables, hybrid functions like COALESCE) BIT-alike constants B'01011101' HEX hybrid constants 0x616263 because the user is obviously doing something wrong. These expressions can very rarely represent a valid TIMESTAMP. So they should require either an explicit unit, or even an explicit CAST.

            bar Okay, I see your point. It seems, that it is not possible to simplify anything if type checking is required in SYSTEM_TIME. Unit resolution comes as a "free bonus". Therefore, closing.

            midenok Aleksey Midenkov added a comment - bar Okay, I see your point. It seems, that it is not possible to simplify anything if type checking is required in SYSTEM_TIME. Unit resolution comes as a "free bonus". Therefore, closing.
            midenok Aleksey Midenkov made changes -
            Fix Version/s 10.3.7 [ 23005 ]
            Resolution Won't Fix [ 2 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            midenok Aleksey Midenkov made changes -
            Description Currently when there is no unit specifier in AS OF query like:
            {code:sql}
            select * from t1 for system_time as of @p1;
            {code}
            it will do some heuristics to detect whether it should treat {{@p1}} is trx_id or timestamp. There is no need in such heuristics as overwhelming majority will mean timestamp.

            Such needless semantic complexity should be removed in favour of default TIMESTAMP.
            Currently when there is no unit specifier in AS OF query like:
            {code:sql}
            select * from t1 for system_time as of @p1;
            {code}
            it will do some heuristics to detect whether it should treat {{@p1}} is trx_id or timestamp.

            Such semantic complexity should be removed in favour of default TIMESTAMP because:

            1. rare practical use and doubtful advantage;
            2. it conflicts with MDEV-16226.
            midenok Aleksey Midenkov made changes -
            Resolution Won't Fix [ 2 ]
            Status Closed [ 6 ] Stalled [ 10000 ]
            midenok Aleksey Midenkov made changes -

            This is the requirement for MDEV-16226.

            midenok Aleksey Midenkov added a comment - This is the requirement for MDEV-16226 .
            midenok Aleksey Midenkov made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            midenok Aleksey Midenkov made changes -
            Fix Version/s 10.3 [ 22126 ]
            Fix Version/s 10.3.7 [ 23005 ]
            midenok Aleksey Midenkov made changes -
            Assignee Aleksey Midenok [ midenok ] Alexander Barkov [ bar ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            midenok Aleksey Midenkov made changes -
            bar Alexander Barkov added a comment - - edited

            Discussed with Aleksey on slack to make the patch more Type_handler friendly.

            bar Alexander Barkov added a comment - - edited Discussed with Aleksey on slack to make the patch more Type_handler friendly.
            bar Alexander Barkov made changes -
            Assignee Alexander Barkov [ bar ] Aleksey Midenok [ midenok ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            midenok Aleksey Midenkov made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            midenok Aleksey Midenkov made changes -
            Assignee Aleksey Midenok [ midenok ] Alexander Barkov [ bar ]
            Status In Progress [ 3 ] In Review [ 10002 ]

            The latest version in https://github.com/MariaDB/server/pull/860 is now Type_handler friendly. Ok to push from my side.

            bar Alexander Barkov added a comment - The latest version in https://github.com/MariaDB/server/pull/860 is now Type_handler friendly. Ok to push from my side.
            bar Alexander Barkov made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            bar Alexander Barkov made changes -
            Assignee Alexander Barkov [ bar ] Aleksey Midenok [ midenok ]
            midenok Aleksey Midenkov made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            midenok Aleksey Midenkov made changes -
            Assignee Aleksey Midenok [ midenok ] Sergei Golubchik [ serg ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            serg Sergei Golubchik made changes -
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.3 [ 22126 ]
            serg Sergei Golubchik made changes -
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.4 [ 22408 ]
            midenok Aleksey Midenkov made changes -
            Assignee Sergei Golubchik [ serg ] Aleksey Midenkov [ midenok ]
            midenok Aleksey Midenkov made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            midenok Aleksey Midenkov made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            midenok Aleksey Midenkov made changes -
            Assignee Aleksey Midenkov [ midenok ] Alexander Barkov [ bar ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            midenok Aleksey Midenkov made changes -
            Assignee Alexander Barkov [ bar ] Aleksey Midenkov [ midenok ]
            midenok Aleksey Midenkov made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            midenok Aleksey Midenkov made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            midenok Aleksey Midenkov made changes -
            issue.field.resolutiondate 2019-09-30 12:39:26.0 2019-09-30 12:39:26.093
            midenok Aleksey Midenkov made changes -
            Fix Version/s 10.5.0 [ 23709 ]
            Fix Version/s 10.5 [ 23123 ]
            Resolution Fixed [ 1 ]
            Status In Progress [ 3 ] Closed [ 6 ]
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 87086 ] MariaDB v4 [ 133542 ]

            People

              midenok Aleksey Midenkov
              midenok Aleksey Midenkov
              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.