Skip to content

Commit 06696a1

Browse files
authored
Merge pull request #2402 from codeaucafe/codeaucafe/2373-fix-subquery-values-projection-aggregate-distinct
#2373 fix VALUES subquery unwrap from dropping clauses
2 parents 39f1f51 + 943e277 commit 06696a1

File tree

3 files changed

+310
-13
lines changed

3 files changed

+310
-13
lines changed

server/analyzer/type_sanitizer.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,14 @@ func TypeSanitizer(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, scope
6363
case *expression.Literal:
6464
// We want to leave limit literals alone, as they are expected to be GMS types when they appear in certain
6565
// 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)
66+
// TODO: fix the limit and offset validation analysis to handle doltgres types
67+
if _, isLimit := n.(*plan.Limit); isLimit {
68+
break
6969
}
70+
if _, isOffset := n.(*plan.Offset); isOffset {
71+
break
72+
}
73+
return typeSanitizerLiterals(ctx, expr)
7074
case *expression.Not, *expression.And, *expression.Or, *expression.Like:
7175
return pgexprs.NewGMSCast(expr), transform.NewTree, nil
7276
case sql.FunctionExpression:

server/ast/aliased_table_expr.go

Lines changed: 23 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,25 @@ 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+
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+
len(s.SelectExprs) != 1 {
163+
return false
164+
}
165+
starExpr, ok := s.SelectExprs[0].(*vitess.StarExpr)
166+
if !ok {
167+
return false
168+
}
169+
return starExpr.TableName.IsEmpty()
170+
}

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)