Skip to content

Commit d7b3469

Browse files
codeaucafecodeaucafe
committed
fix(ast,analyzer): prevent VALUES subquery unwrap from dropping clauses
nodeAliasedTableExpr unconditionally unwrapped any subquery over a VALUES clause when the inner SELECT had a single FROM item. This discarded projections, LIMIT, ORDER BY, DISTINCT, WHERE, GROUP BY, and HAVING because the guard only checked len(inSelect.From) == 1. Replace that check with isTrivialSelectStar, which only permits unwrapping when the inner SELECT is a bare SELECT * with no additional clauses. Non-trivial subqueries now correctly fall through to the vitess.Subquery path, preserving all semantics. Additionally, add a *plan.Offset guard to the TypeSanitizer so OFFSET literals inside subqueries are kept as GMS types, matching the existing *plan.Limit guard. Without this, OFFSET triggered "invalid type: bigint" because GMS validateOffsetAndLimit only recognizes its own type singletons. Tests: - Un-skip three mixed-type VALUES-in-subquery assertions (projection, LIMIT, ORDER BY) - Add mixed-type (int/decimal) variants to all integer-only subquery test groups (DISTINCT, WHERE, GROUP BY, HAVING, column aliasing, combined clauses, JOIN) - Add OFFSET-only and LIMIT+OFFSET subquery assertions for both integer and mixed-type VALUES Refs: #2373
1 parent fd9c0ce commit d7b3469

3 files changed

Lines changed: 320 additions & 15 deletions

File tree

