cloud-fan commented on code in PR #55582:
URL: https://github.com/apache/spark/pull/55582#discussion_r3160669276


##########
python/pyspark/sql/tests/test_column.py:
##########
@@ -558,6 +558,29 @@ def test_select_join_keys(self):
             self.assertTrue(df1.join(df2, "id", how).select(df1["id"]).count() 
>= 0, how)
             self.assertTrue(df1.join(df2, "id", how).select(df2["id"]).count() 
>= 0, how)
 
+    def 
test_select_regular_column_with_reused_dataframe_hidden_in_natural_join(self):
+        # A DataFrame appears both as a direct join side and inside a 
natural/USING
+        # join that hides one of its columns into `metadataOutput`. When 
resolving
+        # `dim["dim_id"]`, two candidates match the plan id: one from 
`p.output`
+        # (the direct join side) and one only visible via `p.metadataOutput` 
(the
+        # reused `dim` nested under the USING-join wrapper). We should prefer 
the
+        # regular candidate and not throw AMBIGUOUS_COLUMN_REFERENCE.
+        fact = self.spark.createDataFrame([(1, 10, "T1"), (2, 20, "T2")], 
["id", "fk", "txn_id"])
+        dim = self.spark.createDataFrame([(10, "X"), (20, "Y"), (30, "Z")], 
["dim_id", "dim_name"])
+        events = self.spark.createDataFrame(
+            [(10, "T1", 100), (20, "T2", 200)], ["dim_id", "txn_id", "amount"]

Review Comment:
   Late catch on my own prior suggestion — the new value-based assertion still 
doesn't pin resolution to the regular candidate. Under this data, every events 
row matches a dim row by `dim_id`, so the USING left-join forces the hidden 
`dim.dim_id` to equal `events.dim_id` for both rows — i.e., both the regular 
path (fact-side direct join's `dim.dim_id`) and the hidden path (enriched-side 
`dim.dim_id`) yield `[10, 20]` per `txn_id`. The assertion would pass either 
way.
   
   To make it discriminate: introduce an `events` row whose `dim_id` doesn't 
match anything in `dim` while sharing a `txn_id` with a matching fact row. Then 
the hidden value is NULL for that row while the regular value is non-null, and 
`[10, 20]` only holds under the regular path. Smallest tweak — flip the second 
`events` row's `dim_id` from `20` to a non-matching value:
   
   ```suggestion
               [(10, "T1", 100), (99, "T2", 200)], ["dim_id", "txn_id", 
"amount"]
   ```
   
   Soft — the current test still verifies the throw is gone, which is the core 
regression. This just turns a silent mis-resolution into a test failure.



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

Reply via email to