Merge #97554

97554: physicalplan: don't combine render nodes into one processor r=DrewKimball a=DrewKimball

During planning for a render node, the physical planning logic checks whether the last created processor rendered its own expressions, adds its render expressions to the processor if not. The check is necessary to handle the case when expressions in a parent render reference those from a child render.

However, render nodes also handle projecting input columns: both removing them and reordering them. Correct planning of the processors depends on the columns at each step being in a particular order. Since physical planning didn't check whether the existing processor projected its columns in some way, the parent render could be combined with its child. This could lead to panics in the parent render expressions when the columns weren't projected as expected. This patch fixes the issue by adding the projection check before allowing the render planning code to add to the previous processor.

Fixes #95633

Release note (bug fix): Fixed a rare bug existing since before 22.1 that
could cause a projected expression to replace column references with the
wrong values.

Co-authored-by: Drew Kimball <[email protected]>

pkg/ccl/logictestccl/testdata/logic_test/multi_region_query_behavior

@@ -410,7 +410,7 @@ vectorized: true
                   table: orders@orders_pkey
                   spans: [/'ap-southeast-2'/'94e4b847-8f2f-4ac5-83f1-641d6e3df727' - /'ap-southeast-2'/'94e4b847-8f2f-4ac5-83f1-641d6e3df727'] [/'us-east-1'/'94e4b847-8f2f-4ac5-83f1-641d6e3df727' - /'us-east-1'/'94e4b847-8f2f-4ac5-83f1-641d6e3df727']
 ·
-Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyslM1uGjEQx-99CmsuaSMT9otAVorkKKUtEYUUIrVSjdBmPSRuwN7a3nariFOfre9V7S4hISpUSXsA2-P5M_Mbz3AL9uscYuh-Ou-f9Abk5eve-GL8of-KjLv97ukF0UagmUpBSWb0QtuDalkZZIrkZFxvpt-lu9a5m1YOd7f7REib6ly5Tcfai7wZDd_XISw5G_YGqyBkeLc7kGqmCeO554W48qxtH991R911euSY7B1FGF12onajMwtmjShJW41OOPMbh5EvDjEUs3bQ3gMKSgscJAu0EH-GCCYUMqNTtFab0nRbOfREAbFHQaosd6V5QiHVBiG-BSfdHCGGi-RyjiNMBJqmBxQEukTOq5-tM2X1Ms1u8AdQONXzfKFsvFFUmSIlJRFQGGdJed3kwDg0OXBeHEWcF1h-XXbecl50Zs2zXz85L2a-4LzwhTouD-09Dk2PJEoQn2h3jQYmSwo6d_fpW5dcIcT-kj4P0f-_iCWM9yzMrWjBVrR7IotGJnOSqypDFBtQk-UfajDQDZ01g036vlxIR_ytqXhPqXJPfUPjUJxpqdA0w81Q9SSwepmWdZxKUQBdy7pFZghrrceEhQ9f4n5gV08wzF1MmE9ZQFlIWbSVIXwKQ5n7qlGiXfmvGqWv9U2ekS9aKqJVTFgpGg4Ia_8V4-4fZQ00QiXQVEyEtShhQfnZZ4dbyaKnkI3QZlpZfNQp2959QgHFFdZdZ3VuUjw3Oq3C1MdhpasMAq2rb4P60FPVVTWkD8X-v4iDneJwQ-w9Foc7xdFucbRT3Hoknixf_A4AAP__TIEqRA==
+Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyslN1u2koQx-_PU6zmJudES_AnEEuRNsqhLRGFFCK1Uhchxzsk28Cuu163riKu-mx9r8o2-SCqqUh7Aeudmb9nfrOzvoPs8xIi6H-4GJ4ORuTf_wfTy-m74X9k2h_2zy6JNgLNXApKUqNXOjuqlo1BJkhOp_XD_Ku0Nzq38yrg3ntIhMwSnSu7HVhHkVeT8ds6RUbOx4PRJgkZ3z8dSbXQhPHccXzcRNa292_6k_5DeeSEHBwHGFz1gm6rt_AWrSBOwlbPX7itTuCKDvpi0fW6B0BBaYGjeIUZRB8hgBmF1OgEs0yb0nRXBQxEAZFDQao0t6V5RiHRBiG6AyvtEiGCy_hqiROMBZq2AxQE2lguq9fWlbJ6mae3-A0onOllvlJZtNVUmSAlJRFQmKZx6W5zYBzaHDgvjgPOCyz_rnqvOS96i_b5j--cFwtXcF64Qp2Um-4Bh7ZDYiWIS7S9QQOzNQWd28fyMxtfI0Tumr4M0f27iCWM8yLMRjSvEe2RKEMj4yXJVVUhii2o2foXPRjplk7b3jb9UK6kJW5jKc4-XR6oL2gsinMtFZq2v52qvgmsXuZlH-dSFEAfZP0iNYSFD9eE-U9P4vHCbo5gnNuIMJcyjzKfsqCRwd-Hoax9MyjBrvo3gzLU-jZPySctFdEqIqwUjUeEdX-Lcf9FaQIKKetQ1m3ECvbBqg8_3CaaoBJoqpyEBZQwr_wdsrAxZ7hPzglmqVYZPhvNpkGbUUBxjfWYZzo3CV4YnVRp6u240lUGgZmtvV69GajKVX0VnordPxF7O8X-lth5LvZ3ioPd4mCnONwtDneKO8_Es_U_PwMAAP__O2xhIQ==
 
 # Regression test for #74890. Code should not panic due to distribution already
 # provided by input.

