Merge pull request #1693 from dolthub/steph/dolt-5570

update separator parsing in group_concat
dolthub · Apr 4, 2023 · 0ba5b35 · 0ba5b35
2 parents dddc9ed + 0537c82
commit 0ba5b35
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 12 deletions.
diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go
@@ -1130,6 +1130,10 @@ var ScriptTests = []ScriptTest{
 				Query:    "SELECT group_concat(o_id) FROM t WHERE `attribute`='color' order by o_id",
 				Expected: []sql.Row{{"2,3"}},
 			},
+			{
+				Query:    "SELECT group_concat(attribute separator '') FROM t WHERE o_id=2 ORDER BY attribute",
+				Expected: []sql.Row{{"colorfabric"}},
+			},
 		},
 	},
 	{

diff --git a/go.mod b/go.mod
@@ -3,7 +3,7 @@ module github.com/dolthub/go-mysql-server
 require (
 	github.com/cespare/xxhash v1.1.0
 	github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
-	github.com/dolthub/vitess v0.0.0-20230403203406-3b7bf02e68b3
+	github.com/dolthub/vitess v0.0.0-20230403222318-aa590a202153
 	github.com/go-kit/kit v0.10.0
 	github.com/go-sql-driver/mysql v1.6.0
 	github.com/gocraft/dbr/v2 v2.7.2

diff --git a/go.sum b/go.sum
@@ -55,10 +55,10 @@ github.com/dolthub/jsonpath v0.0.0-20210609232853-d49537a30474 h1:xTrR+l5l+1Lfq0
 github.com/dolthub/jsonpath v0.0.0-20210609232853-d49537a30474/go.mod h1:kMz7uXOXq4qRriCEyZ/LUeTqraLJCjf0WVZcUi6TxUY=
 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9XGFa6q5Ap4Z/OhNkAMBaK5YeuEzwJt+NZdhiE=
 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
-github.com/dolthub/vitess v0.0.0-20230329002110-9cebb0262ead h1:3enLIBBgBsIOrz/SR4WTjT3XLpwhMxkZKdl608JPRUM=
-github.com/dolthub/vitess v0.0.0-20230329002110-9cebb0262ead/go.mod h1:oVFIBdqMFEkt4Xz2fzFJBNtzKhDEjwdCF0dzde39iKs=
-github.com/dolthub/vitess v0.0.0-20230403203406-3b7bf02e68b3 h1:inZIwt5Jv5X2/tblBMtmyFP6kw/b/8R7BSwuaVEAxKQ=
-github.com/dolthub/vitess v0.0.0-20230403203406-3b7bf02e68b3/go.mod h1:oVFIBdqMFEkt4Xz2fzFJBNtzKhDEjwdCF0dzde39iKs=
+github.com/dolthub/vitess v0.0.0-20230403205610-0c8d4c5aa5a2 h1:l6EfOQNPQcOQ0yaf0NT/NmARfdJB7vQAsylmX56Xnz8=
+github.com/dolthub/vitess v0.0.0-20230403205610-0c8d4c5aa5a2/go.mod h1:oVFIBdqMFEkt4Xz2fzFJBNtzKhDEjwdCF0dzde39iKs=
+github.com/dolthub/vitess v0.0.0-20230403222318-aa590a202153 h1:pYtxt69Tc6Q8VMmDuKPhbKoS/fdtcwEh0nK6ZLUE25g=
+github.com/dolthub/vitess v0.0.0-20230403222318-aa590a202153/go.mod h1:oVFIBdqMFEkt4Xz2fzFJBNtzKhDEjwdCF0dzde39iKs=
 github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs=
 github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU=

diff --git a/sql/expression/function/aggregation/group_concat.go b/sql/expression/function/aggregation/group_concat.go
@@ -132,10 +132,8 @@ func (g *GroupConcat) String() string {
 		}
 	}
 
-	if g.separator != "," {
-		sb.WriteString(" separator ")
-		sb.WriteString(fmt.Sprintf("'%s'", g.separator))
-	}
+	sb.WriteString(" separator ")
+	sb.WriteString(fmt.Sprintf("'%s'", g.separator))
 
 	sb.WriteString(")")
 

diff --git a/sql/expression/function/aggregation/group_concat_test.go b/sql/expression/function/aggregation/group_concat_test.go
@@ -31,7 +31,7 @@ func TestGroupConcat_FunctionName(t *testing.T) {
 	m, err := NewGroupConcat("field", nil, ",", nil, 1024)
 	require.NoError(t, err)
 
-	assert.Equal("group_concat(distinct field)", m.String())
+	assert.Equal("group_concat(distinct field separator ',')", m.String())
 
 	m, err = NewGroupConcat("field", nil, "-", nil, 1024)
 	require.NoError(t, err)

diff --git a/sql/parse/parse.go b/sql/parse/parse.go
@@ -3569,8 +3569,8 @@ func ExprToExpression(ctx *sql.Context, e sqlparser.Expr) (sql.Expression, error
 		}
 
 		separatorS := ","
-		if v.Separator != "" {
-			separatorS = v.Separator
+		if !v.Separator.DefaultSeparator {
+			separatorS = v.Separator.SeparatorString
 		}
 
 		sortFields, err := orderByToSortFields(ctx, v.OrderBy)