fix(expressions): keep tabs inside string literals - #4001
Open
Rodrigo-Palma wants to merge 1 commit into
Open
Rodrigo-Palma wants to merge 1 commit into
Rodrigo-Palma wants to merge 1 commit into
Conversation
pyparsing expands tabs in the input before matching, so a tab inside a string
literal was rewritten into spaces, and the number of spaces depended on where
the literal sat in the expression: parse("a = 'x\ty'") and parse("ab = 'x\ty'")
returned different values for the same literal. A filter on a value containing
a tab silently matched nothing.
This file contains hidden or 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
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.
Rationale for this change
parse()hands the expression straight to pyparsing, which expands tabs in the input before matching. A tab inside a string literal is therefore rewritten into spaces before the literal is ever read, and because the expansion runs over the whole expression, the number of spaces depends on where the literal happens to sit:Two filters written against the same stored value produce two different literals, and neither is the value the user asked for. A row whose column really contains a tab is never matched, and nothing reports an error:
row_filter="col = 'a\tb'"simply returns no rows.pyparsing exposes
parse_with_tabs()precisely for this, and it is set once on the top-level element rather than per call.Are these changes tested?
Yes, two tests in
tests/expressions/test_parser.py: one asserting that a tab survives the round trip, and one asserting that the same literal parses identically from two positions, which is the part that makes the corruption silent. Both fail onmainat0d58407and pass with the change.Ran locally:
make lint(ruff, ruff-format, mypy, pydocstyle, codespell, uv-lock) all green;tests/expressions584 passed,tests/table512 passed,tests/utils203 passed,tests/io/test_io.py30 passed. Not run here:tests/io/test_fsspec.py, which needs a moto server onlocalhost:9000, andtests/avro/test_decoder.py, whoseCythonBinaryDecodercases fail on a cleanmainin this checkout because the Cython extension is stale.Are there any user-facing changes?
Yes, in the sense that a filter over a string containing a tab now matches the rows it names. No public signature changes, and expressions without tabs parse exactly as before.