pkg/sql/logictest/testdata/logic_test/pg_catalog

@@ -4610,3 +4610,8 @@ WHERE
     AND t.typname LIKE 'myt%';
 ----
 0
+
+# Regression test for a panic after physical planning combined rendering steps
+# (#95633).
+statement ok
+SELECT d.datname, pg_catalog.pg_get_userbyid(d.datdba) FROM pg_catalog.pg_database d;

pkg/sql/opt/exec/execbuilder/testdata/distsql_merge_join

@@ -823,7 +823,7 @@ vectorized: true
           table: outer_c1@outer_c1_pkey
           spans: FULL SCAN
 ·
-Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzMll9r4kAUxd_3Uwz3SbcjyeSPtYGFFNcuFqtdtbCwSEmTuyprM9lJhC3F774kqbiGNpMwKr5IMpnf3DnnXDN5hfjPChzo_bgfXPeHpPG1P5lOvg-aZNIb9LpT8pncjEd3hK8TFI8RIzcPgwG5HfWHb0M-Iw-T_vAbaUTLgDWBQsgDHHrPGIPzExhQMICCCRQsoGDDjEIkuI9xzEU65TUD-sFfcHQKyzBaJ-nwjILPBYLzCskyWSE4MPWeVjhGL0ChpesGmHjLVVZmuz13e_EY_cYXoNDlq_VzGDsk3R0lkZeCk8hLhzS9pdnECwPCCE8WKGC2oelSb1vYVX56IQsvXuzXdBnMNjMKceLNERy2oR9I2a3DRYACg-I6F2nh3ax1-N68rFbRkjsUc7zlyxCFdrm_7PQlQiePa_Qw7Y2z0IDCCn8lDZddNL-I5XyRXwKFMYYBCod0R9eD3qTba7iMumaTEtegxLUocW1K3HbBop18s4L8GsKGvMUjjbHCzPdrW3u1WfUuMg7RRXZLY_qR2oidvo06Z9RGRvUozUNEyfSWxo71SjBOn-XVGWVpVs_SOkiWdkszjvW_NE-fJdPPKEyrepj6AcJsaccKsoYQ-z0h_laI_5EQf_trUOLviVKSYXwo40T92D6jdpR8OI4xjngYY6WPCT0VjsEccy9jvhY-3gvuZ2Xy21HGZcdzgHGSP2Vvd_0wf5busAatAlsqcFsF7pTDrAjr_8NGOWyUwmyf1ou0qRSWhFaBJWGVw5KwymFJWJZKWLaS3RJaBZbYXQ5L7C6HJXa3Vey-VLJbQqvAErvLYYnd5bDE7o6K3VdKdktoFVhidzkssbsclr35ax2WRl1cibaU6LYS3ZHQ9U7M2ebTvwAAAP__BTlqQg==
+Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzEVu9r4koU_f7-iuF-0teRZPLD2sCDFJ99WKz2qYWFRUqa3FVZm8lOImwp_u9LTMTV1YxhjH4Rk8yZc-49J3fyCfGPBTjQ-fLcu-_2Se3f7mg8-r9XJ6NOr9Mek7_Jw3DwRPgyQfEaMfLw0uuRx0G3n9_yGXkZdfv_kVo0D1gdKIQ8wL73jjE4X4EBBQMomEDBAgo2TChEgvsYx1ykSz7XgG7wExydwjyMlkl6e0LB5wLB-YRkniwQHBh7bwscoheg0NJ9A0y8-WJNs5Hnbv68Rt_xAyi0-WL5HsYOSdVREnkpcBR56S1Nb2g28cKAMMKTGQqYrGi6VS5hy_z2QWZePNvldBlMVhMKceJNERy2okdK2e7DRYACg_19blLi7apleGjdmmu_JU8opvjI5yEK7XZ32_FHhE5m1-Bl3BmuTQMKC_yW1Fx2U_9HzKez7C9QGCwTh7iMugZ1Tepa1LWp29zryLZa82i1B1T2eYNHGjN2BQ4xDFA4pD2473VG7U4tZTfrlLgGJa5FiWtTUiDBOqHhJVqZi2zurTzMbe9ws9Nza5wjt3ZDY3pFwWWXD27resGVOJdnwqwyuMbp4THPER6mNzRW1dgzLp-eu-ulR2Jdnh6ryvSYp6fHOkt67IZmVDV7zMunh-nXi4_Euzw-dpXxsU6Pj36G-DS0qqJTohD7UCH-phD_WCH-5tegxN8pSqkM42gZF3oDmtd7ASSm5S8Au9Rn4wEJQ4wjHsZ40kehnrYagylm7sV8KXx8Ftxf02SXgzVu_eERYJxkT5mdXXXD7FmqsARaBazE3FKSLdHN9tH672hjB6zvg41CsFnMbBaC2W0xtaVktQStAlZillgtkS3RbRc2vFnc76aK1bdKZknQKmAlZolZEtkS3a3Cft8Vm3WnYhZTm6IyuBJajVtimEy5THrxKGWSWcqUhin7Y5qWM00CV0KrcctMkyiXSS91jhhl4UpoNe6WmnKZ9OLDhElOE1buOJms_voVAAD__whhgYA=
 
 query T
 EXPLAIN (VERBOSE) SELECT * FROM outer_gc1 FULL OUTER JOIN outer_c1 USING (pid1, cid1, cid2)
