Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Dec 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion HISTORY.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
38 changes: 38 additions & 0 deletions issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion sqlgen/delete_sql_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 0 additions & 4 deletions sqlgen/expression_sql_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 10 additions & 16 deletions sqlgen/expression_sql_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()),
Expand All @@ -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}},
)
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions sqlgen/insert_sql_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand All @@ -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(
Expand Down Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions sqlgen/select_sql_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion sqlgen/update_sql_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down