[SPARK-29405][SQL] Alter table / Insert statements should not change a table's ownership

[yaooqinn]

What changes were proposed in this pull request?

In this change, we give preference to the original table's owner if it is not empty.

Why are the changes needed?

When executing 'insert into/overwrite ...' DML, or 'alter table set tblproperties ...' DDL, spark would change the ownership of the table the one who runs the spark application.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

Compare with the behavior of Apache Hive

[yaooqinn]

cc @wangyum

[wangyum]

retest this please

[SparkQA]

Test build #111952 has finished for PR 26068 at commit f408912.

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

[SparkQA]

Test build #111964 has finished for PR 26068 at commit a6721ca.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[wangyum]

Could we move this change to HiveClientImpl.scala#L1043?

Option(table.owner).filter(_.nonEmpty).orElse(userName).foreach(hiveTable.setOwner)

[yaooqinn]

It sounds like a bigger deal than it is. Indeed, I am going to add alter table owner syntax, which may make this kind of change back and forth

[HyukjinKwon]

Hm, yea I think @wangyum's suggestion is more correct.

[wangyum]

Thank you @HyukjinKwon Fixed by #26160.

[yaooqinn]

also cc @cloud-fan

[cloud-fan]

how is INSERT TABLE fixed in this PR?

[yaooqinn]

@cloud-fan

spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala

Line 112 in a45739d

CommandUtils.updateTableStats(sparkSession, table)

would eventually call alterTable here

[cloud-fan]

can we add a test in VersionsSuite?

[SparkQA]

Test build #112206 has finished for PR 26068 at commit a990a31.

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

[wangyum]

To clarify, this PR fixed the case of ALTER TABLE and INSERT TABLE, but we still have owner issues when creating table: SPARK-29498.

[SparkQA]

Test build #112215 has finished for PR 26068 at commit 10cb13b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #112214 has finished for PR 26068 at commit 0743ddd.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

thanks, merging to master!

[HyukjinKwon]

LGTM too, though.