Skip to content

Commit f40e076

Browse files
authored
fix issue with json unmarshalling of operators with space in them (vitessio#16905)
Signed-off-by: Andres Taylor <andres@planetscale.com>
1 parent 2cef46e commit f40e076

File tree

5 files changed

+224
-161
lines changed

5 files changed

+224
-161
lines changed

go/vt/sqlparser/ast_funcs.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,36 +1562,36 @@ func (op ComparisonExprOperator) ToString() string {
15621562
}
15631563
}
15641564

1565-
func ComparisonExprOperatorFromJson(s string) ComparisonExprOperator {
1565+
func ComparisonExprOperatorFromJson(s string) (ComparisonExprOperator, error) {
15661566
switch s {
15671567
case EqualStr:
1568-
return EqualOp
1568+
return EqualOp, nil
15691569
case JsonLessThanStr:
1570-
return LessThanOp
1570+
return LessThanOp, nil
15711571
case JsonGreaterThanStr:
1572-
return GreaterThanOp
1572+
return GreaterThanOp, nil
15731573
case JsonLessThanOrEqualStr:
1574-
return LessEqualOp
1574+
return LessEqualOp, nil
15751575
case JsonGreaterThanOrEqualStr:
1576-
return GreaterEqualOp
1576+
return GreaterEqualOp, nil
15771577
case NotEqualStr:
1578-
return NotEqualOp
1578+
return NotEqualOp, nil
15791579
case NullSafeEqualStr:
1580-
return NullSafeEqualOp
1580+
return NullSafeEqualOp, nil
15811581
case InStr:
1582-
return InOp
1582+
return InOp, nil
15831583
case NotInStr:
1584-
return NotInOp
1584+
return NotInOp, nil
15851585
case LikeStr:
1586-
return LikeOp
1586+
return LikeOp, nil
15871587
case NotLikeStr:
1588-
return NotLikeOp
1588+
return NotLikeOp, nil
15891589
case RegexpStr:
1590-
return RegexpOp
1590+
return RegexpOp, nil
15911591
case NotRegexpStr:
1592-
return NotRegexpOp
1592+
return NotRegexpOp, nil
15931593
default:
1594-
return 0
1594+
return 0, fmt.Errorf("unknown ComparisonExpOperator: %s", s)
15951595
}
15961596
}
15971597

go/vt/vtgate/executor_vexplain_test.go

Lines changed: 40 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ package vtgate
1818

1919
import (
2020
"context"
21+
"encoding/json"
2122
"fmt"
23+
"os"
24+
"path/filepath"
2225
"testing"
2326

2427
"github.com/stretchr/testify/assert"
@@ -115,153 +118,53 @@ func TestSimpleVexplainTrace(t *testing.T) {
115118
}
116119

117120
func TestVExplainKeys(t *testing.T) {
118-
tests := []struct {
119-
query string
120-
expectedRowString string
121-
}{
122-
{
123-
query: "select count(*), col2 from music group by col2",
124-
expectedRowString: `{
125-
"statementType": "SELECT",
126-
"groupingColumns": [
127-
"music.col2"
128-
],
129-
"selectColumns": [
130-
"music.col2"
131-
]
132-
}`,
133-
}, {
134-
query: "select * from user u join user_extra ue on u.id = ue.user_id where u.col1 > 100 and ue.noLimit = 'foo'",
135-
expectedRowString: `{
136-
"statementType": "SELECT",
137-
"joinColumns": [
138-
"user.id =",
139-
"user_extra.user_id ="
140-
],
141-
"filterColumns": [
142-
"user.col1 gt",
143-
"user_extra.noLimit ="
144-
]
145-
}`,
146-
}, {
147-
// same as above, but written differently
148-
query: "select * from user_extra ue, user u where ue.noLimit = 'foo' and u.col1 > 100 and ue.user_id = u.id",
149-
expectedRowString: `{
150-
"statementType": "SELECT",
151-
"joinColumns": [
152-
"user.id =",
153-
"user_extra.user_id ="
154-
],
155-
"filterColumns": [
156-
"user.col1 gt",
157-
"user_extra.noLimit ="
158-
]
159-
}`,
160-
},
161-
{
162-
query: "select u.foo, ue.bar, count(*) from user u join user_extra ue on u.id = ue.user_id where u.name = 'John Doe' group by 1, 2",
163-
expectedRowString: `{
164-
"statementType": "SELECT",
165-
"groupingColumns": [
166-
"user.foo",
167-
"user_extra.bar"
168-
],
169-
"joinColumns": [
170-
"user.id =",
171-
"user_extra.user_id ="
172-
],
173-
"filterColumns": [
174-
"user.name ="
175-
],
176-
"selectColumns": [
177-
"user.foo",
178-
"user_extra.bar"
179-
]
180-
}`,
181-
},
182-
{
183-
query: "select * from (select * from user) as derived where derived.amount > 1000",
184-
expectedRowString: `{
185-
"statementType": "SELECT"
186-
}`,
187-
},
188-
{
189-
query: "select name, sum(amount) from user group by name",
190-
expectedRowString: `{
191-
"statementType": "SELECT",
192-
"groupingColumns": [
193-
"user.name"
194-
],
195-
"selectColumns": [
196-
"user.amount",
197-
"user.name"
198-
]
199-
}`,
200-
},
201-
{
202-
query: "select name from user where age > 30",
203-
expectedRowString: `{
204-
"statementType": "SELECT",
205-
"filterColumns": [
206-
"user.age gt"
207-
],
208-
"selectColumns": [
209-
"user.name"
210-
]
211-
}`,
212-
},
213-
{
214-
query: "select * from user where name = 'apa' union select * from user_extra where name = 'monkey'",
215-
expectedRowString: `{
216-
"statementType": "SELECT",
217-
"filterColumns": [
218-
"user.name =",
219-
"user_extra.name ="
220-
]
221-
}`,
222-
},
223-
{
224-
query: "update user set name = 'Jane Doe' where id = 1",
225-
expectedRowString: `{
226-
"statementType": "UPDATE",
227-
"filterColumns": [
228-
"user.id ="
229-
]
230-
}`,
231-
},
232-
{
233-
query: "delete from user where order_date < '2023-01-01'",
234-
expectedRowString: `{
235-
"statementType": "DELETE",
236-
"filterColumns": [
237-
"user.order_date lt"
238-
]
239-
}`,
240-
},
241-
{
242-
query: "select * from user where name between 'A' and 'C'",
243-
expectedRowString: `{
244-
"statementType": "SELECT",
245-
"filterColumns": [
246-
"user.name ge",
247-
"user.name le"
248-
]
249-
}`,
250-
},
121+
type testCase struct {
122+
Query string `json:"query"`
123+
Expected json.RawMessage `json:"expected"`
251124
}
252125

126+
var tests []testCase
127+
data, err := os.ReadFile("testdata/executor_vexplain.json")
128+
require.NoError(t, err)
129+
130+
err = json.Unmarshal(data, &tests)
131+
require.NoError(t, err)
132+
133+
var updatedTests []testCase
134+
253135
for _, tt := range tests {
254-
t.Run(tt.query, func(t *testing.T) {
136+
t.Run(tt.Query, func(t *testing.T) {
255137
executor, _, _, _, _ := createExecutorEnv(t)
256138
session := NewSafeSession(&vtgatepb.Session{TargetString: "@primary"})
257-
gotResult, err := executor.Execute(context.Background(), nil, "Execute", session, "vexplain keys "+tt.query, nil)
139+
gotResult, err := executor.Execute(context.Background(), nil, "Execute", session, "vexplain keys "+tt.Query, nil)
258140
require.NoError(t, err)
259141

260142
gotRowString := gotResult.Rows[0][0].ToString()
261-
assert.Equal(t, tt.expectedRowString, gotRowString)
143+
assert.JSONEq(t, string(tt.Expected), gotRowString)
144+
145+
updatedTests = append(updatedTests, testCase{
146+
Query: tt.Query,
147+
Expected: json.RawMessage(gotRowString),
148+
})
149+
262150
if t.Failed() {
263-
fmt.Println(gotRowString)
151+
fmt.Println("Test failed for query:", tt.Query)
152+
fmt.Println("Got result:", gotRowString)
264153
}
265154
})
266155
}
156+
157+
// If anything failed, write the updated test cases to a temp file
158+
if t.Failed() {
159+
tempFilePath := filepath.Join(os.TempDir(), "updated_vexplain_keys_tests.json")
160+
fmt.Println("Writing updated tests to:", tempFilePath)
161+
162+
updatedTestsData, err := json.MarshalIndent(updatedTests, "", "\t")
163+
require.NoError(t, err)
164+
165+
err = os.WriteFile(tempFilePath, updatedTestsData, 0644)
166+
require.NoError(t, err)
167+
168+
fmt.Println("Updated tests written to:", tempFilePath)
169+
}
267170
}

go/vt/vtgate/planbuilder/operators/keys.go

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,37 @@ func (cu *ColumnUse) UnmarshalJSON(data []byte) error {
7777
if err := json.Unmarshal(data, &s); err != nil {
7878
return err
7979
}
80-
parts := strings.Fields(s)
81-
if len(parts) != 2 {
80+
spaceIdx := strings.LastIndex(s, " ")
81+
if spaceIdx == -1 {
8282
return fmt.Errorf("invalid ColumnUse format: %s", s)
8383
}
84-
if err := cu.Column.UnmarshalJSON([]byte(`"` + parts[0] + `"`)); err != nil {
85-
return err
84+
85+
for i := spaceIdx - 1; i >= 0; i-- {
86+
// table.column not like
87+
// table.`tricky not` like
88+
if s[i] == '`' || s[i] == '.' {
89+
break
90+
}
91+
if s[i] == ' ' {
92+
spaceIdx = i
93+
break
94+
}
95+
if i == 0 {
96+
return fmt.Errorf("invalid ColumnUse format: %s", s)
97+
}
98+
}
99+
100+
colStr, opStr := s[:spaceIdx], s[spaceIdx+1:]
101+
102+
err := cu.Column.UnmarshalJSON([]byte(`"` + colStr + `"`))
103+
if err != nil {
104+
return fmt.Errorf("failed to unmarshal column: %w", err)
105+
}
106+
107+
cu.Uses, err = sqlparser.ComparisonExprOperatorFromJson(strings.ToLower(opStr))
108+
if err != nil {
109+
return fmt.Errorf("failed to unmarshal operator: %w", err)
86110
}
87-
cu.Uses = sqlparser.ComparisonExprOperatorFromJson(strings.ToLower(parts[1]))
88111
return nil
89112
}
90113

@@ -209,5 +232,9 @@ func createColumn(ctx *plancontext.PlanningContext, col *sqlparser.ColName) *Col
209232
if table == nil {
210233
return nil
211234
}
212-
return &Column{Table: table.Name.String(), Name: col.Name.String()}
235+
return &Column{
236+
// we want the escaped versions of the names
237+
Table: sqlparser.String(table.Name),
238+
Name: sqlparser.String(col.Name),
239+
}
213240
}

go/vt/vtgate/planbuilder/operators/keys_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,21 @@ func TestMarshalUnmarshal(t *testing.T) {
3232
StatementType: "SELECT",
3333
TableName: []string{"users", "orders"},
3434
GroupingColumns: []Column{
35-
{Table: "", Name: "category"},
35+
{Table: "orders", Name: "category"},
3636
{Table: "users", Name: "department"},
3737
},
3838
JoinColumns: []ColumnUse{
3939
{Column: Column{Table: "users", Name: "id"}, Uses: sqlparser.EqualOp},
4040
{Column: Column{Table: "orders", Name: "user_id"}, Uses: sqlparser.EqualOp},
4141
},
4242
FilterColumns: []ColumnUse{
43-
{Column: Column{Table: "", Name: "age"}, Uses: sqlparser.GreaterThanOp},
43+
{Column: Column{Table: "users", Name: "age"}, Uses: sqlparser.GreaterThanOp},
4444
{Column: Column{Table: "orders", Name: "total"}, Uses: sqlparser.LessThanOp},
45+
{Column: Column{Table: "orders", Name: "`tricky name not`"}, Uses: sqlparser.InOp},
4546
},
4647
SelectColumns: []Column{
4748
{Table: "users", Name: "name"},
48-
{Table: "", Name: "email"},
49+
{Table: "users", Name: "email"},
4950
{Table: "orders", Name: "amount"},
5051
},
5152
}

0 commit comments

Comments
 (0)