Avoid promoting inline string types larger than String31 #949
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
Fixes #945. The core issue here is an inconsistency between the initial type detection code and the later-on type promotion code while parsing. The type detection code had a limit setup so that inline string column types weren't allowed larger than `String31`. The promotion code, however, allowed inline string types to continue to promote up to `String255`. That means if a column type was at least initiall detected as some smaller-than-`String31` type, then later on a value larger than 31 bytes was parsed, it would continue to promote up to larger inline string types. The change proposed in this PR is to limit the promotion code to be more consistent with the type detection code: if a parsed value "overflows" the `String31` type, we'll just promote to a regular `String` instead of promoting to larger inline string types.
Codecov Report
@@ Coverage Diff @@
## main #949 +/- ##
==========================================
- Coverage 90.35% 90.27% -0.09%
==========================================
Files 9 9
Lines 2302 2303 +1
==========================================
- Hits 2080 2079 -1
- Misses 222 224 +2
Continue to review full report at Codecov.
|
Fixes #948. As noted in the included code comment, if the last expected value to be parsed is `missing`, the delimited file may not include a final newline, so the EOF, while abrupt, can be treated as valid. In this specific case, we can avoid emitting the warning and just ensure that column is marked as having a `missing` value parsed.
|
Thank you! Are you planning to make a patch release or you are waiting for some additional changes before this? |
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.
Fixes #945. The core issue here is an inconsistency between the initial
type detection code and the later-on type promotion code while parsing.
The type detection code had a limit setup so that inline string column
types weren't allowed larger than
String31. The promotion code,however, allowed inline string types to continue to promote up to
String255. That means if a column type was at least initiall detectedas some smaller-than-
String31type, then later on a value larger than31 bytes was parsed, it would continue to promote up to larger inline
string types. The change proposed in this PR is to limit the promotion
code to be more consistent with the type detection code: if a parsed
value "overflows" the
String31type, we'll just promote to a regularStringinstead of promoting to larger inline string types.