‎server/analyzer/type_sanitizer.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,18 @@ 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 literals alone, as they are expected to be GMS types when they appear in certain
65-
// parts of the query (subqueries in particular)
66-
// TODO: fix the limit validation analysis to handle doltgres types
67-
if _, isLimit := n.(*plan.Limit); !isLimit {
68-
return typeSanitizerLiterals(ctx, expr)
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".
68+
// TODO: fix the limit and offset validation analysis to handle doltgres types
69+
if _, isLimit := n.(*plan.Limit); isLimit {
70+
break
6971
}
72+
if _, isOffset := n.(*plan.Offset); isOffset {
73+
break
74+
}
75+
return typeSanitizerLiterals(ctx, expr)
7076
case *expression.Not, *expression.And, *expression.Or, *expression.Like:
7177
return pgexprs.NewGMSCast(expr), transform.NewTree, nil
7278
case sql.FunctionExpression:

‎server/ast/aliased_table_expr.go‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func nodeAliasedTableExpr(ctx *Context, node *tree.AliasedTableExpr) (*vitess.Al
7171
innerSelect = parentSelect.Select
7272
}
7373
if inSelect, ok := innerSelect.(*vitess.Select); ok {
74-
if len(inSelect.From) == 1 {
74+
if isTrivialSelectStar(inSelect) {
7575
if aliasedTblExpr, ok := inSelect.From[0].(*vitess.AliasedTableExpr); ok {
7676
if valuesStmt, ok := aliasedTblExpr.Expr.(*vitess.ValuesStatement); ok {
7777
if len(node.As.Cols) > 0 {
@@ -146,3 +146,31 @@ func nodeAliasedTableExpr(ctx *Context, node *tree.AliasedTableExpr) (*vitess.Al
146146
Auth: authInfo,
147147
}, nil
148148
}
149+
150+
// isTrivialSelectStar returns true when the Select is just "SELECT * FROM <single table>"
151+
// with no other clauses that would alter semantics (no WHERE, ORDER BY, LIMIT, GROUP BY,
152+
// HAVING, DISTINCT, or WITH).
153+
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 {
169+
return false
170+
}
171+
starExpr, ok := exprs[0].(*vitess.StarExpr)
172+
if !ok {
173+
return false
174+
}
175+
return starExpr.TableName.IsEmpty()
176+
}

‎testing/go/values_statement_test.go‎

Lines changed: 280 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,6 @@ var ValuesStatementTests = []ScriptTest{
189189
Name: "VALUES in subquery",
190190
Assertions: []ScriptTestAssertion{
191191
{
192-
// VALUES as subquery in FROM clause
193-
// TODO: pre-existing bug: arithmetic in subquery over VALUES is not applied (returns original values)
194-
Skip: true,
195192
Query: `SELECT * FROM (SELECT n * 2 AS doubled FROM (VALUES(1),(2.5),(3)) v(n)) sub;`,
196193
Expected: []sql.Row{
197194
{Numeric("2")},
@@ -200,19 +197,13 @@ var ValuesStatementTests = []ScriptTest{
200197
},
201198
},
202199
{
203-
// VALUES with LIMIT inside subquery
204-
// TODO: pre-existing bug: LIMIT inside subquery over VALUES is ignored (returns all rows)
205-
Skip: true,
206200
Query: `SELECT * FROM (SELECT * FROM (VALUES(1),(2.5),(3),(4.5)) v(n) LIMIT 2) sub;`,
207201
Expected: []sql.Row{
208202
{Numeric("1")},
209203
{Numeric("2.5")},
210204
},
211205
},
212206
{
213-
// VALUES with ORDER BY inside subquery
214-
// TODO: pre-existing bug - ORDER BY inside subquery over VALUES is ignored
215-
Skip: true,
216207
Query: `SELECT * FROM (SELECT * FROM (VALUES(3),(1.5),(2)) v(n) ORDER BY n) sub;`,
217208
Expected: []sql.Row{
218209
{Numeric("1.5")},
@@ -570,4 +561,284 @@ var ValuesStatementTests = []ScriptTest{
570561
},
571562
},
572563
},
564+
{
565+
Name: "values inside subquery preserves projections",
566+
Assertions: []ScriptTestAssertion{
567+
{
568+
Query: `SELECT * FROM (SELECT n * 2 AS doubled FROM (VALUES (1), (2), (3)) v(n)) sub;`,
569+
Expected: []sql.Row{
570+
{2},
571+
{4},
572+
{6},
573+
},
574+
},
575+
{
576+
Query: `SELECT * FROM (SELECT n * 2 AS doubled FROM (VALUES (1), (2.5), (3)) v(n)) sub;`,
577+
Expected: []sql.Row{
578+
{Numeric("2")},
579+
{Numeric("5.0")},
580+
{Numeric("6")},
581+
},
582+
},
583+
},
584+
},
585+
{
586+
Name: "values inside subquery preserves LIMIT",
587+
Assertions: []ScriptTestAssertion{
588+
{
589+
Query: `SELECT * FROM (SELECT * FROM (VALUES (1), (2), (3), (4)) v(n) LIMIT 2) sub;`,
590+
Expected: []sql.Row{
591+
{1},
592+
{2},
593+
},
594+
},
595+
{
596+
Query: `SELECT * FROM (SELECT * FROM (VALUES (1), (2.5), (3), (4.5)) v(n) LIMIT 2) sub;`,
597+
Expected: []sql.Row{
598+
{Numeric("1")},
599+
{Numeric("2.5")},
600+
},
601+
},
602+
},
603+
},
604+
{
605+
Name: "values inside subquery preserves ORDER BY",
606+
Assertions: []ScriptTestAssertion{
607+
{
608+
Query: `SELECT * FROM (SELECT * FROM (VALUES (3), (1), (2)) v(n) ORDER BY n) sub;`,
609+
Expected: []sql.Row{
610+
{1},
611+
{2},
612+
{3},
613+
},
614+
},
615+
{
616+
Query: `SELECT * FROM (SELECT * FROM (VALUES (3), (1.5), (2)) v(n) ORDER BY n) sub;`,
617+
Expected: []sql.Row{
618+
{Numeric("1.5")},
619+
{Numeric("2")},
620+
{Numeric("3")},
621+
},
622+
},
623+
},
624+
},
625+
{
626+
Name: "values inside subquery preserves DISTINCT",
627+
Assertions: []ScriptTestAssertion{
628+
{
629+
Query: `SELECT * FROM (SELECT DISTINCT * FROM (VALUES (1), (1), (2), (2), (3)) v(n)) sub;`,
630+
Expected: []sql.Row{
631+
{1},
632+
{2},
633+
{3},
634+
},
635+
},
636+
{
637+
Query: `SELECT * FROM (SELECT DISTINCT * FROM (VALUES (1), (1), (2.5), (2.5), (3)) v(n)) sub;`,
638+
Expected: []sql.Row{
639+
{Numeric("1")},
640+
{Numeric("2.5")},
641+
{Numeric("3")},
642+
},
643+
},
644+
},
645+
},
646+
{
647+
Name: "values inside subquery preserves WHERE",
648+
Assertions: []ScriptTestAssertion{
649+
{
650+
Query: `SELECT * FROM (SELECT * FROM (VALUES (1), (2), (3), (4), (5)) v(n) WHERE n > 3) sub;`,
651+
Expected: []sql.Row{
652+
{4},
653+
{5},
654+
},
655+
},
656+
{
657+
Query: `SELECT * FROM (SELECT * FROM (VALUES (1), (2.5), (3), (4.5), (5)) v(n) WHERE n > 3) sub;`,
658+
Expected: []sql.Row{
659+
{Numeric("4.5")},
660+
{Numeric("5")},
661+
},
662+
},
663+
},
664+
},
665+
{
666+
Name: "values inside subquery preserves OFFSET",
667+
Assertions: []ScriptTestAssertion{
668+
{
669+
Query: `SELECT * FROM (SELECT * FROM (VALUES (10), (20), (30)) v(n) OFFSET 1) sub;`,
670+
Expected: []sql.Row{
671+
{20},
672+
{30},
673+
},
674+
},
675+
{
676+
Query: `SELECT * FROM (SELECT * FROM (VALUES (10), (20), (30), (40), (50)) v(n) LIMIT 2 OFFSET 1) sub;`,
677+
Expected: []sql.Row{
678+
{20},
679+
{30},
680+
},
681+
},
682+
{
683+
Query: `SELECT * FROM (SELECT * FROM (VALUES (10), (20.5), (30)) v(n) OFFSET 1) sub;`,
684+
Expected: []sql.Row{
685+
{Numeric("20.5")},
686+
{Numeric("30")},
687+
},
688+
},
689+
{
690+
Query: `SELECT * FROM (SELECT * FROM (VALUES (10), (20.5), (30), (40.5), (50)) v(n) LIMIT 2 OFFSET 1) sub;`,
691+
Expected: []sql.Row{
692+
{Numeric("20.5")},
693+
{Numeric("30")},
694+
},
695+
},
696+
},
697+
},
698+
{
699+
Name: "values inside subquery preserves ORDER BY with LIMIT",
700+
Assertions: []ScriptTestAssertion{
701+
{
702+
Query: `SELECT * FROM (SELECT * FROM (VALUES (5), (3), (1), (4), (2)) v(n) ORDER BY n LIMIT 3) sub;`,
703+
Expected: []sql.Row{
704+
{1},
705+
{2},
706+
{3},
707+
},
708+
},
709+
{
710+
Query: `SELECT * FROM (SELECT * FROM (VALUES (5), (3.5), (1), (4), (2.5)) v(n) ORDER BY n LIMIT 3) sub;`,
711+
Expected: []sql.Row{
712+
{Numeric("1")},
713+
{Numeric("2.5")},
714+
{Numeric("3.5")},
715+
},
716+
},
717+
},
718+
},
719+
{
720+
Name: "values inside subquery preserves GROUP BY with aggregate",
721+
Assertions: []ScriptTestAssertion{
722+
{
723+
Query: `SELECT * FROM (SELECT n, count(*) AS cnt FROM (VALUES (1), (1), (2), (2), (2), (3)) v(n) GROUP BY n ORDER BY n) sub;`,
724+
Expected: []sql.Row{
725+
{1, int64(2)},
726+
{2, int64(3)},
727+
{3, int64(1)},
728+
},
729+
},
730+
{
731+
Query: `SELECT * FROM (SELECT n, count(*) AS cnt FROM (VALUES (1), (1), (2.5), (2.5), (2.5), (3)) v(n) GROUP BY n ORDER BY n) sub;`,
732+
Expected: []sql.Row{
733+
{Numeric("1"), int64(2)},
734+
{Numeric("2.5"), int64(3)},
735+
{Numeric("3"), int64(1)},
736+
},
737+
},
738+
},
739+
},
740+
{
741+
Name: "values inside subquery preserves HAVING",
742+
Assertions: []ScriptTestAssertion{
743+
{
744+
Query: `SELECT * FROM (SELECT n, count(*) AS cnt FROM (VALUES (1), (1), (2), (2), (2), (3)) v(n) GROUP BY n HAVING count(*) > 1 ORDER BY n) sub;`,
745+
Expected: []sql.Row{
746+
{1, int64(2)},
747+
{2, int64(3)},
748+
},
749+
},
750+
{
751+
Query: `SELECT * FROM (SELECT n, count(*) AS cnt FROM (VALUES (1), (1), (2.5), (2.5), (2.5), (3)) v(n) GROUP BY n HAVING count(*) > 1 ORDER BY n) sub;`,
752+
Expected: []sql.Row{
753+
{Numeric("1"), int64(2)},
754+
{Numeric("2.5"), int64(3)},
755+
},
756+
},
757+
},
758+
},
759+
{
760+
Name: "values inside subquery preserves column aliasing",
761+
Assertions: []ScriptTestAssertion{
762+
{
763+
Query: `SELECT * FROM (SELECT n AS val, n * 10 AS tenfold FROM (VALUES (1), (2), (3)) v(n)) sub;`,
764+
Expected: []sql.Row{
765+
{1, 10},
766+
{2, 20},
767+
{3, 30},
768+
},
769+
},
770+
{
771+
Query: `SELECT * FROM (SELECT n AS val, n * 10 AS tenfold FROM (VALUES (1), (2.5), (3)) v(n)) sub;`,
772+
Expected: []sql.Row{
773+
{Numeric("1"), Numeric("10")},
774+
{Numeric("2.5"), Numeric("25.0")},
775+
{Numeric("3"), Numeric("30")},
776+
},
777+
},
778+
},
779+
},
780+
{
781+
Name: "values inside subquery preserves column subset selection",
782+
Assertions: []ScriptTestAssertion{
783+
{
784+
Query: `SELECT * FROM (SELECT a FROM (VALUES (1, 10), (2, 20), (3, 30)) v(a, b)) sub;`,
785+
Expected: []sql.Row{
786+
{1},
787+
{2},
788+
{3},
789+
},
790+
},
791+
},
792+
},
793+
{
794+
Name: "values subquery trivial SELECT * still unwraps correctly",
795+
Assertions: []ScriptTestAssertion{
796+
{
797+
Query: `SELECT * FROM (SELECT * FROM (VALUES (1), (2), (3)) v(n)) sub;`,
798+
Expected: []sql.Row{
799+
{1},
800+
{2},
801+
{3},
802+
},
803+
},
804+
},
805+
},
806+
{
807+
Name: "values inside subquery with multiple combined clauses",
808+
Assertions: []ScriptTestAssertion{
809+
{
810+
Query: `SELECT * FROM (SELECT DISTINCT n FROM (VALUES (3), (1), (1), (2), (2), (3)) v(n) ORDER BY n LIMIT 2) sub;`,
811+
Expected: []sql.Row{
812+
{1},
813+
{2},
814+
},
815+
},
816+
{
817+
Query: `SELECT * FROM (SELECT DISTINCT n FROM (VALUES (3), (1.5), (1.5), (2), (2), (3)) v(n) ORDER BY n LIMIT 2) sub;`,
818+
Expected: []sql.Row{
819+
{Numeric("1.5")},
820+
{Numeric("2")},
821+
},
822+
},
823+
},
824+
},
825+
{
826+
Name: "values in JOIN preserves inner subquery semantics",
827+
Assertions: []ScriptTestAssertion{
828+
{
829+
Query: `SELECT a.n, b.m FROM (VALUES (1), (2)) a(n) JOIN (SELECT m * 10 AS m FROM (VALUES (1), (2)) v(m)) b ON a.n = b.m / 10;`,
830+
Expected: []sql.Row{
831+
{1, 10},
832+
{2, 20},
833+
},
834+
},
835+
{
836+
Query: `SELECT a.n, b.m FROM (VALUES (1), (2.5)) a(n) JOIN (SELECT m * 10 AS m FROM (VALUES (1), (2.5)) v(m)) b ON a.n = b.m / 10;`,
837+
Expected: []sql.Row{
838+
{Numeric("1"), Numeric("10")},
839+
{Numeric("2.5"), Numeric("25.0")},
840+
},
841+
},
842+
},
843+
},
573844
}

0 commit comments

Comments
 (0)