Skip to content

Commit 5d6d93c

Browse files
rodrigozhourodrigozhou
authored
Refactor SQL query converter (#3950)
1 parent 099cef3 commit 5d6d93c

File tree

5 files changed

+167
-107
lines changed

5 files changed

+167
-107
lines changed

‎common/persistence/visibility/store/sql/query_converter.go

+21-6
Original file line numberDiff line numberDiff line change
@@ -317,9 +317,10 @@ func (c *QueryConverter) convertComparisonExpr(exprRef *sqlparser.Expr) error {
317317

318318
if !isSupportedComparisonOperator(expr.Operator) {
319319
return query.NewConverterError(
320-
"%s: invalid operator '%s'",
320+
"%s: invalid operator '%s' in `%s`",
321321
query.InvalidExpressionErrMessage,
322322
expr.Operator,
323+
sqlparser.String(expr),
323324
)
324325
}
325326

@@ -400,7 +401,11 @@ func (c *QueryConverter) convertColName(
400401
var err error
401402
saFieldName, err = c.saMapper.GetFieldName(saAlias, c.namespaceName.String())
402403
if err != nil {
403-
return "", "", err
404+
return "", "", query.NewConverterError(
405+
"%s: column name '%s' is not a valid search attribute",
406+
query.InvalidExpressionErrMessage,
407+
saAlias,
408+
)
404409
}
405410
}
406411
if saFieldName == searchattribute.TemporalNamespaceDivision {
@@ -437,7 +442,12 @@ func (c *QueryConverter) convertValueExpr(
437442
*exprRef = sqlparser.NewFloatVal([]byte(strconv.FormatFloat(v, 'f', -1, 64)))
438443
default:
439444
// this should never happen: query.ParseSqlValue returns one of the types above
440-
panic(fmt.Sprintf("Unexpected value type: %T", v))
445+
return query.NewConverterError(
446+
"%s: unexpected value type %T for search attribute %s",
447+
query.InvalidExpressionErrMessage,
448+
v,
449+
saName,
450+
)
441451
}
442452
return nil
443453
case sqlparser.BoolVal:
@@ -472,6 +482,7 @@ func (c *QueryConverter) convertValueExpr(
472482
}
473483

474484
// parseSQLVal handles values for specific search attributes.
485+
// Returns a string, an int64 or a float64 if there are no errors.
475486
// For datetime, converts to UTC.
476487
// For execution status, converts string to enum value.
477488
func (c *QueryConverter) parseSQLVal(
@@ -502,10 +513,14 @@ func (c *QueryConverter) parseSQLVal(
502513
var err error
503514
tm, err = time.Parse(time.RFC3339Nano, v)
504515
if err != nil {
505-
return "", err
516+
return nil, query.NewConverterError(
517+
"%s: unable to parse datetime '%s'",
518+
query.InvalidExpressionErrMessage,
519+
v,
520+
)
506521
}
507522
default:
508-
return "", query.NewConverterError(
523+
return nil, query.NewConverterError(
509524
"%s: unexpected value type %T for search attribute %s",
510525
query.InvalidExpressionErrMessage,
511526
v,
@@ -531,7 +546,7 @@ func (c *QueryConverter) parseSQLVal(
531546
}
532547
status = int64(code)
533548
default:
534-
return "", query.NewConverterError(
549+
return nil, query.NewConverterError(
535550
"%s: unexpected value type %T for search attribute %s",
536551
query.InvalidExpressionErrMessage,
537552
v,

‎common/persistence/visibility/store/sql/query_converter_mysql.go

+40-28
Original file line numberDiff line numberDiff line change
@@ -117,50 +117,58 @@ func (c *mysqlQueryConverter) convertKeywordListComparisonExpr(
117117
expr *sqlparser.ComparisonExpr,
118118
) (sqlparser.Expr, error) {
119119
if !isSupportedKeywordListOperator(expr.Operator) {
120-
return nil, query.NewConverterError("invalid query")
120+
return nil, query.NewConverterError(
121+
"%s: operator %s not supported for KeywordList type search attribute",
122+
query.InvalidExpressionErrMessage,
123+
expr.Operator,
124+
)
121125
}
122126

127+
var negate bool
128+
var newExpr sqlparser.Expr
123129
switch expr.Operator {
124-
case sqlparser.EqualStr:
125-
return &memberOfExpr{
130+
case sqlparser.EqualStr, sqlparser.NotEqualStr:
131+
newExpr = &memberOfExpr{
126132
Value: expr.Right,
127133
JSONArr: expr.Left,
128-
}, nil
129-
case sqlparser.NotEqualStr:
130-
return &sqlparser.NotExpr{
131-
Expr: &memberOfExpr{
132-
Value: expr.Right,
133-
JSONArr: expr.Left,
134-
},
135-
}, nil
136-
case sqlparser.InStr:
137-
return c.convertToJsonOverlapsExpr(expr)
138-
case sqlparser.NotInStr:
139-
jsonOverlapsExpr, err := c.convertToJsonOverlapsExpr(expr)
134+
}
135+
negate = expr.Operator == sqlparser.NotEqualStr
136+
case sqlparser.InStr, sqlparser.NotInStr:
137+
var err error
138+
newExpr, err = c.convertToJsonOverlapsExpr(expr)
140139
if err != nil {
141140
return nil, err
142141
}
143-
return &sqlparser.NotExpr{Expr: jsonOverlapsExpr}, nil
142+
negate = expr.Operator == sqlparser.NotInStr
144143
default:
145144
// this should never happen since isSupportedKeywordListOperator should already fail
146-
return nil, query.NewConverterError("invalid query")
145+
return nil, query.NewConverterError(
146+
"%s: operator %s not supported for KeywordList type search attribute",
147+
query.InvalidExpressionErrMessage,
148+
expr.Operator,
149+
)
150+
}
151+
152+
if negate {
153+
newExpr = &sqlparser.NotExpr{Expr: newExpr}
147154
}
155+
return newExpr, nil
148156
}
149157

150158
func (c *mysqlQueryConverter) convertToJsonOverlapsExpr(
151159
expr *sqlparser.ComparisonExpr,
152160
) (*jsonOverlapsExpr, error) {
153161
valTuple, isValTuple := expr.Right.(sqlparser.ValTuple)
154162
if !isValTuple {
155-
return nil, query.NewConverterError("invalid query")
163+
return nil, query.NewConverterError(
164+
"%s: unexpected value type (expected tuple of strings, got %s)",
165+
query.InvalidExpressionErrMessage,
166+
sqlparser.String(expr.Right),
167+
)
156168
}
157-
values := make([]any, len(valTuple))
158-
for i, val := range valTuple {
159-
value, err := query.ParseSqlValue(sqlparser.String(val))
160-
if err != nil {
161-
return nil, err
162-
}
163-
values[i] = value
169+
values, err := getUnsafeStringTupleValues(valTuple)
170+
if err != nil {
171+
return nil, err
164172
}
165173
jsonValue, err := json.Marshal(values)
166174
if err != nil {
@@ -179,7 +187,11 @@ func (c *mysqlQueryConverter) convertTextComparisonExpr(
179187
expr *sqlparser.ComparisonExpr,
180188
) (sqlparser.Expr, error) {
181189
if !isSupportedTextOperator(expr.Operator) {
182-
return nil, query.NewConverterError("invalid query")
190+
return nil, query.NewConverterError(
191+
"%s: operator %s not supported for Text type search attribute",
192+
query.InvalidExpressionErrMessage,
193+
expr.Operator,
194+
)
183195
}
184196
// build the following expression:
185197
// `match ({expr.Left}) against ({expr.Right} in natural language mode)`
@@ -200,8 +212,8 @@ func (c *mysqlQueryConverter) buildSelectStmt(
200212
pageSize int,
201213
token *pageToken,
202214
) (string, []any) {
203-
whereClauses := make([]string, 0, 3)
204-
queryArgs := make([]any, 0, 8)
215+
var whereClauses []string
216+
var queryArgs []any
205217

206218
whereClauses = append(
207219
whereClauses,

‎common/persistence/visibility/store/sql/query_converter_postgresql.go

+42-30
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,16 @@ func (c *pgQueryConverter) convertKeywordListComparisonExpr(
9595
expr *sqlparser.ComparisonExpr,
9696
) (sqlparser.Expr, error) {
9797
if !isSupportedKeywordListOperator(expr.Operator) {
98-
return nil, query.NewConverterError("invalid query")
98+
return nil, query.NewConverterError(
99+
"%s: operator %s not supported for KeywordList type search attribute",
100+
query.InvalidExpressionErrMessage,
101+
expr.Operator,
102+
)
99103
}
100104

101-
var newExpr sqlparser.Expr
102105
switch expr.Operator {
103106
case sqlparser.EqualStr, sqlparser.NotEqualStr:
104-
newExpr = c.newJsonContainsExpr(expr.Left, expr.Right)
107+
newExpr := c.newJsonContainsExpr(expr.Left, expr.Right)
105108
if expr.Operator == sqlparser.NotEqualStr {
106109
newExpr = &sqlparser.NotExpr{Expr: newExpr}
107110
}
@@ -110,54 +113,63 @@ func (c *pgQueryConverter) convertKeywordListComparisonExpr(
110113
valTupleExpr, isValTuple := expr.Right.(sqlparser.ValTuple)
111114
if !isValTuple {
112115
return nil, query.NewConverterError(
113-
"%s: unexpected value type %T",
116+
"%s: unexpected value type (expected tuple of strings, got %s)",
114117
query.InvalidExpressionErrMessage,
115-
expr,
118+
sqlparser.String(expr.Right),
116119
)
117120
}
118-
newExpr, err := c.convertInExpr(expr.Left, valTupleExpr...)
119-
if err != nil {
120-
return nil, err
121+
var newExpr sqlparser.Expr = &sqlparser.ParenExpr{
122+
Expr: c.convertInExpr(expr.Left, valTupleExpr),
121123
}
122-
newExpr = &sqlparser.ParenExpr{Expr: newExpr}
123124
if expr.Operator == sqlparser.NotInStr {
124125
newExpr = &sqlparser.NotExpr{Expr: newExpr}
125126
}
126127
return newExpr, nil
127128
default:
128129
// this should never happen since isSupportedKeywordListOperator should already fail
129-
return nil, query.NewConverterError("invalid query")
130+
return nil, query.NewConverterError(
131+
"%s: operator %s not supported for KeywordList type search attribute",
132+
query.InvalidExpressionErrMessage,
133+
expr.Operator,
134+
)
130135
}
131136
}
132137

133138
func (c *pgQueryConverter) convertInExpr(
134139
leftExpr sqlparser.Expr,
135-
values ...sqlparser.Expr,
136-
) (sqlparser.Expr, error) {
137-
if len(values) == 0 {
138-
return nil, nil
139-
}
140-
141-
newExpr := c.newJsonContainsExpr(leftExpr, values[0])
142-
if len(values) == 1 {
143-
return newExpr, nil
140+
values sqlparser.ValTuple,
141+
) sqlparser.Expr {
142+
exprs := make([]sqlparser.Expr, len(values))
143+
for i, value := range values {
144+
exprs[i] = c.newJsonContainsExpr(leftExpr, value)
144145
}
145-
146-
orRightExpr, err := c.convertInExpr(leftExpr, values[1:]...)
147-
if err != nil {
148-
return nil, err
146+
for len(exprs) > 1 {
147+
k := 0
148+
for i := 0; i < len(exprs); i += 2 {
149+
if i+1 < len(exprs) {
150+
exprs[k] = &sqlparser.OrExpr{
151+
Left: exprs[i],
152+
Right: exprs[i+1],
153+
}
154+
} else {
155+
exprs[k] = exprs[i]
156+
}
157+
k++
158+
}
159+
exprs = exprs[:k]
149160
}
150-
return &sqlparser.OrExpr{
151-
Left: newExpr,
152-
Right: orRightExpr,
153-
}, nil
161+
return exprs[0]
154162
}
155163

156164
func (c *pgQueryConverter) convertTextComparisonExpr(
157165
expr *sqlparser.ComparisonExpr,
158166
) (sqlparser.Expr, error) {
159167
if !isSupportedTextOperator(expr.Operator) {
160-
return nil, query.NewConverterError("invalid query")
168+
return nil, query.NewConverterError(
169+
"%s: operator %s not supported for Text type search attribute",
170+
query.InvalidExpressionErrMessage,
171+
expr.Operator,
172+
)
161173
}
162174
valueExpr, ok := expr.Right.(*unsafeSQLString)
163175
if !ok {
@@ -207,8 +219,8 @@ func (c *pgQueryConverter) buildSelectStmt(
207219
pageSize int,
208220
token *pageToken,
209221
) (string, []any) {
210-
whereClauses := make([]string, 0, 3)
211-
queryArgs := make([]any, 0, 8)
222+
var whereClauses []string
223+
var queryArgs []any
212224

213225
whereClauses = append(
214226
whereClauses,

0 commit comments

Comments
 (0)