@@ -883,7 +883,7 @@ vectorized: true
           table: outer_c1@outer_c1_pkey
           spans: FULL SCAN
 ·
-Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyckt9um0wQxe-_p1jNlf15InvB6R-kShu5pCIidgpYqlRZEWWnFJWwdBekRpbfvQJax6DYcXuD2LNzht8ZZgvmRw4OuJ_u_CtvyUbvvTAKP_pjFrq-u4jY_-w6WN0yVVek79OEs-u177Oblbf8rSWcrUNv-YGNykxyZMmfpzUGhEJJWsYPZMD5DBwQLmGDUGqVkDFKN_K2LfLkT3BmCFlR1lUjbxASpQmcLVRZlRM4sFQXqpzagCCpirO8LdshqLp6MpkqTgmc-Q4PGvPTjaP4S04BxZL0dNZrD_vgYv92X36nR0BYqLx-KIzDhsGRpd0hTeImc1jGTdkFHIPl_wrLn4Pds56FeiaidRTxiawulJakSQ7_0Mslz-S8JZ3SjcoK0lOrnzN6LMnpNnG1jtyg3UdAyOlrNRJ8gsKaoLAn43c6S7_1JUAIqJCkHbZYXfluuHBHgqN4NcYDwULxuifYKN6MkYk5MnGJTLw9Oim7N6kXVjogU6rC0Fk7PWvGRDKlbuxG1TqhO62S9jPdcdX6WkGSqbrbeXfwiu6qATw085Nmu2fmQ7P1F2ZraLZPmmcD7M3uv18BAAD__7KcfNU=
+Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyck1Fvm0AMx9_3KU5-ShZXyQFpO6RJV2V0oqKhC4k0aYoqBh5Doxw7DmlVlO8-AVsaooamfUH4f_6bn7FvA-XvDGxwvt55V-6cDT65wTL44g1Z4HjObMnes-uFf8tkpUndJxFn1yvPYze-O_-nRZytAnf-mQ2KNObIov9PYwgIuYxpHj5QCfY34IAwhTVCoWREZSlVLW-aJDf-A_YEIc2LStfyGiGSisDegE51RmDDXJ7JYmwBQkw6TLMmbYsgK_1kKnWYENjTLe4V5v2Fl-H3jBYUxqTGk0552DUudm_3xS96BISZzKqHvLTZYePIkjZIorDuOSjCOu0MjsHyt8Ly52B3rCehnohoHEV8IqtyqWJSFB9O6OWUZ_q8JZXQjUxzUmOj2-fysSC73UR_tXQWzT4CQkY_9EDwEQpjhMIcDT-qNPnZlQDBr7TNBEdhoDBRWCimKM5RXKC4RPHh6D8wXzOmdlnNLviC8piUzWb-lecEM2dQU5wPcU8wUFx0BBPF5RCZsJCJKbIePqvD98JlWlBZyLykk27TpB4QxQm1Ay9lpSK6UzJqPtOGfuNrhJhK3Z5O28DN26MacN_Me81mx8wPzcYrzMah2ew1W_3YVq95cmBeb9_9DQAA__8XQ7Wb
 
 query T
 EXPLAIN (VERBOSE) SELECT * FROM outer_c1 LEFT OUTER JOIN outer_p1 USING (pid1) WHERE pid1 >= 0 AND pid1 < 40

