Skip to content

fix(expressions): keep tabs inside string literals - #4001

Open
Rodrigo-Palma wants to merge 1 commit into
apache:mainfrom
Rodrigo-Palma:fix/parser-preserve-tabs
Open

Rodrigo-Palma wants to merge 1 commit into
apache:mainfrom
Rodrigo-Palma:fix/parser-preserve-tabs

Conversation

@Rodrigo-Palma

Copy link
Copy Markdown
Contributor

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:

>>> from pyiceberg.expressions.parser import parse
>>> parse("a = 'x\ty'").literal.value
'x  y'
>>> parse("ab = 'x\ty'").literal.value
'x y'

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 on main at 0d58407 and pass with the change.

Ran locally: make lint (ruff, ruff-format, mypy, pydocstyle, codespell, uv-lock) all green; tests/expressions 584 passed, tests/table 512 passed, tests/utils 203 passed, tests/io/test_io.py 30 passed. Not run here: tests/io/test_fsspec.py, which needs a moto server on localhost:9000, and tests/avro/test_decoder.py, whose CythonBinaryDecoder cases fail on a clean main in 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.

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.
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.

1 participant