From edd449f8a85df624c423ec2d9ad85d2ef2f89492 Mon Sep 17 00:00:00 2001 From: Max Hoffman Date: Thu, 26 Jan 2023 12:41:18 -0800 Subject: [PATCH 1/3] fix expression round trip bug --- enginetest/queries/query_plans.go | 10 ++++++---- sql/expression/in.go | 13 +++---------- sql/expression/in_test.go | 15 +++++++++++++++ sql/plan/foreign_key_handler.go | 4 ++++ 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/enginetest/queries/query_plans.go b/enginetest/queries/query_plans.go index 830c49d1f2..e14e35f6af 100644 --- a/enginetest/queries/query_plans.go +++ b/enginetest/queries/query_plans.go @@ -14984,11 +14984,13 @@ WHERE id IN ('1', '2', '3')`, " └─ Delete\n" + " └─ Filter\n" + " ├─ HashIn\n" + - " │ ├─ HGMQ6.id\n" + - " │ └─ ('1', '2', '3')\n" + - " └─ IndexedTableAccess(HGMQ6)\n" + + " │ ├─ HGMQ6.id:0!null\n" + + " │ └─ TUPLE(1 (longtext), 2 (longtext), 3 (longtext))\n" + + " └─ IndexedTableAccess\n" + " ├─ index: [HGMQ6.id]\n" + - " └─ filters: [{[2, 2]}, {[3, 3]}, {[1, 1]}]\n" + + " ├─ static: [{[2, 2]}, {[3, 3]}, {[1, 1]}]\n" + + " └─ Table\n" + + " └─ name: HGMQ6\n" + "", }, { diff --git a/sql/expression/in.go b/sql/expression/in.go index d389cb066e..0f901b5717 100644 --- a/sql/expression/in.go +++ b/sql/expression/in.go @@ -129,11 +129,8 @@ func (in *InTuple) WithChildren(children ...sql.Expression) (sql.Expression, err } func (in *InTuple) String() string { - pr := sql.NewTreePrinter() - _ = pr.WriteNode("IN") - children := []string{fmt.Sprintf("left: %s", in.Left()), fmt.Sprintf("right: %s", in.Right())} - _ = pr.WriteChildren(children...) - return pr.String() + // scalar expression must round-trip + return fmt.Sprintf("(%s IN %s)", in.Left(), in.Right()) } func (in *InTuple) DebugString() string { @@ -310,11 +307,7 @@ func convertOrTruncate(ctx *sql.Context, i interface{}, t sql.Type) (interface{} } func (hit *HashInTuple) String() string { - pr := sql.NewTreePrinter() - _ = pr.WriteNode("HashIn") - children := []string{hit.Left().String(), hit.Right().String()} - _ = pr.WriteChildren(children...) - return pr.String() + return fmt.Sprintf("(%s HASH IN %s)", hit.Left(), hit.Right()) } func (hit *HashInTuple) DebugString() string { diff --git a/sql/expression/in_test.go b/sql/expression/in_test.go index 182fb2670d..3ac659e299 100644 --- a/sql/expression/in_test.go +++ b/sql/expression/in_test.go @@ -15,6 +15,7 @@ package expression_test import ( + "github.com/stretchr/testify/assert" "testing" "time" @@ -32,6 +33,20 @@ var testEnumType = types.MustCreateEnumType([]string{"", "one", "two"}, sql.Coll var testSetType = types.MustCreateSetType([]string{"", "one", "two"}, sql.Collation_Default) +func TestRoundTripNames(t *testing.T) { + assert.Equal(t, "(foo IN (foo, 2))", expression.NewInTuple(expression.NewGetField(0, types.Int64, "foo", false), + expression.NewTuple( + expression.NewGetField(0, types.Int64, "foo", false), + expression.NewLiteral(int64(2), types.Int64), + )).String()) + hit, err := expression.NewHashInTuple(nil, expression.NewGetField(0, types.Int64, "foo", false), + expression.NewTuple( + expression.NewLiteral(int64(2), types.Int64), + )) + assert.NoError(t, err) + assert.Equal(t, "(foo HASH IN (2))", hit.String()) +} + func TestInTuple(t *testing.T) { testCases := []struct { name string diff --git a/sql/plan/foreign_key_handler.go b/sql/plan/foreign_key_handler.go index 1c6a8426ec..334c984859 100644 --- a/sql/plan/foreign_key_handler.go +++ b/sql/plan/foreign_key_handler.go @@ -51,6 +51,10 @@ func (n *ForeignKeyHandler) String() string { return n.OriginalNode.String() } +func (n *ForeignKeyHandler) DebugString() string { + return sql.DebugString(n.OriginalNode) +} + // Schema implements the interface sql.Node. func (n *ForeignKeyHandler) Schema() sql.Schema { return n.OriginalNode.Schema() From fbdc13bddd0c352e18fdfc5fb2c4547118ce9847 Mon Sep 17 00:00:00 2001 From: max-hoffman Date: Thu, 26 Jan 2023 20:42:47 +0000 Subject: [PATCH 2/3] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/expression/in_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/expression/in_test.go b/sql/expression/in_test.go index 3ac659e299..d090c1b9c4 100644 --- a/sql/expression/in_test.go +++ b/sql/expression/in_test.go @@ -15,11 +15,11 @@ package expression_test import ( - "github.com/stretchr/testify/assert" "testing" "time" "github.com/dolthub/vitess/go/sqltypes" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/src-d/go-errors.v1" From f641d7e9dd27bca802a8b4bb03453d2af89b8c7e Mon Sep 17 00:00:00 2001 From: Max Hoffman Date: Thu, 26 Jan 2023 12:43:18 -0800 Subject: [PATCH 3/3] plan updates --- enginetest/queries/query_plans.go | 38 ++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/enginetest/queries/query_plans.go b/enginetest/queries/query_plans.go index e14e35f6af..e80b557b1e 100644 --- a/enginetest/queries/query_plans.go +++ b/enginetest/queries/query_plans.go @@ -15069,27 +15069,37 @@ SET nd.KNG7T = (SELECT gn.id FROM WE72E gn INNER JOIN TDRVG ltnm ON ltnm.SSHPJ = WHERE nd.FGG57 IS NOT NULL AND nd.KNG7T IS NULL`, ExpectedPlan: "RowUpdateAccumulator\n" + " └─ Update\n" + - " └─ UpdateSource(SET nd.KNG7T = Subquery\n" + + " └─ UpdateSource(SET nd.KNG7T:2 = Subquery\n" + " ├─ cacheable: false\n" + " └─ Project\n" + - " ├─ columns: [gn.id]\n" + + " ├─ columns: [gn.id:17!null]\n" + " └─ Filter\n" + - " ├─ (ltnm.FGG57 = nd.FGG57)\n" + + " ├─ Eq\n" + + " │ ├─ ltnm.FGG57:22!null\n" + + " │ └─ nd.FGG57:6\n" + " └─ LookupJoin\n" + - " ├─ (ltnm.SSHPJ = gn.SSHPJ)\n" + + " ├─ Eq\n" + + " │ ├─ ltnm.SSHPJ:23!null\n" + + " │ └─ gn.SSHPJ:19!null\n" + " ├─ TableAlias(gn)\n" + " │ └─ Table\n" + " │ └─ name: WE72E\n" + " └─ TableAlias(ltnm)\n" + - " └─ IndexedTableAccess(TDRVG)\n" + - " └─ index: [TDRVG.SSHPJ]\n" + + " └─ IndexedTableAccess\n" + + " ├─ index: [TDRVG.SSHPJ]\n" + + " └─ Table\n" + + " └─ name: TDRVG\n" + " )\n" + " └─ Filter\n" + - " ├─ ((NOT(nd.FGG57 IS NULL)) AND nd.KNG7T IS NULL)\n" + + " ├─ AND\n" + + " │ ├─ (NOT(nd.FGG57:6 IS NULL))\n" + + " │ └─ nd.KNG7T:2 IS NULL\n" + " └─ TableAlias(nd)\n" + - " └─ IndexedTableAccess(E2I7U)\n" + + " └─ IndexedTableAccess\n" + " ├─ index: [E2I7U.FGG57]\n" + - " └─ filters: [{(NULL, ∞)}]\n" + + " ├─ static: [{(NULL, ∞)}]\n" + + " └─ Table\n" + + " └─ name: E2I7U\n" + "", }, { @@ -16520,8 +16530,9 @@ FROM ( INNER JOIN D34QP vc ON C6PUD.AZ6SP LIKE CONCAT(CONCAT('%', vc.TWMSR), '%')`, ExpectedPlan: "RowUpdateAccumulator\n" + " └─ Insert(id, Z7CP5, YH4XB)\n" + - " ├─ Table\n" + - " │ └─ name: SEQS3\n" + + " ├─ InsertDestination\n" + + " │ └─ Table\n" + + " │ └─ name: SEQS3\n" + " └─ Project\n" + " ├─ columns: [id:0!null, Z7CP5:1!null, YH4XB:2!null]\n" + " └─ Project\n" + @@ -16664,8 +16675,9 @@ FROM ) BPNW2`, ExpectedPlan: "RowUpdateAccumulator\n" + " └─ Insert(id, FV24E, UJ6XY, M22QN, NZ4MQ, ETPQV, PRUV2, YKSSU, FHCYT)\n" + - " ├─ Table\n" + - " │ └─ name: HDDVB\n" + + " ├─ InsertDestination\n" + + " │ └─ Table\n" + + " │ └─ name: HDDVB\n" + " └─ Project\n" + " ├─ columns: [id:0!null, FV24E:1!null, UJ6XY:2!null, M22QN:3!null, NZ4MQ:4!null, ETPQV:5, PRUV2:6, YKSSU:7, FHCYT:8]\n" + " └─ Union distinct\n" +