From 19ebaf61741f07792752a7b05b18a6e4ea0592ad Mon Sep 17 00:00:00 2001 From: Venki Korukanti Date: Thu, 13 Feb 2025 17:44:31 -0800 Subject: [PATCH] review --- .../internal/tablefeatures/TableFeature.java | 4 +- .../internal/tablefeatures/TableFeatures.java | 40 +++++-------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeature.java b/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeature.java index 182a3248d5..65bd843634 100644 --- a/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeature.java +++ b/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeature.java @@ -141,8 +141,8 @@ public boolean hasKernelReadSupport() { } /** - * Does Kernel has support to write a table containing this feature? Default implementation * - * returns true. Features should override this method if they have special requirements * or not + * Does Kernel has support to write a table containing this feature? Default implementation + * returns true. Features should override this method if they have special requirements or not * supported by the Kernel yet. * * @param metadata the metadata of the table. Sometimes checking the metadata is necessary to know diff --git a/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeatures.java b/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeatures.java index 8dec9cad85..0ac22d5d72 100644 --- a/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeatures.java +++ b/kernel/kernel-api/src/main/java/io/delta/kernel/internal/tablefeatures/TableFeatures.java @@ -111,16 +111,6 @@ private static class ColumnMappingFeature extends TableFeature.LegacyReaderWrite public boolean metadataRequiresFeatureToBeEnabled(Protocol protocol, Metadata metadata) { return TableConfig.COLUMN_MAPPING_MODE.fromMetadata(metadata) != NONE; } - - @Override - public boolean hasKernelReadSupport() { - return true; - } - - @Override - public boolean hasKernelWriteSupport(Metadata metadata) { - return true; - } } public static final TableFeature IDENTITY_COLUMNS_W_FEATURE = new IdentityColumnsFeature(); @@ -145,7 +135,7 @@ public boolean metadataRequiresFeatureToBeEnabled(Protocol protocol, Metadata me public static final TableFeature VARIANT_RW_PREVIEW_FEATURE = new VariantTypeTableFeature("variantType-preview"); - static class VariantTypeTableFeature extends TableFeature.ReaderWriterFeature + private static class VariantTypeTableFeature extends TableFeature.ReaderWriterFeature implements FeatureAutoEnabledByMetadata { VariantTypeTableFeature(String featureName) { super( @@ -188,28 +178,21 @@ public boolean metadataRequiresFeatureToBeEnabled(Protocol protocol, Metadata me public Set requiredFeatures() { return Collections.singleton(DOMAIN_METADATA_W_FEATURE); } - - @Override - public boolean hasKernelWriteSupport(Metadata metadata) { - return true; - } } public static final TableFeature DELETION_VECTORS_RW_FEATURE = new DeletionVectorsTableFeature(); + /** + * Kernel currently only support blind appends. So we don't need to do anything special for + * writing into a table with deletion vectors enabled (i.e a table feature with DV enabled is both + * readable and writable. + */ private static class DeletionVectorsTableFeature extends TableFeature.ReaderWriterFeature implements FeatureAutoEnabledByMetadata { DeletionVectorsTableFeature() { super("deletionVectors", /* minReaderVersion = */ 3, /* minWriterVersion = */ 7); } - @Override - public boolean hasKernelWriteSupport(Metadata metadata) { - // We currently only support blind appends. So we don't need to do anything special for - // writing into a table with deletion vectors enabled. - return true; - } - @Override public boolean metadataRequiresFeatureToBeEnabled(Protocol protocol, Metadata metadata) { return TableConfig.DELETION_VECTORS_CREATION_ENABLED.fromMetadata(metadata); @@ -292,19 +275,16 @@ public boolean metadataRequiresFeatureToBeEnabled(Protocol protocol, Metadata me public static final TableFeature CHECKPOINT_V2_RW_FEATURE = new CheckpointV2TableFeature(); + /** + * In order to commit, there is no extra work required when v2 checkpoint is enabled. This affects + * the checkpoint format only. When v2 is enabled, writing classic checkpoints is still allowed. + */ private static class CheckpointV2TableFeature extends TableFeature.ReaderWriterFeature implements FeatureAutoEnabledByMetadata { CheckpointV2TableFeature() { super("v2Checkpoint", /* minReaderVersion = */ 3, /* minWriterVersion = */ 7); } - @Override - public boolean hasKernelWriteSupport(Metadata metadata) { - return true; // In order to commit, there is no extra work required. This affects - // the checkpoint format only. When v2 is enabled, writing classic checkpoints is - // still allowed. - } - @Override public boolean metadataRequiresFeatureToBeEnabled(Protocol protocol, Metadata metadata) { // TODO: define an enum for checkpoint policy when we start supporting writing v2 checkpoints