Phoenix500526 commented on PR #22998: URL: https://github.com/apache/datafusion/pull/22998#issuecomment-6368869277
@alamb Thanks for suggesting a Projection above the USING Join. I’m planning to materialize the merged keys there, while retaining the original keys so that explicit references such as `a.k` and `b.k` remain available. Each merged key would have a unique internal qualifier. The Projection would also carry a `Vec<MergedUsingKey>`, with each entry recording the merged column and its resolved left and right source columns. Name resolution and wildcard expansion could then use this mapping to select the merged key and exclude the corresponding source keys. This also handles chained joins: ```sql FROM (a FULL JOIN b USING (k)) FULL JOIN c USING (k) ``` The mappings would be: ```text merged_1.k ← (a.k, b.k) merged_2.k ← (merged_1.k, c.k) ``` Recording these resolved references avoids having to infer which `k` belongs to which join. An independent `k` introduced by a CROSS JOIN would still remain a separate candidate and cause an ambiguity error. I’d like your feedback on two observable changes: **1. The raw output of `join_using`** Its documentation currently describes an output schema that retains both source keys. This proposal would retain those keys and add the merged key: ```text Current: a.k, b.k Proposed: a.k, b.k, merged.k ``` This changes the raw plan’s schema and direct execution output, even though SQL `SELECT *` would expose only the merged key. Would this change be acceptable with updated documentation, or would you recommend a different approach? **2. Column order for `SELECT *`** Consider: ```sql CREATE TABLE a (x INT, k INT); CREATE TABLE b (k INT, y INT); INSERT INTO a VALUES (10, 1); INSERT INTO b VALUES (1, 100); CREATE TABLE result_table (x INT, k INT, y INT); INSERT INTO result_table SELECT * FROM a JOIN b USING (k); ``` The current wildcard order is `(x, k, y)`, producing `(10, 1, 100)`. Moving the merged key to the front would produce `(k, x, y)` with values `(1, 10, 100)`. Since INSERT maps columns by position, this would silently swap the values stored in `x` and `k`. I propose preserving the current wildcard order by placing the merged key where the existing logic retains the source key. Does that sound reasonable for this PR? Finally, the new mapping would need corresponding protobuf support if it is preserved through plan serialization. I would verify same-version round trips and follow the proto crate’s existing policy, which does not guarantee compatibility across DataFusion versions. -- 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]
