sollhui commented on PR #67418:
URL: https://github.com/apache/doris/pull/67418#issuecomment-6307030679
Reviewed head `34b7a98d2c46c11219e8b28761dc97b97db10090`. I checked the
schema-inference-to-scan call chain and found two issues worth addressing
before merging:
1. **[P1] Preserve original JSONPath positions when projecting a subset of
columns.**
With the new fixture and `jsonpaths=["$.left.id", "$.right.id",
"$.city"]`, querying only `right_id` causes FE to prune the other slots and
send `required_slots=[right_id]` with `column_idxs=[1]`. However,
[`_simdjson_write_columns_by_jsonpath()`](https://github.com/apache/doris/blob/34b7a98d2c46c11219e8b28761dc97b97db10090/be/src/format/json/new_json_reader.cpp#L1554-L1559)
still indexes `_parsed_jsonpaths` by the compacted slot position `i=0`. It
therefore reads `$.left.id` for `right_id`, yielding `1, 2` instead of `10, 20`.
The positional-read issue predates this PR, but it also affects the
duplicate-leaf queries newly enabled here. Please map query slots to their
original JSONPath indices, while preserving load-path semantics, and add a
regression selecting only the non-first column. The current regression selects
all columns and misses this case.
2. **[P2] Ensure the final derived column names are unique.**
[`jsonpath_column_name()` /
`derive_jsonpath_column_names()`](https://github.com/apache/doris/blob/34b7a98d2c46c11219e8b28761dc97b97db10090/be/src/format/json/new_json_reader.cpp#L88-L122)
only count original leaf names and concatenate keys with underscores. This
still produces collisions:
| JSONPaths | Generated column names |
| --- | --- |
| `$.left.id`, `$.right.id`, `$.left_id` | `left_id`, `right_id`,
`left_id` |
| `$.a_b.id`, `$.a.b.id` | `a_b_id`, `a_b_id` |
| `$.items[0].id`, `$.items[1].id` | `items_id`, `items_id` |
The last case loses the supported numeric array indices because the
naming code reads only `JsonPath.key`. All three cases are subsequently
rejected by FE's `fillColumns()` with `Repeated lowercase column names`. Please
preserve array-index context and perform a final case-insensitive uniqueness
pass across all output names, using a deterministic fallback for collisions.
These cases should be covered by tests.
Validation: the projection finding is based on tracing the exact revision's
FE tuple pruning, `column_idxs` construction, and BE reader indexing; it has
not been reproduced against a running cluster. The naming collisions were
reproduced by compiling and running the PR's original helper in an isolated C++
harness with pre-parsed paths and minimal dependency stubs. I did not build
full Doris or run the cluster regression suite.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
