Skip to content

Commit 1ce5c4b

Browse files
committed
refactor(ast,analyzer): address PR review comments
Inline isSelectStarOnly into isTrivialSelectStar. And, revert TypeSanitizer comment to match the original, and instead updated to mention both limit and offset literals. Refs: #2373
1 parent d7b3469 commit 1ce5c4b

File tree

2 files changed

+14
-20
lines changed

2 files changed

+14
-20
lines changed

server/analyzer/type_sanitizer.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,8 @@ func TypeSanitizer(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, scope
6161
}
6262
return expr, transform.SameTree, nil
6363
case *expression.Literal:
64-
// We want to leave limit and offset literals as GMS types. GMS's
65-
// validateOffsetAndLimit uses IsInteger, which only recognizes its
66-
// own type singletons. If we convert these to Doltgres types the
67-
// validation rejects them with "invalid type: bigint".
64+
// We want to leave limit literals alone, as they are expected to be GMS types when they appear in certain
65+
// parts of the query (subqueries in particular)
6866
// TODO: fix the limit and offset validation analysis to handle doltgres types
6967
if _, isLimit := n.(*plan.Limit); isLimit {
7068
break

server/ast/aliased_table_expr.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -151,24 +151,20 @@ func nodeAliasedTableExpr(ctx *Context, node *tree.AliasedTableExpr) (*vitess.Al
151151
// with no other clauses that would alter semantics (no WHERE, ORDER BY, LIMIT, GROUP BY,
152152
// HAVING, DISTINCT, or WITH).
153153
func isTrivialSelectStar(s *vitess.Select) bool {
154-
return len(s.From) == 1 &&
155-
!s.QueryOpts.Distinct &&
156-
s.With == nil &&
157-
s.Limit == nil &&
158-
len(s.OrderBy) == 0 &&
159-
s.Where == nil &&
160-
len(s.GroupBy) == 0 &&
161-
s.Having == nil &&
162-
isSelectStarOnly(s.SelectExprs)
163-
}
164-
165-
// isSelectStarOnly returns true when the SelectExprs list is exactly one
166-
// unqualified star expression (i.e. "SELECT *" with no table qualifier).
167-
func isSelectStarOnly(exprs vitess.SelectExprs) bool {
168-
if len(exprs) != 1 {
154+
if len(s.From) != 1 ||
155+
s.QueryOpts.Distinct ||
156+
s.With != nil ||
157+
s.Limit != nil ||
158+
len(s.OrderBy) != 0 ||
159+
s.Where != nil ||
160+
len(s.GroupBy) != 0 ||
161+
s.Having != nil {
162+
return false
163+
}
164+
if len(s.SelectExprs) != 1 {
169165
return false
170166
}
171-
starExpr, ok := exprs[0].(*vitess.StarExpr)
167+
starExpr, ok := s.SelectExprs[0].(*vitess.StarExpr)
172168
if !ok {
173169
return false
174170
}

0 commit comments

Comments
 (0)