JingsongLi commented on code in PR #9789:
URL: https://github.com/apache/paimon/pull/9789#discussion_r4004700523
##########
paimon-python/pypaimon/read/table_read.py:
##########
@@ -1020,9 +1033,50 @@ def _widen_to_top_level_for_merge(self) ->
List[DataField]:
raise ValueError(
"Nested projection top-level field %r not found in "
"table schema" % (top_name,))
+ paths = paths_by_top[top_name]
+ if (isinstance(field.type, MapType)
+ and all(len(path) > 1 for path in paths)
+ and not self._map_has_aggregator(top_name)):
+ keys = []
+ for path in paths:
+ if path[1] not in keys:
+ keys.append(path[1])
+ try:
+ field = map_selected_keys_field(field, keys)
Review Comment:
[P2] Preserve the full-MAP fallback when the physical reader is ROW
This replaces the MAP read type with a selected-key ROW before the physical
format is known, but `FormatRowReader` still returns a MAP array.
`DataFileBatchReader` then raises `ArrowNotImplementedError: Unsupported cast
from map<string, int64> to struct using function cast_struct`.
This also affects the explicitly supported Parquet data-evolution path:
enable `row-tracking.enabled`, `data-evolution.enabled`, and
`data-evolution.row-sidecar.enabled`, write 100 rows with `attributes =
{'first': row_id}`, and read `_ROW_ID = 5` while projecting only
`attributes['first']`. The sparse read selects the ROW sidecar and fails.
Projecting both the complete `attributes` MAP and the key succeeds and returns
5; a table using `file.format = row` exhibits the same failure.
Please retain the complete-MAP fallback for readers that do not assemble
selected-key ROWs, or add that conversion to the ROW reader. Checking only the
table's `file.format` would miss Parquet tables that select a ROW sidecar.
##########
paimon-python/pypaimon/read/read_builder.py:
##########
@@ -181,16 +197,28 @@ def _resolve_dotted_paths(self, names: List[str]) ->
List[List[int]]:
if name in top_index:
paths.append([top_index[name]])
continue
+
+ map_selector = _map_key_selector(name, table_fields)
+ if map_selector is not None:
+ top, key = map_selector
+ paths.append([top_index[top], MapKey(key)])
+ continue
+
if '.' not in name:
continue
- parts = name.split('.')
- top = parts[0]
- if top not in top_index:
+ candidates = [
+ field_name for field_name in top_index
+ if name.startswith(field_name + '.')
+ and is_row_type(table_fields[top_index[field_name]].type)
+ ]
+ if not candidates:
continue
+ top = max(candidates, key=len)
Review Comment:
[P2] Preserve the existing ROW-path resolution priority
Choosing the longest top-level ROW prefix changes the meaning of an already
valid projection. For example, with top-level fields `a: ROW<b: ROW<c:
BIGINT>>`, `a.b: ROW<d: BIGINT>`, and `id`, `with_projection(['a.b.c', 'id'])`
previously returned `a_b_c` and `id`. This code selects the top-level `a.b`
first, fails to find its child `c`, and silently drops the projection instead
of resolving `a -> b -> c`. If the top-level `a.b` also has a child `c`, the
same projection silently reads a different column. I reproduced both cases
against the base and PR implementations using actual table writes and reads.
Could we preserve the original valid ROW path first, and only try dotted
top-level prefixes when that path cannot be resolved? Exact top-level name
matches can still retain their existing precedence.
--
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]