pkg/sql/physicalplan/physical_plan.go

@@ -662,11 +662,11 @@ func (p *PhysicalPlan) AddRendering(
 	}
 
 	post := p.GetLastStagePost()
-	if len(post.RenderExprs) > 0 {
+	if len(post.RenderExprs) > 0 || len(post.OutputColumns) > 0 {
 		post = execinfrapb.PostProcessSpec{}
-		// The last stage contains render expressions. The new renders refer to
-		// the output of these, so we need to add another "no-op" stage to which
-		// to attach the new rendering.
+		// The last stage contains render expressions, or is projecting input columns.
+		// The new renders refer to the output of these in a particular order, so we
+		// need to add another "no-op" stage to which to attach the new rendering.
 		p.AddNoGroupingStage(
 			execinfrapb.ProcessorCoreUnion{Noop: &execinfrapb.NoopCoreSpec{}},
 			post,

pkg/sql/physicalplan/physical_plan_test.go

@@ -163,7 +163,6 @@ func TestProjectionAndRendering(t *testing.T) {
 				); err != nil {
 					t.Fatal(err)
 				}
-
 			},
 
 			expPost: execinfrapb.PostProcessSpec{
@@ -172,6 +171,38 @@ func TestProjectionAndRendering(t *testing.T) {
 			},
 			expResultTypes: "B,D,C",
 		},
+
+		// Rendering after projection.
+		{
+			post: execinfrapb.PostProcessSpec{
+				Projection:    true,
+				OutputColumns: []uint32{3, 1, 4, 2},
+			},
+			resultTypes: "A,B,C,D",
+
+			action: func(p *PhysicalPlan) {
+				if err := p.AddRendering(
+					context.Background(),
+					[]tree.TypedExpr{
+						&tree.IndexedVar{Idx: 0},
+						&tree.IndexedVar{Idx: 1},
+						&tree.IndexedVar{Idx: 2},
+					},
+					fakeExprContext{},
+					[]int{2, 0, 3, 1},
+					[]*types.T{strToType("C"), strToType("A"), strToType("D")},
+					execinfrapb.Ordering{},
+				); err != nil {
+					t.Fatal(err)
+				}
+			},
+
+			expPost: execinfrapb.PostProcessSpec{
+				Projection:    true,
+				OutputColumns: []uint32{4, 3, 2},
+			},
+			expResultTypes: "C,A,D",
+		},
 	}
 
 	for testIdx, tc := range testCases {