Use correct origin for attributes #325
Conversation
|
Can we propose this upstream? This looks like a generic bug and should be fixed there. Take a look at http://spark.apache.org/contributing.html |
|
@robert3005 kk! Btw in terms of testing, I can't find much reference on tests related to Origin... Should I just add a test to analyze a simple query and check the Origin for every expression? |
|
I would verify for the cases that it failed before that origin is the same before and after analysis. Slightly curious what actual thing didn't respect this since TreeNode#transform/up/down preserves origin so must be some of the nested expressions |
|
It's broken by this commit apache@36b826f |
This reverts commit 80c0808.
| var i = 0 | ||
| while (i < arr.length) { | ||
| arr(i) = f(productElement(i)) | ||
| CurrentOrigin.withOrigin(origin) { |
There was a problem hiding this comment.
I don't think this is the origin you want - the one you want is productElement(i) since this is the element being mapped over. Fixing it here is hard because of types - as f takes Any. I spent some time looking at this yesterday and I think easiest thing to do is #344
|
Closing in favor of #344 |
|
closed by #344 |
What changes were proposed in this pull request?
Fixes bug introduce in apache#19585
How was this patch tested?
Manually tested "projects" in the parsed logical plan have correct origin now
Please review http://spark.apache.org/contributing.html before opening a pull request.