datastrato / gravitino

World's most powerful open data catalog for building a high-performance, geo-distributed and federated metadata lake.
https://datastrato.ai/docs/
Apache License 2.0
616 stars 193 forks source link

[#804] feat(mysql): Support mysql table properties. #1310

Closed Clearvive closed 4 months ago

Clearvive commented 5 months ago

What changes were proposed in this pull request?

Support mysql table properties.

Why are the changes needed?

Fix: #804

Does this PR introduce any user-facing change?

Add mysql properties support.

How was this patch tested?

UT/IT

Clearvive commented 5 months ago

@mchades @FANNG1 Can you help me review it?

Clearvive commented 5 months ago

@jerryshao Can you help me review it?

FANNG1 commented 5 months ago

could you explain the reason why add ENGINE, DEFAULT CHARSET, COLLATE and AUTO_INCREMENT to Mysql table properties meta to be managed by Gravitino.

FANNG1 commented 5 months ago

do you support change table properies or remove table properties ?

Clearvive commented 5 months ago

could you explain the reason why add ENGINE, DEFAULT CHARSET, COLLATE and AUTO_INCREMENT to Mysql table properties meta to be managed by Gravitino.

It's solely because these four attributes are common and frequently used values in production.

Clearvive commented 5 months ago

e

image

Not supported

FANNG1 commented 5 months ago

could you explain the reason why add ENGINE, DEFAULT CHARSET, COLLATE and AUTO_INCREMENT to Mysql table properties meta to be managed by Gravitino.

It's solely because these four attributes are common and frequently used values in production.

I don't see a strong reason, especially for DEFAULT CHARSET COLLATE, suggest to explain more in Why the changes are needed

FANNG1 commented 5 months ago

e

image Not supported

seems we should support change table properites ?

Clearvive commented 5 months ago

could you explain the reason why add ENGINE, DEFAULT CHARSET, COLLATE and AUTO_INCREMENT to Mysql table properties meta to be managed by Gravitino.

It's solely because these four attributes are common and frequently used values in production.

I don't see a strong reason, especially for DEFAULT CHARSET COLLATE, suggest to explain more in Why the changes are needed

Explicitly setting the default character set ensures that all character data in the table uses the same character encoding. This helps to avoid issues such as garbled characters or inconsistencies that may arise when using different character encodings in a mixed environment.

Clearvive commented 5 months ago

e

image Not supported

seems we should support change table properites ? We support modifying table properties

image
FANNG1 commented 4 months ago

I failed to retrive the properies from JDBC drivers, but instead of querying data from internal system table, I prefer to use SHOW TABLE STATUS WHERE Name = 'xx' to get the properties, seems more maintanable.

FANNG1 commented 4 months ago

LGTM, except the few comments

Clearvive commented 4 months ago

LGTM, except the few comments

@jerryshao Can you help review it?

yuqi1129 commented 4 months ago

It's okay for me and have not further comments.

jerryshao commented 4 months ago

@FANNG1 @yuqi1129 you can go ahead if you feel OK.

Besides, we need to also update the doc @Clearvive