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

Spider variables that double as table params overriding mechanism is buggy

Details

    Description

      Spider has variables and table parameters, and some variables double as table parameters. Examples include spider_same_server_link, spider_casual_read and spider_read_only_mode. These are the variables we consider in this ticket.

      All such variables are of the types of int, longlong or double.

      For these variables, the existing overriding mechanism does the following: 1) if the var has value -1, defer to the table parameter value; and 2) if the var value is not -1, it overrides the table param value. The table param value is in turn set to default in spider_set_connect_info_default() if not specified. MDEV-27169 marks 1) as deprecated.

      MDEV-27169 also changed the default value of the vars from -1 to the actual default values used in spider_set_connect_info_default(). As a consequence, table params are ignored unless the user sets the variable to -1 (say with the SET command) which would trigger a deprecation warning.

      It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

      int spider_param_read_only_mode(
        THD *thd,
        int read_only_mode
      ) {
        DBUG_ENTER("spider_param_read_only_mode");
        DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
          read_only_mode : THDVAR(thd, read_only_mode));
      }
      

      Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

      If we want to do a fixup, one idea would be to remove the assignment of default values in spider_set_connect_info_default() to these table params, so that they remain -1 which was assigned at the beginning of spider_parse_connect_info(). Then in functions like spider_param_read_only_mode(), replace the ternary expression with:

        DBUG_RETURN(read_only_mode == -1 ?
          THDVAR(thd, read_only_mode) : read_only_mode);
      

      This way:

      • the var is set to actual default value if the user does not set it
      • if the user sets the table param but not the var, the table param would be used
      • if the user sets the var but not the table param, the var would be used
      • if the user sets both, the table param would be used

      If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

      [update on 2023-07-20]: we did a roll-up for all versions, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.

      Attachments

        Issue Links

          Activity

            ycp Yuchen Pei created issue -
            ycp Yuchen Pei made changes -
            Field Original Value New Value
            ycp Yuchen Pei made changes -
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say use the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say use the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say use the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used
            ycp Yuchen Pei made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used
            ycp Yuchen Pei made changes -
            Fix Version/s 10.9 [ 26905 ]
            Fix Version/s 10.10 [ 27530 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+
            Roel Roel Van de Paar made changes -
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.6 [ 24028 ]
            Affects Version/s 10.4 [ 22408 ]
            Affects Version/s 10.5 [ 23123 ]
            Affects Version/s 10.6 [ 24028 ]
            Affects Version/s 10.9 [ 26905 ]
            Affects Version/s 10.10 [ 27530 ]
            Affects Version/s 10.11 [ 27614 ]
            Affects Version/s 11.0 [ 28320 ]
            Affects Version/s 11.1 [ 28549 ]
            Roel Roel Van de Paar made changes -
            Status Open [ 1 ] Confirmed [ 10101 ]
            ycp Yuchen Pei made changes -
            Status Confirmed [ 10101 ] In Progress [ 3 ]
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables seem to be int (todo: verify).

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+
            ycp Yuchen Pei made changes -
            Assignee Yuchen Pei [ JIRAUSER52627 ] Alexey Botchkov [ holyfoot ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            holyfoot Alexey Botchkov made changes -
            Assignee Alexey Botchkov [ holyfoot ] Yuchen Pei [ JIRAUSER52627 ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            ycp Yuchen Pei made changes -
            ycp Yuchen Pei made changes -
            Fix Version/s 10.4.31 [ 29010 ]
            Fix Version/s 10.5.22 [ 29011 ]
            Fix Version/s 10.6.15 [ 29013 ]
            Fix Version/s 10.9.8 [ 29015 ]
            Fix Version/s 10.10.6 [ 29017 ]
            Fix Version/s 10.11.5 [ 29019 ]
            Fix Version/s 11.0.3 [ 28920 ]
            Fix Version/s 11.1.2 [ 28921 ]
            Fix Version/s 11.2.1 [ 29034 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.6 [ 24028 ]
            Fix Version/s 10.9 [ 26905 ]
            Fix Version/s 10.10 [ 27530 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

            [update on 2023-07-20]: we did a roll-up, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

            [update on 2023-07-20]: we did a roll-up, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

            [update on 2023-07-20]: we did a roll-up for all versions, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.
            ycp Yuchen Pei made changes -
            Description Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, existing overriding mechanism, which was marked for deprecation by MDEV-27169, says: if the var has value -1, defer to the table parameter value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified.

            MDEV-27169 changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

            [update on 2023-07-20]: we did a roll-up for all versions, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.
            Spider has variables and table parameters, and some variables double as table parameters. Examples include {{spider_same_server_link}}, {{spider_casual_read}} and {{spider_read_only_mode}}. These are the variables we consider in this ticket.

            All such variables are of the types of int, longlong or double.

            For these variables, the existing overriding mechanism does the following: 1) if the var has value -1, defer to the table parameter value; and 2) if the var value is not -1, it overrides the table param value. The table param value is in turn set to default in {{spider_set_connect_info_default()}} if not specified. MDEV-27169 marks 1) as deprecated.

            MDEV-27169 also changed the default value of the vars from -1 to the actual default values used in {{spider_set_connect_info_default()}}. As a consequence, table params are ignored unless the user sets the variable to -1 (say with the {{SET}} command) which would trigger a deprecation warning.

            It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

            {code:c++}
            int spider_param_read_only_mode(
              THD *thd,
              int read_only_mode
            ) {
              DBUG_ENTER("spider_param_read_only_mode");
              DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0
                read_only_mode : THDVAR(thd, read_only_mode));
            }
            {code}

            Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

            If we want to do a fixup, one idea would be to remove the assignment of default values in {{spider_set_connect_info_default()}} to these table params, so that they remain -1 which was assigned at the beginning of {{spider_parse_connect_info()}}. Then in functions like {{spider_param_read_only_mode()}}, replace the ternary expression with:

            {code:c++}
              DBUG_RETURN(read_only_mode == -1 ?
                THDVAR(thd, read_only_mode) : read_only_mode);
            {code}

            This way:
            - the var is set to actual default value if the user does not set it
            - if the user sets the table param but not the var, the table param would be used
            - if the user sets the var but not the table param, the var would be used
            - if the user sets both, the table param would be used

            If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

            [update on 2023-07-20]: we did a roll-up for all versions, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.

            People

              ycp Yuchen Pei
              ycp Yuchen Pei
              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.