-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Support creating Delta tables with column mapping mode #17565
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
Conversation
23d28c4
to
bacd0ba
Compare
9de4af1
to
a36356e
Compare
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableProperties.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Show resolved
Hide resolved
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeColumnMapping.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % UX failure messages
a36356e
to
c55b7a4
Compare
...n/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/MetadataEntry.java
Outdated
Show resolved
Hide resolved
...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java
Show resolved
Hide resolved
core/trino-spi/src/main/java/io/trino/spi/session/PropertyMetadata.java
Outdated
Show resolved
Hide resolved
core/trino-spi/src/main/java/io/trino/spi/session/PropertyMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableProperties.java
Outdated
Show resolved
Hide resolved
Object serializedType = serializeColumnType(columnMappingMode, fieldId, column.getType()); | ||
Type physicalType = deserializeType(typeManager, serializedType, usePhysicalName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks weird to serializeColumnType
only to deserializeType
in the next line.
can this be simplified?
also, why serializeColumnType
doesn't need usePhysicalName
?
the input, column.getType()
, doesn't have physical and logical names, so how are .../metadata/delta.columnMapping.physicalName
and .../metadata/name
populated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be simplified?
That's possible, but not so simple. Let me handle in follow-up.
also, why serializeColumnType doesn't need usePhysicalName?
The method uses ColumnMappingMode
argument instead of the flag. I passed the mode because it affects not only delta.columnMapping.physicalName
but also delta.columnMapping.id
.
how are .../metadata/delta.columnMapping.physicalName and .../metadata/name populated?
delta.columnMapping.physicalName
is populated in serializeStructType → generateColumnMetadata in case of id and name modename
is always populated in serializeStructType → serializeStructField
...n/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/MetadataEntry.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableProperties.java
Show resolved
Hide resolved
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java
Show resolved
Hide resolved
This method tried to use 'hive' catalog by default.
Ran CI with the internal environment. |
Description
Support creating Delta tables with column mapping mode
Fixes #12638
Release notes
(x) Release notes are required, with the following suggested text: