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

Fixing mssql_test.js example with correct sp_executesql syntax #40

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

leandrodotec
Copy link
Contributor

sp_executesql syntax needs a statement with @p parameters, not $1.

Just fixing the example for MS-SQL Server 2022.

This PR fixes issue: #39 .

sp_executesql syntax needs a statement  with @p  parameters, not $1. 

Just fixing the example
@CLAassistant
Copy link

CLAassistant commented Jun 11, 2023

CLA assistant check
All committers have signed the CLA.

@imiric
Copy link

imiric commented Jun 12, 2023

Hi, thanks for reporting and fixing this! 🙇 We missed this difference with MS SQL in #6, since we haven't automated testing against real servers yet.

I just tested manually against a SQL Server 2022 container (https://hub.docker.com/_/microsoft-mssql-server, mcr.microsoft.com/mssql/server:2022-latest), and you're right that the previous $1 syntax fails with:

GoError: mssql: Incorrect syntax near the keyword 'key'.

But I still get the same error after changing it to @p1. 😕 @param1 didn't work either. Can you clarify if the new example worked for you? Or maybe which edition of SQL Server 2022 you're running? The container runs the Developer edition, though basic syntax like this shouldn't change. I'm not familiar with MS SQL, so not sure if we have to use SET somehow to define the variable, but according to the documentation, @param1 should work...

@leandrodotec
Copy link
Contributor Author

Hi, sorry. I have just realised the mistake I made. Here is the right query line:

SELECT * FROM keyvalues WHERE [key] = @p1;

I ran the example again with the query above and now it is working fine. I thought having brackets was optional, but apparently "key" is a reserved word in MS-SQL.

It is all good now. I ran over SQL 2022 developer edition.

Copy link

@imiric imiric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, it works now! Thanks a lot for explaining and fixing this! 🙇

We'll make sure to prioritize #5 so that these issues don't happen anymore.

@imiric imiric linked an issue Jun 13, 2023 that may be closed by this pull request
@imiric imiric merged commit 78ccee4 into grafana:master Jun 13, 2023
jt-shippit pushed a commit to jt-shippit/xk6-sql that referenced this pull request Jun 14, 2024
…na#40)

* Fixing mssql_test.js example with correct sp_executesql syntax

sp_executesql syntax needs a statement  with @p  parameters, not $1. 

Just fixing the example

* Update mssql_test.js
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

Successfully merging this pull request may close these issues.

Example for MS-SQL does not retrieve values properly from SQL Server 2022
3 participants