Skip to content

Conversation

@Peng-Lei
Copy link
Contributor

What changes were proposed in this pull request?

Take external as a reserved table property. and do not allow use external for end-user when spark.sql.legacy.notReserveProperties == false.

Why are the changes needed?

#disscuss.
keep it consistent with other properties like location owner provider and so on.

Does this PR introduce any user-facing change?

Yes. end-user could not use external as property key when create table with tblproperties and alter table set tblproperties.

How was this patch tested?

existed testcase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Since Spark 3.3, the property `external` become reserved for table; commands fail if you specify `external` property in places like `CREATE TABLE ... TBLPROPERTIES` and `ALTER TABLE ... SET TBLPROPERTIES`. You can set `spark.sql.legacy.notReserveProperties` to `true` to ignore the `ParseException`, in this case, `external` will be silently removed.
- Since Spark 3.3, the table property `external` becomes reserved. Certain commands will fail if you specify the `external` property, such as `CREATE TABLE ... TBLPROPERTIES` and `ALTER TABLE ... SET TBLPROPERTIES`. In Spark 3.2 and earlier, the table property `external` is silently ignored. You can set `spark.sql.legacy.notReserveProperties` to `true` to restore the old behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PROP_EXTERNAL, ctx, "it will be set according the location")
PROP_EXTERNAL, ctx, "please use CREATE EXTERNAL TABLE")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 'External'='bar' still works?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. 'External'='bar' still works. EXTERNAL is the key word of the hive metastore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then let's use External and keep this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 8ff48b3 Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants