JingsongLi commented on code in PR #10336:
URL: https://github.com/apache/paimon/pull/10336#discussion_r4180330567
##########
paimon-python/pypaimon/read/table_read.py:
##########
@@ -372,7 +381,7 @@ def to_arrow(
"""
effective_bp = self._resolve_blob_parallelism(blob_parallelism)
effective = self._effective_parallelism(parallelism, len(splits))
- schema = PyarrowFieldParser.from_paimon_schema(self.read_type)
+ schema = self._output_arrow_schema()
Review Comment:
**[P2] Apply named projection after converting merged rows to Arrow**
The output schema can have more fields than the physical read type because
multiple aliases may reference the same source column, and
`_parse_expression_projection()` deduplicates those source columns. The Python
row-reader branch in `_arrow_batch_generator()` converts physical row tuples
directly with this output schema and never applies
`_project_batch_to_output()`; the parallel row-reader branch has the same issue.
I reproduced this on a primary-key table with native reading disabled, after
two overlapping commits (`id=1, val=20`, then `id=1, val=30`):
```python
builder = table.new_read_builder().with_projection({
'one': 'id',
'two': 'id',
'value': 'val',
})
```
The physical read type is `['id', 'val']`, but the output schema is `['one',
'two', 'value']`. Both `to_arrow(splits, parallelism=1)` and
`to_arrow_batch_reader(splits, parallelism=1)` raise `KeyError: 'value'`
instead of returning `one=1, two=1, value=30`.
Please convert merged rows with the physical schema first, then apply the
existing expression projection, preserving row kind. A regression test should
cover repeated source columns on a non-raw-convertible primary-key split.
##########
paimon-python/pypaimon/read/read_builder.py:
##########
@@ -186,6 +208,78 @@ def read_type(self) -> List[DataField]:
# Helpers
# ------------------------------------------------------------------
+ def _parse_expression_projection(self, expressions: Dict[str, str]):
+ if not expressions:
+ raise ValueError("Projection expression mapping must not be empty")
+ table_fields = self.table.fields
+ if self.table.options.row_tracking_enabled():
+ table_fields =
SpecialFields.row_type_with_row_tracking(table_fields)
+ field_map = {field.name: field for field in table_fields}
+ projection = []
+ variants = {}
+ outputs = []
+ direct_columns = set()
+ for alias, expression in expressions.items():
+ if not isinstance(alias, str) or not alias:
+ raise TypeError("Projection output names must be non-empty
strings")
+ if alias == ROW_KIND_COLUMN:
+ raise ValueError("Projection output name %r is reserved" %
alias)
+ if not isinstance(expression, str) or not expression:
+ raise TypeError("Projection expressions must be non-empty
strings")
+ if expression in field_map:
+ source, child = expression, None
+ direct_columns.add(source)
+ else:
+ try:
+ call = ast.parse(expression, mode='eval').body
Review Comment:
**[P2] Preserve SQL string-literal semantics when parsing paths**
`ast.parse()` applies Python literal rules, so SQL's doubled single quote is
silently removed by concatenating adjacent Python string literals. For example,
`try_variant_get(payload, '$["it''s"]', 'float')` is parsed as the path
`$["its"]`.
I reproduced this with an actual table containing `{"it's": 1.0, "its":
2.0}` and the native reader: `with_projection({'x': expression})` returns
`2.0`, while `SQLContext` executing the exact same expression returns `1.0`.
Both `variant_get` and `try_variant_get` are affected. This reads a different
field without reporting an error.
Please parse literals using SQL rules, or at least reject this syntax
explicitly instead of silently rewriting the path. A regression test should
compare the named projection with SQL for a key containing an apostrophe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
