[FIXED] WITH clause without a RETURNING clause will panic [#177]

* Removed check for returning columns when using insert/update/delete in sub select
doug-martin · Dec 7, 2019 · 864162a · 864162a
1 parent 096254c
commit 864162a
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 28 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -1,9 +1,10 @@
 # v9.5.1
 
+* [FIXED] WITH clause without a RETURNING clause will panic [#177](https://github.com/doug-martin/goqu/issues/177)
+* [FIXED] SQlite dialect escapes single quotes wrong, leads to SQL syntax error [#178](https://github.com/doug-martin/goqu/issues/178)
 * [FIXED] Fix ReturnsColumns() nil pointer panic [#181](https://github.com/doug-martin/goqu/issues/181) - [@yeaha](https://github.com/yeaha)
 * [FIXED] SelectDataset From with Error [#183](https://github.com/doug-martin/goqu/issues/183)
 * [FIXED] Unable to execute union with order by expression [#185](https://github.com/doug-martin/goqu/issues/185)
-* [FIXED] SQlite dialect escapes single quotes wrong, leads to SQL syntax error [#178](https://github.com/doug-martin/goqu/issues/178)
 
 # v9.5.0
 

diff --git a/issues_test.go b/issues_test.go
@@ -328,6 +328,44 @@ func (gis *githubIssuesSuite) TestIssue164() {
 	)
 }
 
+// Test for https://github.com/doug-martin/goqu/issues/177
+func (gis *githubIssuesSuite) TestIssue177() {
+	ds := goqu.Dialect("postgres").
+		From("ins1").
+		With("ins1",
+			goqu.Dialect("postgres").
+				Insert("account").
+				Rows(goqu.Record{"email": "email@email.com", "status": "active", "uuid": "XXX-XXX-XXXX"}).
+				Returning("*"),
+		).
+		With("ins2",
+			goqu.Dialect("postgres").
+				Insert("account_user").
+				Cols("account_id", "user_id").
+				FromQuery(goqu.Dialect("postgres").
+					From("ins1").
+					Select(
+						"id",
+						goqu.V(1001),
+					),
+				),
+		).
+		Select("*")
+	sql, args, err := ds.ToSQL()
+	gis.NoError(err)
+	gis.Equal(`WITH ins1 AS (`+
+		`INSERT INTO "account" ("email", "status", "uuid") VALUES ('email@email.com', 'active', 'XXX-XXX-XXXX') RETURNING *),`+
+		` ins2 AS (INSERT INTO "account_user" ("account_id", "user_id") SELECT "id", 1001 FROM "ins1")`+
+		` SELECT * FROM "ins1"`, sql)
+	gis.Len(args, 0)
+
+	sql, args, err = ds.Prepared(true).ToSQL()
+	gis.NoError(err)
+	gis.Equal(`WITH ins1 AS (INSERT INTO "account" ("email", "status", "uuid") VALUES ($1, $2, $3) RETURNING *), ins2`+
+		` AS (INSERT INTO "account_user" ("account_id", "user_id") SELECT "id", $4 FROM "ins1") SELECT * FROM "ins1"`, sql)
+	gis.Equal(args, []interface{}{"email@email.com", "active", "XXX-XXX-XXXX", int64(1001)})
+}
+
 // Test for https://github.com/doug-martin/goqu/issues/183
 func (gis *githubIssuesSuite) TestIssue184() {
 	expectedErr := fmt.Errorf("an error")

diff --git a/sqlgen/delete_sql_generator_test.go b/sqlgen/delete_sql_generator_test.go
@@ -109,7 +109,7 @@ func (dsgs *deleteSQLGeneratorSuite) TestGenerate_withCommonTables() {
 	opts.WithFragment = []byte("with ")
 	opts.RecursiveFragment = []byte("recursive ")
 
-	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil, true)
+	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil)
 
 	dc := exp.NewDeleteClauses().SetFrom(exp.NewIdentifierExpression("", "test_cte", ""))
 	dcCte1 := dc.CommonTablesAppend(exp.NewCommonTableExpression(false, "test_cte", tse))

diff --git a/sqlgen/expression_sql_generator.go b/sqlgen/expression_sql_generator.go
@@ -194,10 +194,6 @@ func (esg *expressionSQLGenerator) placeHolderSQL(b sb.SQLBuilder, i interface{}
 
 // Generates creates the sql for a sub select on a Dataset
 func (esg *expressionSQLGenerator) appendableExpressionSQL(b sb.SQLBuilder, a exp.AppendableExpression) {
-	if !a.ReturnsColumns() {
-		b.SetError(errNoReturnColumnsForAppendableExpression)
-		return
-	}
 	b.WriteRunes(esg.dialectOptions.LeftParenRune)
 	a.AppendSQL(b)
 	b.WriteRunes(esg.dialectOptions.RightParenRune)

diff --git a/sqlgen/expression_sql_generator_test.go b/sqlgen/expression_sql_generator_test.go
@@ -27,9 +27,8 @@ func newTestAppendableExpression(
 	sql string,
 	args []interface{},
 	err error,
-	alias exp.IdentifierExpression,
-	returnsColumns bool) exp.AppendableExpression {
-	return &testAppendableExpression{sql: sql, args: args, err: err, alias: alias, returnsColumns: returnsColumns}
+	alias exp.IdentifierExpression) exp.AppendableExpression {
+	return &testAppendableExpression{sql: sql, args: args, err: err, alias: alias}
 }
 
 func (tae *testAppendableExpression) Expression() exp.Expression {
@@ -350,12 +349,10 @@ func (esgs *expressionSQLGeneratorSuite) TestGenerateUnsupportedExpression() {
 
 func (esgs *expressionSQLGeneratorSuite) TestGenerate_AppendableExpression() {
 	ti := exp.NewIdentifierExpression("", "b", "")
-	a := newTestAppendableExpression(`select * from "a"`, []interface{}{}, nil, nil, true)
-	aliasedA := newTestAppendableExpression(`select * from "a"`, []interface{}{}, nil, ti, true)
-	argsA := newTestAppendableExpression(`select * from "a" where x=?`, []interface{}{true}, nil, ti, true)
-	ae := newTestAppendableExpression(`select * from "a"`, emptyArgs, errors.New("expected error"), nil, true)
-
-	aenr := newTestAppendableExpression(`update "foo" set "a"='b'`, emptyArgs, nil, nil, false)
+	a := newTestAppendableExpression(`select * from "a"`, []interface{}{}, nil, nil)
+	aliasedA := newTestAppendableExpression(`select * from "a"`, []interface{}{}, nil, ti)
+	argsA := newTestAppendableExpression(`select * from "a" where x=?`, []interface{}{true}, nil, ti)
+	ae := newTestAppendableExpression(`select * from "a"`, emptyArgs, errors.New("expected error"), nil)
 
 	esgs.assertCases(
 		NewExpressionSQLGenerator("test", DefaultDialectOptions()),
@@ -368,9 +365,6 @@ func (esgs *expressionSQLGeneratorSuite) TestGenerate_AppendableExpression() {
 		expressionTestCase{val: ae, err: "goqu: expected error"},
 		expressionTestCase{val: ae, err: "goqu: expected error", isPrepared: true},
 
-		expressionTestCase{val: aenr, err: errNoReturnColumnsForAppendableExpression.Error()},
-		expressionTestCase{val: aenr, err: errNoReturnColumnsForAppendableExpression.Error(), isPrepared: true},
-
 		expressionTestCase{val: argsA, sql: `(select * from "a" where x=?) AS "b"`, args: []interface{}{true}},
 		expressionTestCase{val: argsA, sql: `(select * from "a" where x=?) AS "b"`, isPrepared: true, args: []interface{}{true}},
 	)
@@ -475,7 +469,7 @@ func (esgs *expressionSQLGeneratorSuite) TestGenerate_AliasedExpression() {
 }
 
 func (esgs *expressionSQLGeneratorSuite) TestGenerate_BooleanExpression() {
-	ae := newTestAppendableExpression(`SELECT "id" FROM "test2"`, emptyArgs, nil, nil, true)
+	ae := newTestAppendableExpression(`SELECT "id" FROM "test2"`, emptyArgs, nil, nil)
 	re := regexp.MustCompile("(a|b)")
 	ident := exp.NewIdentifierExpression("", "", "a")
 
@@ -862,7 +856,7 @@ func (esgs *expressionSQLGeneratorSuite) TestGenerate_CastExpression() {
 
 // Generates the sql for the WITH clauses for common table expressions (CTE)
 func (esgs *expressionSQLGeneratorSuite) TestGenerate_CommonTableExpressionSlice() {
-	ae := newTestAppendableExpression(`SELECT * FROM "b"`, emptyArgs, nil, nil, true)
+	ae := newTestAppendableExpression(`SELECT * FROM "b"`, emptyArgs, nil, nil)
 
 	cteNoArgs := []exp.CommonTableExpression{
 		exp.NewCommonTableExpression(false, "a", ae),
@@ -961,7 +955,7 @@ func (esgs *expressionSQLGeneratorSuite) TestGenerate_CommonTableExpressionSlice
 }
 
 func (esgs *expressionSQLGeneratorSuite) TestGenerate_CommonTableExpression() {
-	ae := newTestAppendableExpression(`SELECT * FROM "b"`, emptyArgs, nil, nil, true)
+	ae := newTestAppendableExpression(`SELECT * FROM "b"`, emptyArgs, nil, nil)
 
 	cteNoArgs := exp.NewCommonTableExpression(false, "a", ae)
 	cteArgs := exp.NewCommonTableExpression(false, "a(x,y)", ae)
@@ -986,7 +980,7 @@ func (esgs *expressionSQLGeneratorSuite) TestGenerate_CommonTableExpression() {
 }
 
 func (esgs *expressionSQLGeneratorSuite) TestGenerate_CompoundExpression() {
-	ae := newTestAppendableExpression(`SELECT * FROM "b"`, emptyArgs, nil, nil, true)
+	ae := newTestAppendableExpression(`SELECT * FROM "b"`, emptyArgs, nil, nil)
 
 	u := exp.NewCompoundExpression(exp.UnionCompoundType, ae)
 	ua := exp.NewCompoundExpression(exp.UnionAllCompoundType, ae)

diff --git a/sqlgen/insert_sql_generator_test.go b/sqlgen/insert_sql_generator_test.go
@@ -215,7 +215,7 @@ func (igs *insertSQLGeneratorSuite) TestGenerate_withEmptyRows() {
 func (igs *insertSQLGeneratorSuite) TestGenerate_withRowsAppendableExpression() {
 	ic := exp.NewInsertClauses().
 		SetInto(exp.NewIdentifierExpression("", "test", "")).
-		SetRows([]interface{}{newTestAppendableExpression(`select * from "other"`, emptyArgs, nil, nil, true)})
+		SetRows([]interface{}{newTestAppendableExpression(`select * from "other"`, emptyArgs, nil, nil)})
 
 	igs.assertCases(
 		NewInsertSQLGenerator("test", DefaultDialectOptions()),
@@ -227,7 +227,7 @@ func (igs *insertSQLGeneratorSuite) TestGenerate_withRowsAppendableExpression()
 func (igs *insertSQLGeneratorSuite) TestGenerate_withFrom() {
 	ic := exp.NewInsertClauses().
 		SetInto(exp.NewIdentifierExpression("", "test", "")).
-		SetFrom(newTestAppendableExpression(`select c, d from test where a = 'b'`, nil, nil, nil, true))
+		SetFrom(newTestAppendableExpression(`select c, d from test where a = 'b'`, nil, nil, nil))
 
 	icCols := ic.SetCols(exp.NewColumnListExpression("a", "b"))
 	igs.assertCases(
@@ -372,7 +372,7 @@ func (igs *insertSQLGeneratorSuite) TestGenerate_withCommonTables() {
 	opts.WithFragment = []byte("with ")
 	opts.RecursiveFragment = []byte("recursive ")
 
-	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil, true)
+	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil)
 
 	ic := exp.NewInsertClauses().SetInto(exp.NewIdentifierExpression("", "test_cte", ""))
 	icCte1 := ic.CommonTablesAppend(exp.NewCommonTableExpression(false, "test_cte", tse))

diff --git a/sqlgen/select_sql_generator_test.go b/sqlgen/select_sql_generator_test.go
@@ -471,7 +471,7 @@ func (ssgs *selectSQLGeneratorSuite) TestGenerate_withOffset() {
 
 func (ssgs *selectSQLGeneratorSuite) TestGenerate_withCommonTables() {
 
-	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil, true)
+	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil)
 
 	sc := exp.NewSelectClauses().SetFrom(exp.NewColumnListExpression("test_cte"))
 	scCte1 := sc.CommonTablesAppend(exp.NewCommonTableExpression(false, "test_cte", tse))
@@ -489,7 +489,7 @@ func (ssgs *selectSQLGeneratorSuite) TestGenerate_withCommonTables() {
 }
 
 func (ssgs *selectSQLGeneratorSuite) TestGenerate_withCompounds() {
-	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil, true)
+	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil)
 	sc := exp.NewSelectClauses().SetFrom(exp.NewColumnListExpression("test")).
 		CompoundsAppend(exp.NewCompoundExpression(exp.UnionCompoundType, tse)).
 		CompoundsAppend(exp.NewCompoundExpression(exp.IntersectCompoundType, tse))

diff --git a/sqlgen/update_sql_generator_test.go b/sqlgen/update_sql_generator_test.go
@@ -222,7 +222,7 @@ func (usgs *updateSQLGeneratorSuite) TestGenerate_withLimit() {
 }
 
 func (usgs *updateSQLGeneratorSuite) TestGenerate_withCommonTables() {
-	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil, true)
+	tse := newTestAppendableExpression("select * from foo", emptyArgs, nil, nil)
 	uc := exp.NewUpdateClauses().
 		SetTable(exp.NewIdentifierExpression("", "test_cte", "")).
 		SetSetValues(exp.Record{"a": "b", "b": "c"})