szehon-ho commented on PR #55329:
URL: https://github.com/apache/spark/pull/55329#issuecomment-4580763636

   Thanks for the fix - switching to `UnresolvedAttribute.quoted(...)` is 
correct and matches the existing convention already used for static-partition 
resolution in `Analyzer.scala` (the `UnresolvedAttribute.quoted(attr.name)` 
call there, which has a comment explaining the same deferred-resolution 
rationale).
   
   Two additional test scenarios that would be good to cover, possibly as a 
follow-up:
   
   1. **Nested field with a special-char name.** Both new cases add *top-level* 
columns. A source `struct` column evolving to add a nested field whose name 
contains a dot (e.g. an `info` struct gaining a `` `job.title` `` subfield) 
would exercise the nested path. I believe this already works today since the 
parsing only happened at the top-level assignment key and nested expansion uses 
field names verbatim, but it's the natural sibling case and is currently 
untested.
   
   2. **Special-char column that already exists in the target.** The bug was 
only in the `getOrElse` (new-column) branch; when `findAttrInTarget(...)` 
returns `Some`, the key is an already-resolved attribute. A case where the 
target *already* has a `` `job.title` `` column being updated (ideally combined 
with a type change that triggers widening) would cover the other half of that 
branch.
   
   Both are coverage-only and don't block the fix - happy for them to land as a 
follow-up PR if you'd prefer to keep this one focused.


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