fixup: review comments

Author: luohao

core/trino-server/src/main/provisio/trino.xml

@@ -157,6 +157,12 @@
         </artifact>
     </artifactSet>
 
+    <artifactSet to="plugin/lance">
+        <artifact id="${project.groupId}:trino-lance:zip:${project.version}">
+            <unpack />
+        </artifact>
+    </artifactSet>
+
     <artifactSet to="plugin/loki">
         <artifact id="${project.groupId}:trino-loki:zip:${project.version}">
             <unpack />

lib/trino-lance-file/pom.xml

@@ -13,6 +13,7 @@
     <description>Trino - Lance file format</description>
 
     <properties>
+        <air.compiler.fail-warnings>true</air.compiler.fail-warnings>
         <air.test.jvm.additional-arguments>${air.test.jvm.additional-arguments.default}
             --add-opens=java.base/java.nio=ALL-UNNAMED
             --sun-misc-unsafe-memory-access=allow</air.test.jvm.additional-arguments>
@@ -47,6 +48,7 @@
         <dependency>
             <groupId>io.github.luohao</groupId>
             <artifactId>fastlanes-java</artifactId>
+            <version>1</version>
         </dependency>
 
         <dependency>

lib/trino-lance-file/src/main/java/io/trino/lance/file/LanceReader.java

@@ -124,7 +124,7 @@ public LanceReader(LanceDataSource dataSource,
                         entry -> metadata.get(entry.getValue())));
 
         this.columnReaders = fields.stream()
