Skip to content

Commit efb36fa

Browse files
authored
Merge pull request #898 from dolthub/zachmu/enginetests2
more unskipped engine tests
2 parents af82daa + 6466d7b commit efb36fa

File tree

7 files changed

+187
-19
lines changed

7 files changed

+187
-19
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ require (
1212
github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi v0.0.0-20240827111219-e4bb9ca3442d
1313
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
1414
github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662
15-
github.com/dolthub/go-mysql-server v0.18.2-0.20241028201017-2c6e3dd1d10b
15+
github.com/dolthub/go-mysql-server v0.18.2-0.20241028220705-fc9e96ed4c1d
1616
github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216
17-
github.com/dolthub/vitess v0.0.0-20241016191424-d14e107a654e
17+
github.com/dolthub/vitess v0.0.0-20241028204000-267861bc75a0
1818
github.com/fatih/color v1.13.0
1919
github.com/goccy/go-json v0.10.2
2020
github.com/gogo/protobuf v1.3.2

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
224224
github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
225225
github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662 h1:aC17hZD6iwzBwwfO5M+3oBT5E5gGRiQPdn+vzpDXqIA=
226226
github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
227-
github.com/dolthub/go-mysql-server v0.18.2-0.20241028201017-2c6e3dd1d10b h1:3qUuxazPaUkIx8uEW44ZEifCiH3aFodLw7KeTpl3zqI=
228-
github.com/dolthub/go-mysql-server v0.18.2-0.20241028201017-2c6e3dd1d10b/go.mod h1:z/GGuH2asedC+lkJA4sx+C3oyRH1HRx8ET6N9AGBVms=
227+
github.com/dolthub/go-mysql-server v0.18.2-0.20241028220705-fc9e96ed4c1d h1:FLs7/W5OmRnp/UPRw5PEa+PrcvtVk5ZV+C9RCQ78CnE=
228+
github.com/dolthub/go-mysql-server v0.18.2-0.20241028220705-fc9e96ed4c1d/go.mod h1:jlzVUA+tsjDw6YKbhRsCLHT3OVO6nn4BWrUanECTo3s=
229229
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
230230
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
231231
github.com/dolthub/ishell v0.0.0-20240701202509-2b217167d718 h1:lT7hE5k+0nkBdj/1UOSFwjWpNxf+LCApbRHgnCA17XE=
@@ -238,8 +238,8 @@ github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216 h1:JWkKRE4
238238
github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216/go.mod h1:e/FIZVvT2IR53HBCAo41NjqgtEnjMJGKca3Y/dAmZaA=
239239
github.com/dolthub/swiss v0.1.0 h1:EaGQct3AqeP/MjASHLiH6i4TAmgbG/c4rA6a1bzCOPc=
240240
github.com/dolthub/swiss v0.1.0/go.mod h1:BeucyB08Vb1G9tumVN3Vp/pyY4AMUnr9p7Rz7wJ7kAQ=
241-
github.com/dolthub/vitess v0.0.0-20241016191424-d14e107a654e h1:Ssd/iV0hAOShAgr0c4pJQNgh2E4my2XHblFIIam0D+4=
242-
github.com/dolthub/vitess v0.0.0-20241016191424-d14e107a654e/go.mod h1:uBvlRluuL+SbEWTCZ68o0xvsdYZER3CEG/35INdzfJM=
241+
github.com/dolthub/vitess v0.0.0-20241028204000-267861bc75a0 h1:eeKypNsi1nQmjWxSAAWT6tvRsDWdmll03BozAUUIE4E=
242+
github.com/dolthub/vitess v0.0.0-20241028204000-267861bc75a0/go.mod h1:uBvlRluuL+SbEWTCZ68o0xvsdYZER3CEG/35INdzfJM=
243243
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
244244
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
245245
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=

server/ast/group_by.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ package ast
1717
import (
1818
vitess "github.com/dolthub/vitess/go/vt/sqlparser"
1919

20+
pgexprs "github.com/dolthub/doltgresql/server/expression"
21+
2022
"github.com/dolthub/doltgresql/postgres/parser/sem/tree"
2123
)
2224

@@ -25,6 +27,25 @@ func nodeGroupBy(node tree.GroupBy) (vitess.GroupBy, error) {
2527
if len(node) == 0 {
2628
return nil, nil
2729
}
28-
exprs, err := nodeExprs(tree.Exprs(node))
29-
return vitess.GroupBy(exprs), err
30+
31+
groupBys := make(vitess.GroupBy, len(node))
32+
var err error
33+
for i, expr := range node {
34+
groupBys[i], err = nodeExpr(expr)
35+
if err != nil {
36+
return nil, err
37+
}
38+
39+
// GMS order by is hardcoded to expect vitess.SQLVal for expressions such as `ORDER BY 1`.
40+
// In addition, there is the requirement that columns in the order by also need to be referenced somewhere in
41+
// the query, which is not a requirement for Postgres. Whenever we add that functionality, we also need to
42+
// remove the dependency on vitess.SQLVal. For now, we'll just convert our literals to a vitess.SQLVal.
43+
if injectedExpr, ok := groupBys[i].(vitess.InjectedExpr); ok {
44+
if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok {
45+
groupBys[i] = literal.ToVitessLiteral()
46+
}
47+
}
48+
}
49+
50+
return groupBys, nil
3051
}

testing/go/enginetest/doltgres_engine_test.go

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,34 @@ func TestSingleScript(t *testing.T) {
148148

149149
var scripts = []queries.ScriptTest{
150150
{
151-
Name: "dolt_revert() detects not null violation (issue #4527)",
151+
Name: "Group by BINARY: https://github.com/dolthub/dolt/issues/6179",
152152
SetUpScript: []string{
153-
"create table test2 (pk int primary key, c0 int)",
154-
"alter table test2 modify c0 int not null",
153+
"create table t (s varchar(100));",
154+
"insert into t values ('abc'), ('def');",
155+
"create table t1 (b binary(3));",
156+
"insert into t1 values ('abc'), ('abc'), ('def'), ('abc'), ('def');",
155157
},
156158
Assertions: []queries.ScriptTestAssertion{
157159
{
158-
Query: "call dolt_revert('head~1');",
159-
ExpectedErrStr: "revert currently does not handle constraint violations",
160+
Query: "select binary s from t group by binary s order by binary s",
161+
Expected: []sql.Row{
162+
{[]uint8("abc")},
163+
{[]uint8("def")},
164+
},
165+
},
166+
{
167+
Query: "select count(b), b from t1 group by b order by b",
168+
Expected: []sql.Row{
169+
{3, []uint8("abc")},
170+
{2, []uint8("def")},
171+
},
172+
},
173+
{
174+
Query: "select binary s from t group by binary s order by s",
175+
Expected: []sql.Row{
176+
{[]uint8("abc")},
177+
{[]uint8("def")},
178+
},
160179
},
161180
},
162181
},
@@ -245,15 +264,23 @@ func TestInfoSchema(t *testing.T) {
245264
}
246265

247266
func TestColumnAliases(t *testing.T) {
248-
t.Skip()
249-
h := newDoltgresServerHarness(t)
267+
h := newDoltgresServerHarness(t).WithSkippedQueries([]string{
268+
"SELECT s as coL1, SUM(i) coL2 FROM mytable group by 1 order by 2", // incorrect result
269+
"SELECT s as Date, SUM(i) TimeStamp FROM mytable group by 1 order by 2", // ERROR: at or near "timestamp": syntax error
270+
"select \"foo\" as dummy, (select dummy)", // Unhandled OID 705
271+
"SELECT 1 as a, (select a) as b from dual", // table not found: dual
272+
})
250273
defer h.Close()
251274
enginetest.TestColumnAliases(t, h)
252275
}
253276

254277
func TestOrderByGroupBy(t *testing.T) {
255-
t.Skip()
256-
h := newDoltgresServerHarness(t)
278+
h := newDoltgresServerHarness(t).WithSkippedQueries([]string{
279+
"Group by with decimal columns", // syntax error
280+
"Validation for use of non-aggregated columns with implicit grouping of all rows", // bad error matching
281+
"group by with any_value()", // @@ vars not supported
282+
"group by with strict errors", // @@ vars not supported
283+
})
257284
defer h.Close()
258285
enginetest.TestOrderByGroupBy(t, h)
259286
}

testing/go/enginetest/doltgres_harness_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/dolthub/go-mysql-server/sql"
3535
"github.com/dolthub/go-mysql-server/sql/analyzer"
3636
gmstypes "github.com/dolthub/go-mysql-server/sql/types"
37+
"github.com/dolthub/vitess/go/sqltypes"
3738
vitess "github.com/dolthub/vitess/go/vt/sqlparser"
3839
_ "github.com/jackc/pgx/v4/stdlib"
3940
"github.com/jackc/pgx/v5"
@@ -848,8 +849,12 @@ func columns(rows pgx.Rows) (sql.Schema, []interface{}, error) {
848849
colVal := gosql.NullTime{}
849850
columnVals = append(columnVals, &colVal)
850851
schema = append(schema, &sql.Column{Name: field.Name, Type: gmstypes.Timestamp, Nullable: true})
852+
case uint32(oid.T_bytea):
853+
colVal := gosql.NullString{}
854+
columnVals = append(columnVals, &colVal)
855+
schema = append(schema, &sql.Column{Name: field.Name, Type: gmstypes.MustCreateBinary(sqltypes.Binary, 100), Nullable: true})
851856
case uint32(oid.T_json):
852-
colVal := gosql.NullByte{}
857+
colVal := gosql.NullString{}
853858
columnVals = append(columnVals, &colVal)
854859
schema = append(schema, &sql.Column{Name: field.Name, Type: gmstypes.JSON, Nullable: true})
855860
default:

testing/go/enginetest/query_converter_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,46 @@ func convertDdlStatement(statement *sqlparser.DDL) ([]string, bool) {
135135
// transformSelect converts a MySQL SELECT statement to a postgres-compatible SELECT statement.
136136
// This is a very broad surface area, so we do this very selectively
137137
func transformSelect(stmt *sqlparser.Select) ([]string, bool) {
138-
if !containsUserVars(stmt) {
138+
if !shouldRewriteSelect(stmt) {
139139
return nil, false
140140
}
141141
return []string{formatNode(stmt)}, true
142142
}
143143

144+
func shouldRewriteSelect(stmt *sqlparser.Select) bool {
145+
return containsUserVars(stmt) ||
146+
containsBinaryConversion(stmt)
147+
}
148+
149+
func containsBinaryConversion(stmt *sqlparser.Select) bool {
150+
foundBinaryConversionExpr := false
151+
fn := func(node sqlparser.SQLNode) (bool, error) {
152+
switch node := node.(type) {
153+
case *sqlparser.BinaryExpr:
154+
if node.Operator == "binary " {
155+
foundBinaryConversionExpr = true
156+
return false, nil
157+
}
158+
case *sqlparser.UnaryExpr:
159+
if node.Operator == "binary " {
160+
foundBinaryConversionExpr = true
161+
return false, nil
162+
}
163+
}
164+
return true, nil
165+
}
166+
167+
for _, sel := range stmt.SelectExprs {
168+
sqlparser.Walk(fn, sel)
169+
}
170+
171+
if stmt.Where != nil {
172+
sqlparser.Walk(fn, stmt.Where)
173+
}
174+
175+
return foundBinaryConversionExpr
176+
}
177+
144178
func containsUserVars(stmt *sqlparser.Select) bool {
145179
foundUserVar := false
146180
detectUserVar := func(node sqlparser.SQLNode) (bool, error) {
@@ -213,6 +247,12 @@ func PostgresNodeFormatter(buf *sqlparser.TrackedBuffer, node sqlparser.SQLNode)
213247
} else {
214248
buf.Myprintf("%s", node.Lowered())
215249
}
250+
case *sqlparser.UnaryExpr:
251+
if node.Operator == "binary " {
252+
buf.Myprintf("%v::text::bytea", node.Expr)
253+
} else {
254+
buf.Myprintf("%v", node)
255+
}
216256
case *sqlparser.Limit:
217257
if node == nil {
218258
return

testing/go/groupby_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright 2024 Dolthub, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package _go
16+
17+
import (
18+
"testing"
19+
20+
"github.com/dolthub/go-mysql-server/sql"
21+
)
22+
23+
func TestGroupBy(t *testing.T) {
24+
RunScripts(t, []ScriptTest{
25+
{
26+
Name: "Basic order by/group by cases",
27+
SetUpScript: []string{
28+
"create table members (id bigint primary key, team text);",
29+
"insert into members values (3,'red'), (4,'red'),(5,'orange'),(6,'orange'),(7,'orange'),(8,'purple');",
30+
},
31+
Assertions: []ScriptTestAssertion{
32+
{
33+
Query: "select team as f from members order by id, f",
34+
Expected: []sql.Row{{"red"}, {"red"}, {"orange"}, {"orange"}, {"orange"}, {"purple"}},
35+
},
36+
{
37+
Query: "SELECT team, COUNT(*) FROM members GROUP BY team ORDER BY 2",
38+
Expected: []sql.Row{
39+
{"purple", int64(1)},
40+
{"red", int64(2)},
41+
{"orange", int64(3)},
42+
},
43+
},
44+
{
45+
Query: "SELECT team, COUNT(*) FROM members GROUP BY 1 ORDER BY 2",
46+
Expected: []sql.Row{
47+
{"purple", int64(1)},
48+
{"red", int64(2)},
49+
{"orange", int64(3)},
50+
},
51+
},
52+
{
53+
Query: "SELECT team, COUNT(*) FROM members GROUP BY team ORDER BY columndoesnotexist",
54+
ExpectedErr: "not be found",
55+
},
56+
{
57+
Query: "SELECT DISTINCT t1.id as id FROM members AS t1 JOIN members AS t2 ON t1.id = t2.id WHERE t2.id > 0 ORDER BY t1.id",
58+
Expected: []sql.Row{{3}, {4}, {5}, {6}, {7}, {8}},
59+
},
60+
{
61+
Query: "SELECT id as alias1, (SELECT alias1+1 group by alias1 having alias1 > 0) FROM members where id < 6;",
62+
Expected: []sql.Row{{3, 4}, {4, 5}, {5, 6}},
63+
},
64+
{
65+
Query: "SELECT id, (SELECT UPPER(team) having id > 3) as upper_team FROM members where id < 6;",
66+
Expected: []sql.Row{{3, nil}, {4, "RED"}, {5, "ORANGE"}},
67+
},
68+
{
69+
Query: "SELECT id, (SELECT -1 as id having id < 10) as upper_team FROM members where id < 6;",
70+
Expected: []sql.Row{{3, -1}, {4, -1}, {5, -1}},
71+
},
72+
},
73+
},
74+
})
75+
}

0 commit comments

Comments
 (0)