Add support for changing Iceberg table properties

[findinpath]

Syntax for changing Iceberg table properties

ALTER TABLE an-iceberg-table SET PROPERTIES a-property-name = 'a-value'

Implementation tips

Implement the method io.trino.spi.connector.ConnectorMetadata#setTableProperties in io.trino.plugin.iceberg.IcebergMetadata.

Use org.apache.iceberg.BaseTable#updateProperties for updating table properties.

Test tips

Add a integration test in io.trino.plugin.iceberg.BaseIcebergConnectorTest for testing the functionality. Use https://trino.io/docs/current/connector/iceberg.html#properties-table for reading the properties of the Iceberg table.

Add a product test to the class io.trino.tests.product.iceberg.TestIcebergSparkCompatibility for checking the compatibility with Spark on updating table properties.

[puchengy]

This is very useful, thanks!

[findepi]

Use org.apache.iceberg.BaseTable#updateProperties for updating table properties.

ALTER TABLE SET PROPERTIES should change Trino Iceberg table properties (IcebergTableProperties).
It's not supposed to be a pass-thru for Iceberg properties.

From Trino Iceberg table properties (IcebergTableProperties) perspective, we want to allow changing of partitioning field.

cc @electrum @phd3 @ebyhr @alexjo2144

[osscm]

@findepi thanks for the detail!
As you mentioned that until a property is not part of the IcebergTableProperties we should not allow setting/adding random properties. So, this issue, if needed, can only tackle the updates to the partitioning field.

There was another #10480 (comment) which seems to be looking for setting new properties to the icebergTable (pass-through)
And it was discussed in this thread.

@findinpath If I understood correctly, this issue was to allow the random properties can be set to the underlying iceberg table as pass-through only?

[findinpath]

@osscm the scenario which I had in mind revolved around changing custom / random properties.

@findepi what would be the appropriate syntax in your opinion for changing custom table properties?

[findepi]

Same as in Hive -- i.e. to be figured out; #954 (comment)

[Aashishthakur10]

@findepi @findinpath I was looking for a good first issue but don't have any background in Iceberg. It also looks like there are things that have to be figured out here. Is it still a good first issue that I can take up? For some background on this if you guys have any recommendations I'd be happy to go through them first!

[findinpath]

I'm removing the good first issue label because it is not clear yet how to expose this functionality.

[Aashishthakur10]

Makes sense, thanks!

[osscm]

@findepi is it ok to take the approach of adding extra_properties as you mentioned in the hive #954 (comment) related issue.
This can allow users for set the pass-through properties?

I believe we are waiting for final decision on this hive's PR #9475 as well?

do we need to take more blessings to go ahead, please guide, thanks.

@Aashishthakur10, Sorry missed your comment. Actually, I was working on it, but as @findinpath mentioned it was not clear, so didnt make any progress.

we also discussed in this thread: https://trinodb.slack.com/archives/CJ6UC075E/p1645767604315919?thread_ts=1645621806.119119&cid=CJ6UC075E

[Aashishthakur10]

No worries @osscm, that makes sense. Thanks for letting me know!

[findepi]

@findinpath let's close this ticket, it's too generic.
let's have a dedicated ticket to allow changing table format version, i.e. allow upgrading tables from v1 to v2: #12138

cc @alexjo2144 @ebyhr @homar

[findinpath]

let's have a dedicated ticket to allow changing table format version, i.e. allow upgrading tables from v1 to v2: #12138

The implementation details for #12138 would fit then the #11190 (comment) comment.

@findepi I'm fine with closing this ticket. However there seems to be an actual need for being able to set arbitrary table properties in Hive / Iceberg / Delta? via Trino. Should we continue the discussion on #954 ?

[findepi]

#954 remains desirable.
Yes, we may need to have something similar for Iceberg too, but this issue didn't sound to me like "#954 but for Iceberg".

[C-h-e-r-r-y]

@findinpath

Could you please clarify why did you close this ticket? I understand that there is a prehook with alter table - but as I understand it will be called every time the model executed. Looks like creating a table with properties more appropriate solution, no?

[findinpath]

@C-h-e-r-r-y pls follow the issue #17427
This is the agreed syntax on setting/changing non-canonical Iceberg table properties.