review

vkorukanti · Feb 14, 2025 · 19ebaf6 · 19ebaf6
1 parent 3c7209e
commit 19ebaf6
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 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
@@ -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
@@ -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<TableFeature> 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