Type Cast all update expressions in verify queries

[GuptaManan100]

Description

This PR fixes the bug described in #14589.

Since fk_t16 has child tables and this is a non-literal update, Vitess has to cascade the changes and then update the fk_t16 table with foreign key checks turned off. This requires Vitess to verify that the updated values are present in the parent table i.e. fk_t15.

Previously Vitess was using a query like select 1 from fk_t16 left join fk_t15 on fk_t15.col = fk_t16.col * (fk_t16.col - fk_t16.col) where fk_t16.col * (fk_t16.col - fk_t16.col) is not null and not (fk_t16.col) <=> (fk_t16.col * (fk_t16.col - fk_t16.col)) and fk_t16.id = 3 and fk_t15.col is null limit 1 for share nowait.

The problem is that fk_t16.col * (fk_t16.col - fk_t16.col) evaluates to -0 but of type float. When compared to fk_t15.col of type varchar 0, MySQL ends up doing the comparison in float space and concludes that -0 and 0 are the same values. This makes Vitess infer that the verification is successful and it is safe to proceed with the update.

The final value however is set in Varchar since the column is of varchar type. This leads to Vitess ending up in a broken state and allowing an update that otherwise should have been rejected.

The way adopted to fix this problem has been to CAST the update expression to the type of the column before using it for comparison. i.e. the verification query now looks like select 1 from fk_t16 left join fk_t15 on fk_t15.col = cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR) where cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR) is not null and not (fk_t16.col) <=> (cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR)) and fk_t16.id = 3 and fk_t15.col is null limit 1 for share nowait

The overall plan looks like -

{
  "OperatorType": "FKVerify",
  "Inputs": [
    {
      "InputName": "VerifyParent-1",
      "OperatorType": "Route",
      "Variant": "Unsharded",
      "Keyspace": {
        "Name": "uks",
        "Sharded": false
      },
      "FieldQuery": "select 1 from fk_t16 left join fk_t15 on fk_t15.col = cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR) where 1 != 1",
      "Query": "select 1 from fk_t16 left join fk_t15 on fk_t15.col = cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR) where cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR) is not null and not (fk_t16.col) <=> (cast(fk_t16.col * (fk_t16.col - fk_t16.col) as CHAR)) and fk_t16.id = 3 and fk_t15.col is null limit 1 for share nowait",
      "Table": "fk_t15, fk_t16"
    },
    {
      "InputName": "PostVerify",
      "OperatorType": "FkCascade",
      "Inputs": [
        {
          "InputName": "Selection",
          "OperatorType": "Route",
          "Variant": "Unsharded",
          "Keyspace": {
            "Name": "uks",
            "Sharded": false
          },
          "FieldQuery": "select col, col <=> cast(col * (col - col) as CHAR), cast(col * (col - col) as CHAR) from fk_t16 where 1 != 1",
          "Query": "select col, col <=> cast(col * (col - col) as CHAR), cast(col * (col - col) as CHAR) from fk_t16 where id = 3 for update nowait",
          "Table": "fk_t16"
        },
        {
          "InputName": "CascadeChild-1",
          "OperatorType": "FkCascade",
          "BvName": "fkc_vals",
          "Cols": [
            0
          ],
          "NonLiteralUpdateInfo": [
            {
              "CompExprCol": 1,
              "UpdateExprCol": 2,
              "UpdateExprBvName": "fkc_upd"
            }
          ],
          "Inputs": [
            {
              "InputName": "Selection",
              "OperatorType": "Route",
              "Variant": "Unsharded",
              "Keyspace": {
                "Name": "uks",
                "Sharded": false
              },
              "FieldQuery": "select col from fk_t17 where 1 != 1",
              "Query": "select col from fk_t17 where (col) in ::fkc_vals and (:fkc_upd is null or (col) not in ((:fkc_upd))) for update nowait",
              "Table": "fk_t17"
            },
            {
              "InputName": "CascadeChild-1",
              "OperatorType": "Update",
              "Variant": "Unsharded",
              "Keyspace": {
                "Name": "uks",
                "Sharded": false
              },
              "TargetTabletType": "PRIMARY",
              "BvName": "fkc_vals1",
              "Cols": [
                0
              ],
              "Query": "update fk_t19 set col = null where (col) in ::fkc_vals1",
              "Table": "fk_t19"
            },
            {
              "InputName": "CascadeChild-2",
              "OperatorType": "Update",
              "Variant": "Unsharded",
              "Keyspace": {
                "Name": "uks",
                "Sharded": false
              },
              "TargetTabletType": "PRIMARY",
              "BvName": "fkc_vals2",
              "Cols": [
                0
              ],
              "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ fk_t18 set col = null where (col) in ::fkc_vals2",
              "Table": "fk_t18"
            },
            {
              "InputName": "Parent",
              "OperatorType": "Update",
              "Variant": "Unsharded",
              "Keyspace": {
                "Name": "uks",
                "Sharded": false
              },
              "TargetTabletType": "PRIMARY",
              "Query": "update fk_t17 set col = null where (col) in ::fkc_vals and (:fkc_upd is null or (col) not in ((:fkc_upd)))",
              "Table": "fk_t17"
            }
          ]
        },
        {
          "InputName": "Parent",
          "OperatorType": "Update",
          "Variant": "Unsharded",
          "Keyspace": {
            "Name": "uks",
            "Sharded": false
          },
          "TargetTabletType": "PRIMARY",
          "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ fk_t16 set col = col * (col - col) where id = 3",
          "Table": "fk_t16"
        }
      ]
    }
  ]
}

This works as intended.

Since we are now type-casting the expressions using the CAST function, we get the correctly typed expressions in SELECT and the changes introduced in #14524 are no longer required. They have now been reverted.

Related Issue(s)

• Fixes Bug Report: Vitess Allows updating a child to value -0 when the parent has 0 if the update is done via an arithmetic expression #14589

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on the CI
• Documentation was added or is not required

Deployment Notes

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.