-                .filter(field -> columnIds.contains(field.getId()))
+                .filter(field -> columnIds.contains(field.id()))
                 .map(field ->
                         ColumnReader.createColumnReader(
                                 dataSource,
@@ -149,7 +149,7 @@ public static List<Field> toFields(List<build.buf.gen.lance.file.Field> fieldsPr
         }
         List<Field> fields = new ArrayList<>();
         for (Map.Entry<Integer, Field> entry : fieldMap.entrySet()) {
-            int parentId = fieldMap.get(entry.getKey()).getParentId();
+            int parentId = fieldMap.get(entry.getKey()).parentId();
             Field field = entry.getValue();
             if (parentId == -1) {
                 fields.add(field);
@@ -299,7 +299,7 @@ public Block getBlock(int channel)
 
             Block block = blocks[channel];
             if (block == null) {
-                block = columnReaders[channel].read().getBlock();
+                block = columnReaders[channel].read().block();
                 block = selectedPositions.apply(block);
             }
             blocks[channel] = block;

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/metadata/AllNullLayout.java

@@ -13,18 +13,16 @@
  */
 package io.trino.lance.file.v2.metadata;
 
-import java.util.List;
+import com.google.common.collect.ImmutableList;
 
-import static java.util.Objects.requireNonNull;
+import java.util.List;
 
-public final class AllNullLayout
+public record AllNullLayout(List<RepDefLayer> layers)
         implements PageLayout
 {
-    public final List<RepDefLayer> layers;
-
     public AllNullLayout(List<RepDefLayer> layers)
     {
-        this.layers = requireNonNull(layers, "layers is null");
+        this.layers = ImmutableList.copyOf(layers);
     }
 
     public static AllNullLayout fromProto(build.buf.gen.lance.encodings21.AllNullLayout proto)

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/metadata/ColumnMetadata.java

@@ -23,19 +23,14 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static java.util.Objects.requireNonNull;
 
-public class ColumnMetadata
+public record ColumnMetadata(int index, List<PageMetadata> pages, List<DiskRange> bufferOffsets)
 {
-    private final int index;
-    private final List<PageMetadata> pages;
-    private final List<DiskRange> bufferOffsets;
-
     public ColumnMetadata(int index, List<PageMetadata> pages, List<DiskRange> bufferOffsets)
     {
         this.index = index;
-        this.pages = requireNonNull(pages, "pages is null");
-        this.bufferOffsets = requireNonNull(bufferOffsets, "bufferOffsets is null");
+        this.pages = ImmutableList.copyOf(pages);
+        this.bufferOffsets = ImmutableList.copyOf(bufferOffsets);
     }
 
     public static ColumnMetadata from(int columnIndex, Slice data)
@@ -88,19 +83,4 @@ private static PageLayout getPageLayout(build.buf.gen.lance.file.v2.ColumnMetada
             default -> throw new UnsupportedOperationException("Invalid encoding: " + encoding);
         };
     }
-
-    public int getIndex()
-    {
-        return index;
-    }
-
-    public List<PageMetadata> getPages()
-    {
-        return pages;
-    }
-
-    public List<DiskRange> getBufferOffsets()
-    {
-        return bufferOffsets;
-    }
 }

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/metadata/Field.java

@@ -39,16 +39,8 @@
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.util.Objects.requireNonNull;
 
-public class Field
+public record Field(String name, int id, int parentId, String logicalType, Map<String, String> metadata, boolean nullable, List<Field> children)
 {
-    private final String name;
-    private final int id;
-    private final int parentId;
-    private final String logicalType;
-    private final Map<String, String> metadata;
-    private final boolean nullable;
-    private final List<Field> children;
-
     @JsonCreator
     public Field(
             @JsonProperty("name") String name,
@@ -89,44 +81,51 @@ public void addChild(Field child)
         this.children.add(child);
     }
 
+    @Override
     @JsonProperty
-    public int getId()
+    public int id()
     {
         return id;
     }
 
+    @Override
     @JsonProperty
-    public String getName()
+    public String name()
     {
         return name;
     }
 
+    @Override
     @JsonProperty
-    public int getParentId()
+    public int parentId()
     {
         return parentId;
     }
 
+    @Override
     @JsonProperty
-    public String getLogicalType()
+    public String logicalType()
     {
         return logicalType;
     }
 
+    @Override
     @JsonProperty
-    public Map<String, String> getMetadata()
+    public Map<String, String> metadata()
     {
         return metadata;
     }
 
+    @Override
     @JsonProperty
-    public boolean isNullable()
+    public boolean nullable()
     {
         return nullable;
     }
 
+    @Override
     @JsonProperty
-    public List<Field> getChildren()
+    public List<Field> children()
     {
         return children;
     }

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/metadata/FullZipLayout.java

@@ -17,16 +17,10 @@
 
 import static java.util.Objects.requireNonNull;
 
-public final class FullZipLayout
+public record FullZipLayout(int numRepBits, int numDeflBits, io.trino.lance.file.v2.metadata.FullZipLayout.Block block, int numItems, int numVisibleItems,
+                            List<RepDefLayer> repDefLayers)
         implements PageLayout
 {
-    private final int numRepBits;
-    private final int numDeflBits;
-    private final Block block;
-    private final int numItems;
-    private final int numVisibleItems;
-    private final List<RepDefLayer> repDefLayers;
-
     public FullZipLayout(int numRepBits,
             int numDeflBits,
             Block block,
@@ -58,36 +52,6 @@ public static FullZipLayout fromProto(build.buf.gen.lance.encodings21.FullZipLay
                 RepDefLayer.fromProtoList(proto.getLayersList()));
     }
 
-    public int getNumRepBits()
-    {
-        return numRepBits;
-    }
-
-    public int getNumDeflBits()
-    {
-        return numDeflBits;
-    }
-
-    public Block getBlock()
-    {
-        return block;
-    }
-
-    public int getNumItems()
-    {
-        return numItems;
-    }
-
-    public int getNumVisibleItems()
-    {
-        return numVisibleItems;
-    }
-
-    public List<RepDefLayer> getRepDefLayers()
-    {
-        return repDefLayers;
-    }
-
     public sealed interface Block
             permits
             Block.FixedWidthBlock,

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/metadata/TypeUtil.java

@@ -47,7 +47,7 @@ public T primitive(Field primitive)
 
     public static <T> T visit(Field field, FieldVisitor<T> visitor)
     {
-        return switch (LogicalType.from(field.getLogicalType())) {
+        return switch (LogicalType.from(field.logicalType())) {
             case LogicalType.Int8Type _,
                  LogicalType.Int16Type _,
                  LogicalType.Int32Type _,
@@ -58,15 +58,15 @@ public static <T> T visit(Field field, FieldVisitor<T> visitor)
                  LogicalType.BinaryType _,
                  LogicalType.DateType _ -> visitor.primitive(field);
             case LogicalType.StructType _ -> {
-                List<T> results = new ArrayList<>(field.getChildren().size());
-                for (Field child : field.getChildren()) {
+                List<T> results = new ArrayList<>(field.children().size());
+                for (Field child : field.children()) {
                     results.add(visit(child, visitor));
                 }
                 yield visitor.struct(field, results);
             }
             case LogicalType.ListType _ -> {
-                verify(field.getChildren().size() == 1);
-                T result = visit(field.getChildren().get(0), visitor);
+                verify(field.children().size() == 1);
+                T result = visit(field.children().get(0), visitor);
                 yield visitor.list(field, result);
             }
             case LogicalType.FixedSizeListType _ -> throw new UnsupportedOperationException("FIXED LIST TYPES not yet supported");
@@ -87,7 +87,7 @@ public static class FieldIdToColumnIndexVisitor
         @Override
         public Map<Integer, Integer> primitive(Field primitive)
         {
-            return ImmutableMap.of(primitive.getId(), current++);
+            return ImmutableMap.of(primitive.id(), current++);
         }
 
         @Override

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/reader/ColumnReader.java

@@ -30,7 +30,7 @@ static ColumnReader createColumnReader(LanceDataSource dataSource,
             List<Range> readRanges,
             AggregatedMemoryContext memoryContext)
     {
-        return switch (LogicalType.from(field.getLogicalType())) {
+        return switch (LogicalType.from(field.logicalType())) {
             case LogicalType.Int8Type _,
                  LogicalType.Int16Type _,
                  LogicalType.Int32Type _,
@@ -39,10 +39,10 @@ static ColumnReader createColumnReader(LanceDataSource dataSource,
                  LogicalType.DoubleType _,
                  LogicalType.StringType _,
                  LogicalType.BinaryType _,
-                 LogicalType.DateType _ -> new PrimitiveColumnReader(dataSource, field, columnMetadata.get(field.getId()), readRanges, memoryContext);
+                 LogicalType.DateType _ -> new PrimitiveColumnReader(dataSource, field, columnMetadata.get(field.id()), readRanges, memoryContext);
             case LogicalType.ListType _ -> new ListColumnReader(dataSource, field, columnMetadata, readRanges, memoryContext);
             case LogicalType.StructType _ -> new StructColumnReader(dataSource, field, columnMetadata, readRanges, memoryContext);
-            default -> throw new RuntimeException("Unsupported logical type: " + field.getLogicalType());
+            default -> throw new RuntimeException("Unsupported logical type: " + field.logicalType());
         };
     }
 

lib/trino-lance-file/src/main/java/io/trino/lance/file/v2/reader/DecodedPage.java

@@ -17,24 +17,11 @@
 
 import static java.util.Objects.requireNonNull;
 
-public class DecodedPage
+public record DecodedPage(Block block, RepetitionDefinitionUnraveler unraveler)
 {
-    private final Block block;
-    private final RepetitionDefinitionUnraveler unraveler;
-
     public DecodedPage(Block block, RepetitionDefinitionUnraveler unraveler)
     {
         this.block = requireNonNull(block, "block is null");
         this.unraveler = requireNonNull(unraveler, "unraveler is null");
     }
-
-    public Block getBlock()
-    {
-        return block;
-    }
-
-    public RepetitionDefinitionUnraveler getUnraveler()
-    {
-        return unraveler;
-    }
 }