fix: Database#execute_batch properly handles non-ASCII strings #526
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Math2
reviewed
Apr 19, 2024
| @@ -72,7 +72,7 @@ prepare(VALUE self, VALUE db, VALUE sql) | |||
| CHECK(db_ctx->db, status); | |||
| timespecclear(&db_ctx->stmt_deadline); | |||
|
|
|||
| return rb_str_new2(tail); | |||
| return rb_utf8_str_new_cstr(tail); | |||
There was a problem hiding this comment.
Would it be better to reuse the encoding of the original string?
There was a problem hiding this comment.
Regardless of the original encoding, the query ends up encoded as UTF-8 in Statement#initialize and the remainder string being returned from sqlite_prepare is also UTF-8.
This call isn't re-encoding anything, it's just telling Ruby that the C string is in fact UTF-8 (the default is otherwise ASCII-8BIT which is what led to the problem you reported).
There was a problem hiding this comment.
Ah, I see. Alright, thanks for fixing this!
Fix a regression in v2.0.0 that caused `Database#execute_batch` to raise an encoding exception when passed some non-ASCII strings. As a result of this fix, `Database#prepare` now ensures the "remainder" string will always be encoded as UTF-8. Closes #524
flavorjones
force-pushed
the
524-utf8-execute-batch
branch
from
3c67c7b
to
f4c326e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a regression in v2.0.0 that caused
Database#execute_batchto raise an encoding exception when passed some non-ASCII strings. As a result of this fix,Database#preparenow ensures the "remainder" string will always be encoded as UTF-8.Closes #524