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

Fix ResultWrapper handling for non-prepared statements #3009

Closed

Conversation

ng-galien
Copy link
Contributor

@ng-galien ng-galien commented Nov 23, 2023

In order to fix #3008

@ng-galien
Copy link
Contributor Author

@vlsi I did not align all the test because I'm not sure this is the expected behavior.
I mainly apply SET handling only on prepared statements.
When specs for expected results are ok I will finish the job.

@olavloite
Copy link
Contributor

In order to fix #3008

@ng-galien I think you meant to reference #3007

@ng-galien
Copy link
Contributor Author

In order to fix #3008

@ng-galien I think you meant to reference #3007

My bad

@ng-galien
Copy link
Contributor Author

@olavloite
I think all tests related to inlining SET in Statement, except yours, can be removed as it should only affect PreparedStatement.

@olavloite
Copy link
Contributor

`

@olavloite I think all tests related to inlining SET in Statement, except yours, can be removed as it should only affect PreparedStatement.

I don't quite understand what you mean with that:

  1. If your change only affects PreparedStatement, then any existing test for Statement can remain, as they should be unaffected.
  2. If the tests using Statement are failing, then it would seem to me that the assumption that it only affects PreparedStatement is wrong.

Or am I misunderstanding what you mean?

@ng-galien
Copy link
Contributor Author

@olavloite there was not existing test for combining SET and statement. New ones where aligned with the unexpected behavior you related in the issue.

@ng-galien ng-galien closed this Nov 24, 2023
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.

Server default DateStyle is being overwritten
2 participants