From fbd82a3e39cbbfb398e7eb3102943b6a3e63058c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E7=9B=8A?= Date: Fri, 15 Nov 2019 16:29:24 +0800 Subject: [PATCH 1/8] Fix ReturnsColumns() nil pointer panic --- delete_dataset.go | 5 ++++- insert_dataset.go | 5 ++++- update_dataset.go | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/delete_dataset.go b/delete_dataset.go index f57b1e5d..e48d13c0 100644 --- a/delete_dataset.go +++ b/delete_dataset.go @@ -217,7 +217,10 @@ func (dd *DeleteDataset) GetAs() exp.IdentifierExpression { } func (dd *DeleteDataset) ReturnsColumns() bool { - return !dd.clauses.Returning().IsEmpty() + if cols := dd.clauses.Returning(); cols != nil { + return !cols.IsEmpty() + } + return false } // Creates an QueryExecutor to execute the query. diff --git a/insert_dataset.go b/insert_dataset.go index 3f3ddc90..e77e009c 100644 --- a/insert_dataset.go +++ b/insert_dataset.go @@ -229,7 +229,10 @@ func (id *InsertDataset) GetAs() exp.IdentifierExpression { } func (id *InsertDataset) ReturnsColumns() bool { - return !id.clauses.Returning().IsEmpty() + if cols := id.clauses.Returning(); cols != nil { + return !cols.IsEmpty() + } + return false } // Generates the INSERT sql, and returns an QueryExecutor struct with the sql set to the INSERT statement diff --git a/update_dataset.go b/update_dataset.go index 2d24d732..cd5d0376 100644 --- a/update_dataset.go +++ b/update_dataset.go @@ -222,7 +222,10 @@ func (ud *UpdateDataset) GetAs() exp.IdentifierExpression { } func (ud *UpdateDataset) ReturnsColumns() bool { - return !ud.clauses.Returning().IsEmpty() + if cols := ud.clauses.Returning(); cols != nil { + return !cols.IsEmpty() + } + return false } // Generates the UPDATE sql, and returns an exec.QueryExecutor with the sql set to the UPDATE statement From e1d26c857fa76f220c7cc849d098994d4501e374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E7=9B=8A?= Date: Mon, 18 Nov 2019 10:27:45 +0800 Subject: [PATCH 2/8] Add tests for InsertDataset.ReturnsColumns() UpdateDataset.ReturnsColumns() DeleteDataset.ReturnsColumns() --- delete_dataset_test.go | 6 ++++++ insert_dataset_test.go | 6 ++++++ update_dataset_test.go | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/delete_dataset_test.go b/delete_dataset_test.go index 4b53ed5d..aef2f447 100644 --- a/delete_dataset_test.go +++ b/delete_dataset_test.go @@ -379,6 +379,12 @@ func (dds *deleteDatasetSuite) TestReturning() { ) } +func (dds *deleteDatasetSuite) TestReturnsColumns() { + ds := Delete("test") + dds.False(ds.ReturnsColumns()) + dds.True(ds.Returning("foo", "bar").ReturnsColumns()) +} + func (dds *deleteDatasetSuite) TestToSQL() { md := new(mocks.SQLDialect) ds := Delete("test").SetDialect(md) diff --git a/insert_dataset_test.go b/insert_dataset_test.go index f9d27e00..b27bb98b 100644 --- a/insert_dataset_test.go +++ b/insert_dataset_test.go @@ -383,6 +383,12 @@ func (ids *insertDatasetSuite) TestReturning() { ) } +func (ids *insertDatasetSuite) TestReturnsColumns() { + ds := Insert("test") + ids.False(ds.ReturnsColumns()) + ids.True(ds.Returning("foo", "bar").ReturnsColumns()) +} + func (ids *insertDatasetSuite) TestExecutor() { mDb, _, err := sqlmock.New() ids.NoError(err) diff --git a/update_dataset_test.go b/update_dataset_test.go index 2bbc4046..3f111b1d 100644 --- a/update_dataset_test.go +++ b/update_dataset_test.go @@ -378,6 +378,12 @@ func (uds *updateDatasetSuite) TestReturning() { ) } +func (uds *updateDatasetSuite) TestReturnsColumns() { + ds := Update("test") + uds.False(ds.ReturnsColumns()) + uds.True(ds.Returning("foo", "bar").ReturnsColumns()) +} + func (uds *updateDatasetSuite) TestToSQL() { md := new(mocks.SQLDialect) ds := Update("test").SetDialect(md) From b954441a6e8a177ebad2caa30750d3e5e4bf4a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E7=9B=8A?= Date: Tue, 19 Nov 2019 12:13:53 +0800 Subject: [PATCH 3/8] Refactor ReturnsColumns(), use dataset clauses.HasReturning() --- delete_dataset.go | 5 +---- exp/delete_clauses.go | 2 +- exp/insert_clauses.go | 2 +- exp/update_clauses.go | 2 +- insert_dataset.go | 5 +---- update_dataset.go | 5 +---- 6 files changed, 6 insertions(+), 15 deletions(-) diff --git a/delete_dataset.go b/delete_dataset.go index e48d13c0..e4f8bded 100644 --- a/delete_dataset.go +++ b/delete_dataset.go @@ -217,10 +217,7 @@ func (dd *DeleteDataset) GetAs() exp.IdentifierExpression { } func (dd *DeleteDataset) ReturnsColumns() bool { - if cols := dd.clauses.Returning(); cols != nil { - return !cols.IsEmpty() - } - return false + return dd.clauses.HasReturning() } // Creates an QueryExecutor to execute the query. diff --git a/exp/delete_clauses.go b/exp/delete_clauses.go index 6d80e840..6ba21c86 100644 --- a/exp/delete_clauses.go +++ b/exp/delete_clauses.go @@ -166,7 +166,7 @@ func (dc *deleteClauses) Returning() ColumnListExpression { } func (dc *deleteClauses) HasReturning() bool { - return dc.returning != nil + return dc.returning != nil && !dc.returning.IsEmpty() } func (dc *deleteClauses) SetReturning(cl ColumnListExpression) DeleteClauses { diff --git a/exp/insert_clauses.go b/exp/insert_clauses.go index 295486ed..e3176629 100644 --- a/exp/insert_clauses.go +++ b/exp/insert_clauses.go @@ -112,7 +112,7 @@ func (ic *insertClauses) Returning() ColumnListExpression { } func (ic *insertClauses) HasReturning() bool { - return ic.returning != nil + return ic.returning != nil && !ic.returning.IsEmpty() } func (ic *insertClauses) SetReturning(cl ColumnListExpression) InsertClauses { diff --git a/exp/update_clauses.go b/exp/update_clauses.go index 35715700..e683f787 100644 --- a/exp/update_clauses.go +++ b/exp/update_clauses.go @@ -205,7 +205,7 @@ func (uc *updateClauses) Returning() ColumnListExpression { } func (uc *updateClauses) HasReturning() bool { - return uc.returning != nil + return uc.returning != nil && !uc.returning.IsEmpty() } func (uc *updateClauses) SetReturning(cl ColumnListExpression) UpdateClauses { diff --git a/insert_dataset.go b/insert_dataset.go index e77e009c..06b5268f 100644 --- a/insert_dataset.go +++ b/insert_dataset.go @@ -229,10 +229,7 @@ func (id *InsertDataset) GetAs() exp.IdentifierExpression { } func (id *InsertDataset) ReturnsColumns() bool { - if cols := id.clauses.Returning(); cols != nil { - return !cols.IsEmpty() - } - return false + return id.clauses.HasReturning() } // Generates the INSERT sql, and returns an QueryExecutor struct with the sql set to the INSERT statement diff --git a/update_dataset.go b/update_dataset.go index cd5d0376..03cc5e1f 100644 --- a/update_dataset.go +++ b/update_dataset.go @@ -222,10 +222,7 @@ func (ud *UpdateDataset) GetAs() exp.IdentifierExpression { } func (ud *UpdateDataset) ReturnsColumns() bool { - if cols := ud.clauses.Returning(); cols != nil { - return !cols.IsEmpty() - } - return false + return ud.clauses.HasReturning() } // Generates the UPDATE sql, and returns an exec.QueryExecutor with the sql set to the UPDATE statement From b658b89ee40f44b46cc57b7add4546fe8c4f2f91 Mon Sep 17 00:00:00 2001 From: Douglas Martin Date: Thu, 5 Dec 2019 20:17:39 -0600 Subject: [PATCH 4/8] Fix for #185 --- HISTORY.md | 4 ++++ Makefile | 2 +- issues_test.go | 26 ++++++++++++++++++++++++++ select_dataset.go | 7 +------ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 64947751..fb37329b 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,7 @@ +# v9.5.1 + +* [FIXED] Unable to execute union with order by expression [#185](https://github.com/doug-martin/goqu/issues/185) + # v9.5.0 * [ADDED] Ability to use regexp like, ilike, notlike, and notilike without a regexp [#172](https://github.com/doug-martin/goqu/issues/172) diff --git a/Makefile b/Makefile index bbc4f602..0249d4d9 100644 --- a/Makefile +++ b/Makefile @@ -3,4 +3,4 @@ phony: lint: - golangci-lint run + docker run --rm -v ${CURDIR}:/app -w /app golangci/golangci-lint:v1.21.0 golangci-lint run -v diff --git a/issues_test.go b/issues_test.go index 25e05abe..067ad77a 100644 --- a/issues_test.go +++ b/issues_test.go @@ -1,11 +1,14 @@ package goqu_test import ( + "context" "strings" "sync" "testing" "time" + "github.com/DATA-DOG/go-sqlmock" + "github.com/doug-martin/goqu/v9" "github.com/stretchr/testify/suite" ) @@ -324,6 +327,29 @@ func (gis *githubIssuesSuite) TestIssue164() { ) } +// Test for https://github.com/doug-martin/goqu/issues/185 +func (gis *githubIssuesSuite) TestIssue185() { + mDb, sqlMock, err := sqlmock.New() + gis.NoError(err) + sqlMock.ExpectQuery( + `SELECT \* FROM \(SELECT "id" FROM "table" ORDER BY "id" ASC\) AS "t1" UNION +\(SELECT \* FROM \(SELECT "id" FROM "table" ORDER BY "id" ASC\) AS "t1"\)`, + ). + WillReturnRows(sqlmock.NewRows([]string{"id"}).FromCSVString("1\n2\n3\n4\n")) + db := goqu.New("mock", mDb) + + ds := db.Select("id").From("table").Order(goqu.C("id").Asc()). + Union( + db.Select("id").From("table").Order(goqu.C("id").Asc()), + ) + + ctx := context.Background() + var i []int + gis.NoError(ds.ScanValsContext(ctx, &i)) + gis.Equal([]int{1, 2, 3, 4}, i) + +} + func TestGithubIssuesSuite(t *testing.T) { suite.Run(t, new(githubIssuesSuite)) } diff --git a/select_dataset.go b/select_dataset.go index ede6b452..4d9fa078 100644 --- a/select_dataset.go +++ b/select_dataset.go @@ -277,12 +277,7 @@ func (sd *SelectDataset) From(from ...interface{}) *SelectDataset { // Returns a new Dataset with the current one as an source. If the current Dataset is not aliased (See Dataset#As) then // it will automatically be aliased. See examples. func (sd *SelectDataset) FromSelf() *SelectDataset { - builder := SelectDataset{ - dialect: sd.dialect, - clauses: exp.NewSelectClauses(), - } - return builder.From(sd) - + return sd.copy(exp.NewSelectClauses()).From(sd) } // Alias to InnerJoin. See examples. From 3b783f933f26ee14c067f58fffc5238dbc7bca12 Mon Sep 17 00:00:00 2001 From: doug-martin Date: Fri, 6 Dec 2019 22:13:53 -0600 Subject: [PATCH 5/8] Fix for #183 * Changed AppendSQL to set error if one is present --- HISTORY.md | 1 + delete_dataset.go | 4 ++++ insert_dataset.go | 4 ++++ issues_test.go | 42 +++++++++++++++++++++++++++++++++++++++++- select_dataset.go | 4 ++++ update_dataset.go | 4 ++++ 6 files changed, 58 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index fb37329b..67a7e354 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,7 @@ # v9.5.1 * [FIXED] Unable to execute union with order by expression [#185](https://github.com/doug-martin/goqu/issues/185) +* [FIXED] SelectDataset From with Error [#183](https://github.com/doug-martin/goqu/issues/183) # v9.5.0 diff --git a/delete_dataset.go b/delete_dataset.go index e4f8bded..032961c3 100644 --- a/delete_dataset.go +++ b/delete_dataset.go @@ -209,6 +209,10 @@ func (dd *DeleteDataset) ToSQL() (sql string, params []interface{}, err error) { // Appends this Dataset's DELETE statement to the SQLBuilder // This is used internally when using deletes in CTEs func (dd *DeleteDataset) AppendSQL(b sb.SQLBuilder) { + if dd.err != nil { + b.SetError(dd.err) + return + } dd.dialect.ToDeleteSQL(b, dd.GetClauses()) } diff --git a/insert_dataset.go b/insert_dataset.go index 06b5268f..e8b106c7 100644 --- a/insert_dataset.go +++ b/insert_dataset.go @@ -221,6 +221,10 @@ func (id *InsertDataset) ToSQL() (sql string, params []interface{}, err error) { // Appends this Dataset's INSERT statement to the SQLBuilder // This is used internally when using inserts in CTEs func (id *InsertDataset) AppendSQL(b sb.SQLBuilder) { + if id.err != nil { + b.SetError(id.err) + return + } id.dialect.ToInsertSQL(b, id.GetClauses()) } diff --git a/issues_test.go b/issues_test.go index 067ad77a..ff4861a6 100644 --- a/issues_test.go +++ b/issues_test.go @@ -2,14 +2,15 @@ package goqu_test import ( "context" + "fmt" "strings" "sync" "testing" "time" "github.com/DATA-DOG/go-sqlmock" - "github.com/doug-martin/goqu/v9" + "github.com/doug-martin/goqu/v9/exp" "github.com/stretchr/testify/suite" ) @@ -327,6 +328,45 @@ func (gis *githubIssuesSuite) TestIssue164() { ) } +// Test for https://github.com/doug-martin/goqu/issues/183 +func (gis *githubIssuesSuite) TestIssue184() { + expectedErr := fmt.Errorf("an error") + testCases := []struct { + ds exp.AppendableExpression + }{ + {ds: goqu.From("test").As("t").SetError(expectedErr)}, + {ds: goqu.Insert("test").Rows(goqu.Record{"foo": "bar"}).Returning("foo").SetError(expectedErr)}, + {ds: goqu.Update("test").Set(goqu.Record{"foo": "bar"}).Returning("foo").SetError(expectedErr)}, + {ds: goqu.Update("test").Set(goqu.Record{"foo": "bar"}).Returning("foo").SetError(expectedErr)}, + {ds: goqu.Delete("test").Returning("foo").SetError(expectedErr)}, + } + + for _, tc := range testCases { + ds := goqu.From(tc.ds) + sql, args, err := ds.ToSQL() + gis.Equal(expectedErr, err) + gis.Empty(sql) + gis.Empty(args) + + sql, args, err = ds.Prepared(true).ToSQL() + gis.Equal(expectedErr, err) + gis.Empty(sql) + gis.Empty(args) + + ds = goqu.From("test2").Where(goqu.Ex{"foo": tc.ds}) + + sql, args, err = ds.ToSQL() + gis.Equal(expectedErr, err) + gis.Empty(sql) + gis.Empty(args) + + sql, args, err = ds.Prepared(true).ToSQL() + gis.Equal(expectedErr, err) + gis.Empty(sql) + gis.Empty(args) + } +} + // Test for https://github.com/doug-martin/goqu/issues/185 func (gis *githubIssuesSuite) TestIssue185() { mDb, sqlMock, err := sqlmock.New() diff --git a/select_dataset.go b/select_dataset.go index 4d9fa078..7c177977 100644 --- a/select_dataset.go +++ b/select_dataset.go @@ -549,6 +549,10 @@ func (sd *SelectDataset) Executor() exec.QueryExecutor { // Appends this Dataset's SELECT statement to the SQLBuilder // This is used internally for sub-selects by the dialect func (sd *SelectDataset) AppendSQL(b sb.SQLBuilder) { + if sd.err != nil { + b.SetError(sd.err) + return + } sd.dialect.ToSelectSQL(b, sd.GetClauses()) } diff --git a/update_dataset.go b/update_dataset.go index 03cc5e1f..4b9c3668 100644 --- a/update_dataset.go +++ b/update_dataset.go @@ -214,6 +214,10 @@ func (ud *UpdateDataset) ToSQL() (sql string, params []interface{}, err error) { // Appends this Dataset's UPDATE statement to the SQLBuilder // This is used internally when using updates in CTEs func (ud *UpdateDataset) AppendSQL(b sb.SQLBuilder) { + if ud.err != nil { + b.SetError(ud.err) + return + } ud.dialect.ToUpdateSQL(b, ud.GetClauses()) } From a0ce8631ca7db8ae48bb37c4c4e815b6040feeaa Mon Sep 17 00:00:00 2001 From: doug-martin Date: Fri, 6 Dec 2019 22:26:28 -0600 Subject: [PATCH 6/8] Update history with #181 --- HISTORY.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index 67a7e354..10ed2e11 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,7 +1,8 @@ # v9.5.1 -* [FIXED] Unable to execute union with order by expression [#185](https://github.com/doug-martin/goqu/issues/185) +* [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) # v9.5.0 From 9b920a3bf0d4b0cc20cd6e88b7f7f7ecb3d77f85 Mon Sep 17 00:00:00 2001 From: doug-martin Date: Fri, 6 Dec 2019 22:46:17 -0600 Subject: [PATCH 7/8] Fix for issue 178 * [FIXED] SQlite dialect escapes single quotes wrong, leads to SQL syntax error [#178](https://github.com/doug-martin/goqu/issues/178) --- HISTORY.md | 1 + dialect/sqlite3/sqlite3.go | 8 +------ dialect/sqlite3/sqlite3_dialect_test.go | 28 ++++++++++++------------- dialect/sqlite3/sqlite3_test.go | 14 +++++++++++-- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 10ed2e11..9fae7138 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ * [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/dialect/sqlite3/sqlite3.go b/dialect/sqlite3/sqlite3.go index c2bed1af..6ad1149a 100644 --- a/dialect/sqlite3/sqlite3.go +++ b/dialect/sqlite3/sqlite3.go @@ -50,13 +50,7 @@ func DialectOptions() *goqu.SQLDialectOptions { } opts.UseLiteralIsBools = false opts.EscapedRunes = map[rune][]byte{ - '\'': []byte("\\'"), - '"': []byte("\\\""), - '\\': []byte("\\\\"), - '\n': []byte("\\n"), - '\r': []byte("\\r"), - 0: []byte("\\x00"), - 0x1a: []byte("\\x1a"), + '\'': []byte("''"), } opts.InsertIgnoreClause = []byte("INSERT OR IGNORE") opts.ConflictFragment = []byte("") diff --git a/dialect/sqlite3/sqlite3_dialect_test.go b/dialect/sqlite3/sqlite3_dialect_test.go index d349f698..47067553 100644 --- a/dialect/sqlite3/sqlite3_dialect_test.go +++ b/dialect/sqlite3/sqlite3_dialect_test.go @@ -63,31 +63,31 @@ func (sds *sqlite3DialectSuite) TestLiteralString() { sql, _, err = ds.Where(goqu.C("a").Eq("test'test")).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\'test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test''test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq(`test"test`)).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\\"test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\"test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq(`test\test`)).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\\\test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq("test\ntest")).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\ntest')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\ntest')", sql) sql, _, err = ds.Where(goqu.C("a").Eq("test\rtest")).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\rtest')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\rtest')", sql) sql, _, err = ds.Where(goqu.C("a").Eq("test\x00test")).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\x00test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\x00test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq("test\x1atest")).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\x1atest')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\x1atest')", sql) } func (sds *sqlite3DialectSuite) TestLiteralBytes() { @@ -98,31 +98,31 @@ func (sds *sqlite3DialectSuite) TestLiteralBytes() { sql, _, err = ds.Where(goqu.C("a").Eq([]byte("test'test"))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\'test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test''test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq([]byte(`test"test`))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\\"test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\"test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq([]byte(`test\test`))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\\\test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq([]byte("test\ntest"))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\ntest')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\ntest')", sql) sql, _, err = ds.Where(goqu.C("a").Eq([]byte("test\rtest"))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\rtest')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\rtest')", sql) sql, _, err = ds.Where(goqu.C("a").Eq([]byte("test\x00test"))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\x00test')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\x00test')", sql) sql, _, err = ds.Where(goqu.C("a").Eq([]byte("test\x1atest"))).ToSQL() sds.NoError(err) - sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\\x1atest')", sql) + sds.Equal("SELECT * FROM `test` WHERE (`a` = 'test\x1atest')", sql) } func (sds *sqlite3DialectSuite) TestBooleanOperations() { diff --git a/dialect/sqlite3/sqlite3_test.go b/dialect/sqlite3/sqlite3_test.go index 5a6210d2..f5e60ad4 100644 --- a/dialect/sqlite3/sqlite3_test.go +++ b/dialect/sqlite3/sqlite3_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/doug-martin/goqu/v9" + "github.com/doug-martin/goqu/v9/dialect/mysql" _ "github.com/mattn/go-sqlite3" "github.com/stretchr/testify/suite" @@ -360,25 +361,34 @@ func (st *sqlite3Suite) TestInsert() { {Int: 12, Float: 1.200000, String: "1.200000", Time: now, Bool: true, Bytes: []byte("1.200000")}, {Int: 13, Float: 1.300000, String: "1.300000", Time: now, Bool: false, Bytes: []byte("1.300000")}, {Int: 14, Float: 1.400000, String: "1.400000", Time: now, Bool: true, Bytes: []byte("1.400000")}, + {Int: 14, Float: 1.400000, String: `abc'd"e"f\\gh\n\ri\x00`, Time: now, Bool: true, Bytes: []byte("1.400000")}, } _, err = ds.Insert().Rows(entries).Executor().Exec() st.NoError(err) var newEntries []entry st.NoError(ds.Where(goqu.C("int").In([]uint32{11, 12, 13, 14})).ScanStructs(&newEntries)) - st.Len(newEntries, 4) + for i, e := range newEntries { + st.Equal(entries[i].Int, e.Int) + st.Equal(entries[i].Float, e.Float) + st.Equal(entries[i].String, e.String) + st.Equal(entries[i].Time.UTC().Format(mysql.DialectOptions().TimeFormat), e.Time.Format(mysql.DialectOptions().TimeFormat)) + st.Equal(entries[i].Bool, e.Bool) + st.Equal(entries[i].Bytes, e.Bytes) + } _, err = ds.Insert().Rows( entry{Int: 15, Float: 1.500000, String: "1.500000", Time: now, Bool: false, Bytes: []byte("1.500000")}, entry{Int: 16, Float: 1.600000, String: "1.600000", Time: now, Bool: true, Bytes: []byte("1.600000")}, entry{Int: 17, Float: 1.700000, String: "1.700000", Time: now, Bool: false, Bytes: []byte("1.700000")}, entry{Int: 18, Float: 1.800000, String: "1.800000", Time: now, Bool: true, Bytes: []byte("1.800000")}, + entry{Int: 18, Float: 1.800000, String: `abc'd"e"f\\gh\n\ri\x00`, Time: now, Bool: true, Bytes: []byte("1.800000")}, ).Executor().Exec() st.NoError(err) newEntries = newEntries[0:0] st.NoError(ds.Where(goqu.C("int").In([]uint32{15, 16, 17, 18})).ScanStructs(&newEntries)) - st.Len(newEntries, 4) + st.Len(newEntries, 5) } func (st *sqlite3Suite) TestInsert_returning() { From 864162a46f06df3e36c4b1ce22ffa5b14589b8a9 Mon Sep 17 00:00:00 2001 From: doug-martin Date: Sat, 7 Dec 2019 12:22:00 -0600 Subject: [PATCH 8/8] [FIXED] WITH clause without a RETURNING clause will panic [#177] * Removed check for returning columns when using insert/update/delete in sub select --- HISTORY.md | 3 +- issues_test.go | 38 +++++++++++++++++++++++++ sqlgen/delete_sql_generator_test.go | 2 +- sqlgen/expression_sql_generator.go | 4 --- sqlgen/expression_sql_generator_test.go | 26 +++++++---------- sqlgen/insert_sql_generator_test.go | 6 ++-- sqlgen/select_sql_generator_test.go | 4 +-- sqlgen/update_sql_generator_test.go | 2 +- 8 files changed, 57 insertions(+), 28 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 9fae7138..72179a03 100644 --- 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 index ff4861a6..7c3bdf20 100644 --- 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 index fc27344c..6582ffca 100644 --- 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 index 4fb6de62..f3e255cf 100644 --- 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 index 8a507b25..33d797c2 100644 --- 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 index ef31871a..8f3e4286 100644 --- 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 index b46645f5..6e0c73ca 100644 --- 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 index bb438ddc..5b6527dd 100644 --- 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"})