Skip unsupported variant type in Delta Lake #22310
Conversation
ebyhr
force-pushed
the
ebi/delta-variant
branch
2 times, most recently
from
fecd850 to
c02b7b0
Compare
findinpath
requested review from
findepi and
alexjo2144
and removed request for
alexjo2144
...elta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/DeltaLakeSchemaSupport.java
Show resolved
Hide resolved
| columns.add(mapColumn(typeManager, nodes.next(), mappingMode)); | ||
| } | ||
| catch (UnsupportedTypeException e) { | ||
| log.debug("Skip unsupported column type: %s", e.type()); |
There was a problem hiding this comment.
We can ignore on read, but can't ignore on writes, if the column has constraints. Which we won't know if we skip it completely.
What about adding a direct support for variant instead, mapping it to json?
There was a problem hiding this comment.
The write operations are protected by the unsupported writer feature variantType. Why are we caring about this column skipping logic?
There was a problem hiding this comment.
We can ignore on read, but can't ignore on writes
I haven't seen yet this "best effort" practice.
I find it misleading from user perspective to see only parts of the columns.
At best, such a functionality should be enabled explicitly by the user through a session property.
This strategy is however not worth the effort.
I'm advocating either for supporting reading variants and succeed the read operation or fail the read operation otherwise.
There was a problem hiding this comment.
@findinpath Skipping unsupported column types is the default behavior in the existing connectors.
I can't find a reason we want to change the stance only in Delta Lake connector.
...elta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/DeltaLakeSchemaSupport.java
Show resolved
Hide resolved
| columns.add(mapColumn(typeManager, nodes.next(), mappingMode)); | ||
| } | ||
| catch (UnsupportedTypeException e) { | ||
| log.debug("Skip unsupported column type: %s", e.type()); |
| public static final String VARIANT_TYPE_FEATURE_NAME = "variantType"; | ||
| public static final String VARIANT_TYPE_PREVIEW_FEATURE_NAME = "variantType-preview"; |
There was a problem hiding this comment.
This feature is still in closed source in Delta right?
I don't see it in https://github.com/delta-io/delta/blob/master/PROTOCOL.md#valid-feature-names-in-table-features
Landing soon https://github.com/delta-io/delta/releases/tag/v4.0.0-preview-rc2
There was a problem hiding this comment.
Right. We should add a product test once we get the environment. #22342 is the follow-up issue.
ebyhr
force-pushed
the
ebi/delta-variant
branch
2 times, most recently
from
a574ac5 to
50856d8
Compare
Description
Skip unsupported variant type in Delta Lake.
Databricks doesn't allow the type in partition columns.
Release notes
(x) Release notes are required, with the following suggested text: