[SPARK-23823][SQL] ResolveReferences should preserve treenode origins

[jiahuijiang]

What changes were proposed in this pull request?

Fixes https://issues.apache.org/jira/browse/SPARK-23823

Introduced in #19585

ResolveReferences stopped doing transfromsUp after this change and Attributes sometimes lose its correct origin

How was this patch tested?

Manually tested after this change analyze won't throw away correct Origin for columns

[gatorsmile]

@jiahuijiang Did you hit any error?

[gatorsmile]

ok to test

[SparkQA]

Test build #88733 has finished for PR 20939 at commit b5588ca.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[jiahuijiang]

Yes, for any SQL queries with columns inside.
e.g. "create table /targetTable as (\nselect col1 from /path/to/table1\n)", after parsing the query, I got a QueryPlan where the "col1" project's origin is (2, 7). But after running analyzer.analyze on that query plan, the origin becomes (2, 0). Tested after this change it retains as (2, 7)

[gatorsmile]

retest this please

[gatorsmile]

@jiahuijiang Why do you need to check the value of origin? This is being used for error reporting only.

[SparkQA]

Test build #88755 has finished for PR 20939 at commit b5588ca.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[jiahuijiang]

We are implementing a language server that needs to understand where the column locations, so we are depending on some of these internal APIs

[jiahuijiang]

@gatorsmile Seems the tests are still flaking :/
And @robertzk proposed this fix #20961 instead, where we keep the origin directly inside transformExpression, so that all the mapChildren and mapExpressions automatically have their origins kept. Is it better to take that change instead?

[jiahuijiang]

@gatorsmile Is #20961 crazy to do? It makes sure all the origins are kept correctly after analyzing, so future refactor won't cause similar issue. But not sure whether that behavior should be expected

[gatorsmile]

#20961 looks good to me. Could you close this PR? Thanks!