Skip to content

feat: add support for 1.17 back#385

Merged
marco6 merged 6 commits intov3from
fix-build-1.17
Jul 10, 2025
Merged

feat: add support for 1.17 back#385
marco6 merged 6 commits intov3from
fix-build-1.17

Conversation

@marco6
Copy link
Copy Markdown
Collaborator

@marco6 marco6 commented Jul 10, 2025

No description provided.

@marco6 marco6 requested review from just-now and letFunny July 10, 2025 09:27
@letFunny
Copy link
Copy Markdown
Collaborator

@marco6 I see that TestIntegration_ExecBindError is failing for some versions.

@marco6
Copy link
Copy Markdown
Collaborator Author

marco6 commented Jul 10, 2025

I see that TestIntegration_ExecBindError is failing for some versions.

@letFunny it is failing for all 1.18.x versions and will keep doing that as the EXEC_SQL command is broken by design. If you remember, we had 2 choices (which the current test clearly is not highlighting):

  • support binding only on the first statement (which was the case for 1.17) so db.Exec("INSERT INTO a VALUES (?); INSERT INTO b VALUES (?)", 1, 2) fails (and it is clearly not tested as it wouldn't have looked great!) but on the other side db.Exec("INSERT INTO a VALUES (?)", 1, 2) is checked correctly and fails
  • supporting binding on multiple statements (which is the case for 1.18) so the exact reverse. It would be a bit weird to return "error".

There is a "fix" that uses a prepared statement in #383.

Copy link
Copy Markdown
Contributor

@just-now just-now left a comment

Choose a reason for hiding this comment

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

Looks good.

@letFunny
Copy link
Copy Markdown
Collaborator

@marco6 so you are fixing the tests as part of another PR, then it's okay for me. I just don't want tests always failing.

Copy link
Copy Markdown
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

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

Just one question and LGTM!

Comment thread internal/bindings/server.go Outdated
@marco6 marco6 requested a review from letFunny July 10, 2025 13:40
@marco6 marco6 merged commit 990ab58 into v3 Jul 10, 2025
13 of 30 checks passed
@marco6 marco6 deleted the fix-build-1.17 branch July 10, 2025 14:23
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.

3 participants