Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Hive table property to for arbitrary properties #954

Closed
dain opened this issue Jun 10, 2019 · 5 comments · Fixed by #17172
Closed

Add Hive table property to for arbitrary properties #954

dain opened this issue Jun 10, 2019 · 5 comments · Fixed by #17172

Comments

@dain
Copy link
Member

dain commented Jun 10, 2019

Currently only table properties explicitly listed HiveTableProperties are supported in Presto, but many Hive environments use extended properties for administration.

Add a property named extra_properties of type MAP(VARCHAR, VARCHAR).

On read (e.g. SHOW CREATE TABLE) will show only the properties not mapped to existing table properties, and properties created by presto such as presto_version and presto_query_id.

On write, these properties are merged with the other properties, and if there are duplicates and error is thrown.

This is equivalent of Hive's TBLPROPERTIES.

@ankitdixit
Copy link
Member

@dain Can you please help me understand why we do not want to show properties mapped to existing table properties?

@dain
Copy link
Member Author

dain commented Jul 23, 2019

@dain Can you please help me understand why we do not want to show properties mapped to existing table properties?

I believe it would be confusing to users if the a property was presented in two different ways. I expect this would raise a lot of questions about which one is supposed to be used, and what happens on conflicts. Also, things like "I only set X and now I see X and Y".

In general, I see this feature as an "escape hatch" for cases when we don't directly support a standard property, or there the user has a custom property in their environment, but I want to encourage the use of the Presto property system because it is safer for end users to use due to the type safety of the syntax and the property specific validation code we have in some cases.

@ankitdixit
Copy link
Member

@dain Please have a look at the initial WIP pr, i am able to take input and store map but while visiting in ShowCreateTable , we have to convert map into an expression, which it seems is not supported as of yet.
One workaround could be to create a String out of map and then convert that to expression.
@Praveen2112 pointed out prestodb/presto#5065, adding literal type for map would inherently solve this problem.
Need your inputs on which way to approach.

@findepi
Copy link
Member

findepi commented Feb 28, 2022

@posulliv has #9475 open for this
and @dain has #9523

should we have discussion about way forward?
if it was for me to decide, i would just go with adding extra_properties property, so i personally don't need a discussion :)

@Chaho12
Copy link
Member

Chaho12 commented Mar 31, 2023

Any plans on developing this? For iceberg, we need to set properties to clean data files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment