Skip to content

Commit

Permalink
sql: fix placeholder type resolution
Browse files Browse the repository at this point in the history
When assignment casts for inserts were add in cockroachdb#70722, type resolution of
placeholders in prepared statements was changed so that during the
prepare phase the optimizer would only plan assignment casts when it
could not be guaranteed that an insert column would be an identical type
to the target column. Unfortunately, the change to placeholder type
resolution change strays from Postgres's behavior and breaks some
third-party tools. This commit reverts the type resolution changes, and
as a result, assignment casts are always planned to convert each insert
column to its target column type, even if the cast ends up being a no-op
because the types are identical.

I attempted to implement a solution that did not plan assignment casts
in all cases, but it proved to be tedious and error prone, so I opted
for this simpler solution for now. My attempt required annotating each
placeholder expression in the memo with a context: a non-assignment
context or an assignment context. This context would be used during
the EXECUTE to determine whether a placeholder value should undergo a
regular cast or an assignment cast: a critical decision due to their
slightly different behaviors (see `PerformAssignmentCast`). The
problem is that correctly annotating placeholders in complex mutations
with expressions like subqueries or CTEs is difficult.

Fixes cockroachdb#71576

There is no release note because this bug was never present in a
release.

Release note: None
  • Loading branch information
mgartner committed Nov 19, 2021
1 parent d96947e commit eab56b1
Show file tree
Hide file tree
Showing 54 changed files with 6,851 additions and 5,216 deletions.
45 changes: 28 additions & 17 deletions pkg/sql/opt/exec/execbuilder/testdata/explain
Original file line number Diff line number Diff line change
Expand Up @@ -793,8 +793,8 @@ vectorized: true
into: t(k, v)
auto commit
size: 2 columns, 1 row
row 0, expr 0: (1)[int]
row 0, expr 1: (2)[int]
row 0, expr 0: (crdb_internal.assignment_cast((1)[int], ((NULL)[unknown]::INT8)[int]))[int]
row 0, expr 1: (crdb_internal.assignment_cast((2)[int], ((NULL)[unknown]::INT8)[int]))[int]

query T
EXPLAIN (TYPES) SELECT 42 AS a
Expand Down Expand Up @@ -1408,20 +1408,26 @@ vectorized: true
│ auto commit
│ arbiter indexes: u_pkey, u_v_key
└── • lookup join (anti)
│ table: u@u_v_key
│ equality: (column2) = (v)
│ equality cols are key
└── • distinct
│ distinct on: column2
│ nulls are distinct
└── • cross join (anti)
├── • values
│ size: 2 columns, 1 row
└── • distinct
│ distinct on: column1
│ nulls are distinct
└── • scan
missing stats
table: u@u_pkey
spans: [/1 - /1]
└── • lookup join (anti)
│ table: u@u_pkey
│ equality: (column1) = (k)
│ equality cols are key
└── • lookup join (anti)
│ table: u@u_v_key
│ equality: (column2) = (v)
│ equality cols are key
└── • values
size: 2 columns, 1 row

# Make sure EXPLAIN (VERBOSE) works when there is a constrained scan of a
# virtual table (#58193).
Expand Down Expand Up @@ -1560,9 +1566,14 @@ EXPLAIN INSERT INTO t2 SELECT x FROM t2 WHERE false
distribution: local
vectorized: true
·
• insert fast path
into: t2(x)
auto commit
• insert
│ into: t2(x)
│ auto commit
└── • render
│ estimated row count: 0
└── • norows

statement ok
PREPARE prep AS SELECT $1 + 1
Expand Down
18 changes: 12 additions & 6 deletions pkg/sql/opt/exec/execbuilder/testdata/explain_analyze_plans
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,17 @@ regions: <hidden>
│ └── • buffer
│ │ label: buffer 1
│ │
│ └── • values
│ nodes: <hidden>
│ regions: <hidden>
│ actual row count: 1
│ size: 2 columns, 1 row
│ └── • render
│ │ nodes: <hidden>
│ │ regions: <hidden>
│ │ actual row count: 1
│ │ estimated row count: 1
│ │
│ └── • values
│ nodes: <hidden>
│ regions: <hidden>
│ actual row count: 1
│ size: 1 column, 1 row
├── • subquery
│ │ id: @S1
Expand Down Expand Up @@ -414,5 +420,5 @@ regions: <hidden>
label: buffer 1
·
Diagram 1 (subquery): https://cockroachdb.github.io/distsqlplan/decode.html#eJysUtFq20oQfb9fMcyTDRssOVwo-5Q0uGDiyMV2DKWYsF4NyhJpV90d1XaNP6s_0C8rlqw2IsRtaR_nzJzZM2fPHsOnHCWOk_lotoBxspiCfjR5Csvryf1oDr1YQG8-moxuFlAY2yv78G42vYNSebLc76NA61JKVEEB5UeMUeD_uBJYeqcpBOeP8L4eGqdblJFAY8uKj_BKoHaeUO6RDeeEEhN34crBEAWmxMrk9VLakq7YOAtsCpIQffsaUOBasX6kAK7ismIJEQr0bvMTiHF1ENhUp_cCq4xQXh7EM03xeU0Ltc5pRiolP4i6yhobrkpvCuV3KHBeKhskXKDAiSkMw9GQ22VX-O0StLNM9uVNt0uoT_CkUtmS1zumFnoDb1Hg3fLmBgJTCdpVlqFHWx4Yy30JUa2xGSB6em2gUFsoqHB-ByrPnVZMqYSo3v4XxsZ_Yux1lnnKFDs_iLu-XicfHpLp4iG5n0x6V_ExZv8-BMOO1l8Ec0ahdDZQR-drm6PDSiClGTXhD67ymt57p-tnmnJa82ogpcBN97IpxrZpHQU-J8dnycPz5OFZctQl16fUV6El3jj_BLlisnr3w_kW3yjD3T9JKZA3Kjdf1MsPa2mnmGsyn-kU9bbV5r3tNZlvuwWFoLLOQPS7QVgd_vseAAD___Xaogg=
Diagram 2 (main-query): https://cockroachdb.github.io/distsqlplan/decode.html#eJyskcFq8zAQhO__Uyx7SkAQK0ed8lNcMKRxSdJeig-uvCQCRXKlVRsIfqy-QJ-s2C60KbQQ6HFmZ0Yf6ITxyaLCYrXJ11soVtsS9N7YBu7_L-_yDUykgMkmX-ZXWzgYN2mncL0ub6CtAzmeTlGg8w2t6gNFVA8osRLYBq8pRh966zQEiuaIKhNoXJu4tyuB2gdCdUI2bAkVWq9rC8-1TRQhm2UosCGujR2Wy8QKFnOxkCiQjqQTG--AzYEUZG-vEQU-1qz3FMEnbvt4PxH8y6chseoEjuqDInK9I1SyE5eSap8cg5zJc9C_Z5tfwram2HoX6Qzqp-WsqwRSs6Pxp6JPQdNt8Hp4ZpTl0BuMhiKPVzmKwo2nHvBrWf5ann8rV92_9wAAAP__OpjUsw==
Diagram 2 (main-query): https://cockroachdb.github.io/distsqlplan/decode.html#eJysktGK1TAQhu99imGuTiG4zV5JrhSpUKg90tP1RsqSTYazgTSpyVQXDn0sX8Ank7aCu4LKgnf5_5l_8mXIBfNnjwrr9lR1PdRtfwRz77yFj2-am-oEByngcKqa6m0PowuHqYB33fE9TDpR4KJAgSFaavVIGdUnlDgInFI0lHNMq3XZGmr7gKoU6MI082oPAk1MhOqC7NgTKvTRaA9ftJ8pQ3lVokBLrJ3fJncULCUFJtm7WxeYUtD-pc7ZncNIgW-NznyQSqm67V-J9qZp9mMh_pF5LR93o0B6IDOziwHYjaSg_P4to8A7zeaeMsSZp5kVrIApfv1lSBwWgbv6-cbM-kyo5CKeuwcT58Agr-TTNfx_tuvnsHWUpxgyPYH60-RyGQSSPdP-D3Kck6EPKZrtml0et9xmWMq8V-Uu6rCXVsDHYfnX8PVv4WF58SMAAP__iA_1uQ==
Diagram 3 (postquery): https://cockroachdb.github.io/distsqlplan/decode.html#eJy0lMGO2jAQhu99itGcQLJEAnuofNrtipWyZJMKslwqDsYZdt0Ndmo7KgjxWH2BPlmVmFVLV6BStTfm9_zjb_it7NB9qZBjks3G0wKSrMhBPquqhPlN-jieQS9m0JuN0_FtAWule3Uf7qb5A9TCkvb9PjLUpqRMrMkh_4QxLhjW1khyzthW2nUNSblBHjFUum58Ky8YSmMJ-Q698hUhx8pIUYGTQsOyWa3IQjSIkGFJXqiqG583nsP1EBnShmTjldHg1Zo4RN-_OWS4FF4-kwPT-Lrtbf3WfP0pxLjYMwzVgcN58UTI4z37c9Y7VXmyZAfxMWDQOVzHkMwgywvIHtP0v_AOL-G9N0pPSZRkB8Nj4mJbE4d0fFfATVYkcJ8nGTIMAV_XVq2F3SLD1JiXpobPRmkwut0Q2SGP9ld3ObToLfNr7byoquONJ_M3tTTak37750zm3UCwJMowdTKH5dbTq_QePiDDh_ntLThPNUjTaA892viB0r7PDw8oNBC9nGr423Cik-GMLgknPHyy1lhQq7BzPBgd5_TvGa8uYZySq412dAR1anK0XzCk8onCB8CZxkr6aI3srgll3vk6oSTnw2kcikSHoxbwV3N81jw8bx6eNY_Om0dnzVe_mRf7dz8CAAD__4iLwK8=
Loading

0 comments on commit eab56b1

Please sign in to comment.