Add ALTER TABLE SET PROPERTIES statement by ebyhr · Pull Request #9401 · trinodb/trino

ebyhr · 2021-09-27T13:50:06Z

Supersedes #8908

https://trinodb.slack.com/archives/CP1MUNEUX/p1631676807141200

findepi · 2021-09-27T20:31:56Z

Some properties are easy to change (e.g. format in case of Hive partitioned table).
In some cases, a property change is possible, but would require some computations (e.g. format in case of Hive unpartitioned table).

I wonder whether we should integrate this statement with ALTER TABLE .. EXECUTE work @ #9398 / #9466 cc @losipiuk

kokosing

skimmed

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java

core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java

core/trino-main/src/main/java/io/trino/server/QueryExecutionFactoryModule.java

core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java

core/trino-parser/src/test/java/io/trino/sql/parser/TestSqlParser.java

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java

losipiuk · 2021-10-04T07:27:51Z

Some properties are easy to change (e.g. format in case of Hive partitioned table). In some cases, a property change is possible, but would require some computations (e.g. format in case of Hive unpartitioned table).

I wonder whether we should integrate this statement with ALTER TABLE .. EXECUTE work @ #9398 / #9466 cc @losipiuk

Eventually we may want to decide to share execution logic for such. But I would start with low hanging fruits, and just support "simple" properties in ALTER TABLE SET PROPERTIES so we are not overthinking too much and slow things down.

sopel39

lgtm % comments

core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java

core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java

core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java

core/trino-parser/src/main/java/io/trino/sql/SqlFormatter.java

core/trino-parser/src/test/java/io/trino/sql/parser/TestSqlParser.java

...toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java

core/trino-main/src/main/java/io/trino/sql/analyzer/FeaturesConfig.java

core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java

core/trino-main/src/main/java/io/trino/security/AccessControl.java

core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java

sopel39

lgtm % comments

core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java

core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java

core/trino-main/src/main/java/io/trino/sql/analyzer/FeaturesConfig.java

core/trino-main/src/test/java/io/trino/connector/MockConnector.java

core/trino-main/src/main/java/io/trino/security/AccessControl.java

core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java

ebyhr · 2021-10-13T00:47:00Z

CI #8691 and #9612

ebyhr · 2021-10-13T05:57:16Z

@sopel39 Applied comments.

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java

cla-bot bot added the cla-signed label Sep 27, 2021

kokosing reviewed Sep 28, 2021

View reviewed changes

ebyhr force-pushed the ebi/alter-table-properties branch 5 times, most recently from 935c407 to c5dcb74 Compare October 4, 2021 06:04

ebyhr marked this pull request as ready for review October 4, 2021 06:04

ebyhr requested a review from sopel39 October 4, 2021 06:05

ebyhr force-pushed the ebi/alter-table-properties branch 2 times, most recently from 729ecee to 658f2f5 Compare October 8, 2021 01:09

sopel39 reviewed Oct 8, 2021

View reviewed changes

sopel39 reviewed Oct 11, 2021

View reviewed changes

core/trino-main/src/main/java/io/trino/metadata/AbstractPropertyManager.java Outdated Show resolved Hide resolved

core/trino-main/src/main/java/io/trino/security/AccessControl.java Outdated Show resolved Hide resolved

ebyhr force-pushed the ebi/alter-table-properties branch 2 times, most recently from 4a03c7f to 0800d4f Compare October 12, 2021 02:20

ebyhr requested a review from sopel39 October 12, 2021 06:32

sopel39 reviewed Oct 12, 2021

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java Outdated Show resolved Hide resolved

sopel39 reviewed Oct 12, 2021

View reviewed changes

core/trino-main/src/main/java/io/trino/security/AccessControl.java Outdated Show resolved Hide resolved

core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java Outdated Show resolved Hide resolved

sopel39 approved these changes Oct 13, 2021

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/ForwardingJdbcClient.java Outdated Show resolved Hide resolved

sopel39 mentioned this pull request Oct 13, 2021

Support ALTER MATERIALIZED VIEW ... SET PROPERTIES ... #9613

Merged

ebyhr added 3 commits October 13, 2021 22:22

Add ALTER TABLE SET PROPERTIES statement

72df2b9

Check SET PROPERTIES permission in CREATE TABLE task

fb3928e

Implement ALTER TABLE SET PROPERTIES in ClickHouse

cf368de

ebyhr force-pushed the ebi/alter-table-properties branch from 4e1693b to cf368de Compare October 13, 2021 13:38

ebyhr merged commit a50f16e into trinodb:master Oct 14, 2021

ebyhr deleted the ebi/alter-table-properties branch October 14, 2021 00:52

github-actions bot added this to the 364 milestone Oct 14, 2021

ebyhr mentioned this pull request Oct 14, 2021

Release notes for 364 #9534

Closed

12 tasks

findepi mentioned this pull request Feb 3, 2022

Authorize table parameters in CTAS and remove deprecated check methods #10939

Merged

pratyakshsharma mentioned this pull request Dec 7, 2023

Add ALTER TABLE SET PROPERTIES statement prestodb/presto#21495

Merged

6 tasks

Comments

Conversation

ebyhr commented Sep 27, 2021

Uh oh!

findepi commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kokosing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

losipiuk commented Oct 4, 2021

Uh oh!

sopel39 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sopel39 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebyhr commented Oct 13, 2021

Uh oh!

ebyhr commented Oct 13, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

findepi commented Sep 27, 2021 •

edited

Loading