Support column properties with jdbc connector#26561
Conversation
|
@ebyhr @chenjian2664 Could you help review it? Thanks |
hqbhoho
requested review from
chenjian2664 and
ebyhr
and removed request for
chenjian2664
hqbhoho
force-pushed
the
feature/support_column_properties_with_jdbc_connector
branch
2 times, most recently
from
521ff3e to
cf6c266
Compare
hqbhoho
force-pushed
the
feature/support_column_properties_with_jdbc_connector
branch
2 times, most recently
from
64d84a9 to
9ce5fcb
Compare
Thank you for your feedback! Currently, the To address this, I will implement a validation check to prevent such table creation attempts and add corresponding test cases to ensure the functionality works as expected |
hqbhoho
force-pushed
the
feature/support_column_properties_with_jdbc_connector
branch
from
9ce5fcb to
aba59d3
Compare
hqbhoho
force-pushed
the
feature/support_column_properties_with_jdbc_connector
branch
from
aba59d3 to
b5c1436
Compare
| return List.of(); | ||
| } | ||
|
|
||
| default ColumnMetadata toColumnMetadata(JdbcColumnHandle columnHandle) |
There was a problem hiding this comment.
Since the getColumnProperties will hold column properties, is the change still necessary?
There was a problem hiding this comment.
I think directly passing columnProperties from JdbcColumnHandle to ColumnMetadata might not be appropriate. Could there be a scenario where some attributes stored in JdbcColumnHandle are not intended to be passed to ColumnMetadata? Adding a toColumnMetadata method on the client side would allow clients to define custom conversion logic.
In the current scenario, I want to add extraInfo to ColumnMetadata based on columnProperties.
|
|
||
| private static Map<String, Object> verifyColumnProperties(Map<String, Object> columnProperties) | ||
| { | ||
| if (columnProperties != null) { |
There was a problem hiding this comment.
Is the columnProperties nullable, when?
There was a problem hiding this comment.
No, it's not nullable when use builder() to new , but constructor method is public. I will add a check.
| } | ||
|
|
||
| @Override | ||
| public List<PropertyMetadata<?>> getColumnProperties() |
There was a problem hiding this comment.
Where is the usage of the method?
There was a problem hiding this comment.
In MySqlClientModule will inject ColumnPropertiesProvider with bindColumnPropertiesProvider(binder, MySqlColumnProperties.class);
ColumnPropertyManager#getProperties will be used when execute create table,add column or show table table and so on.
hqbhoho
force-pushed
the
feature/support_column_properties_with_jdbc_connector
branch
from
b5c1436 to
dbf53a8
Compare
hqbhoho
force-pushed
the
feature/support_column_properties_with_jdbc_connector
branch
from
dbf53a8 to
2f8ce8f
Compare
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
|
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time. |
3 participants
Description
ReOpening #25174 since it went stale.
Add column properties for base jdbc connector to support auto_increment and other column attribute.
When running SHOW CREATE TABLE include column properties present for these column attribute.
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: