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

sqlserver dialect generates invalid limit syntax for prepared statement #225

Closed
1 task done
gavbaa opened this issue Jul 10, 2020 · 0 comments
Closed
1 task done

Comments

@gavbaa
Copy link

gavbaa commented Jul 10, 2020

Describe the bug
SQL Server requires SELECT TOP (@p1) ... when generating a parameterized statement (parentheses around the parameter, only for TOP). This library generates SELECT TOP @p1 ... which results in an error: mssql: Incorrect syntax near '@p1'

To Reproduce
I wrote the following (a copy of the TestLimitOffset test) that can be dropped right in sqlserver_test.go:

func (mt *sqlserverTest) TestLimitOffsetParameterized() {
	ds := mt.db.From("entry").Prepared(true).Where(goqu.C("id").Gte(1)).Limit(1)
	s, _, _ := ds.ToSQL()
	println(s)

	var e entry
	found, err := ds.ScanStruct(&e)
	mt.NoError(err)
	mt.True(found)
	mt.Equal(uint32(1), e.ID)

	ds = mt.db.From("entry").Where(goqu.C("id").Gte(1)).Order(goqu.C("id").Desc()).Limit(1)
	found, err = ds.ScanStruct(&e)
	mt.NoError(err)
	mt.True(found)
	mt.Equal(uint32(10), e.ID)

	ds = mt.db.From("entry").Where(goqu.C("id").Gte(1)).Order(goqu.C("id").Asc()).Offset(1).Limit(1)
	found, err = ds.ScanStruct(&e)
	mt.NoError(err)
	mt.True(found)
	mt.Equal(uint32(2), e.ID)
}

Expected behavior
Running the tests produces the following output:

SELECT  TOP @p1 * FROM "entry" WHERE ("id" >= @p2)
--- FAIL: TestSqlServerSuite (0.34s)
    --- FAIL: TestSqlServerSuite/TestLimitOffsetParameterized (0.02s)
        sqlserver_test.go:368:
            	Error Trace:	sqlserver_test.go:368
            	Error:      	Received unexpected error:
            	            	mssql: Incorrect syntax near '@p1'.
            	Test:       	TestSqlServerSuite/TestLimitOffsetParameterized
        sqlserver_test.go:369:
            	Error Trace:	sqlserver_test.go:369
            	Error:      	Should be true
            	Test:       	TestSqlServerSuite/TestLimitOffsetParameterized
        sqlserver_test.go:370:
            	Error Trace:	sqlserver_test.go:370
            	Error:      	Not equal:
            	            	expected: 0x1
            	            	actual  : 0x0
            	Test:       	TestSqlServerSuite/TestLimitOffsetParameterized
FAIL
exit status 1
FAIL	github.com/doug-martin/goqu/v9/dialect/sqlserver	0.465s

Dialect:

  • sqlserver

Additional context
This is a known SQL Server limitation that other people have also experienced in DB drivers, example: https://stackoverflow.com/questions/56132497/parametrizing-top-value-in-tsql-and-pyodbc

I looked into common_sql_generator.go, LimitSQL, but since it only takes a fragment, not a template string or similar, there's no way to wrap the limit parameter. That might have to be an additional piece of configuration in the dialect, but I suspect you might have something similar used elsewhere that would be more appropriate/well-designed than my theoretical hack of a template string.

doug-martin added a commit that referenced this issue Sep 17, 2020
* [FIXED] SELECT inherits dialect from INSERT in INSERT FROM SELECT.  #229, #223 - @vlanse
* [FIXED] SQLServer dialect: support prepared statements with TOP.  #230, #225 - @vlanse
* [ADDED] IsPrepared to SQLExpression interface.  #231 - @vlanse
@doug-martin doug-martin mentioned this issue Sep 17, 2020
doug-martin added a commit that referenced this issue Sep 17, 2020
* [FIXED] SELECT inherits dialect from INSERT in INSERT FROM SELECT.  #229, #223 - @vlanse
* [FIXED] SQLServer dialect: support prepared statements with TOP.  #230, #225 - @vlanse
* [ADDED] IsPrepared to SQLExpression interface.  #231 - @vlanse
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant