Remove PageBuilder newPageBuilderLike

This method only has one usage and the existing reset method is better
miqdigital · Oct 17, 2024 · 8e070d6 · 8e070d6
1 parent a619fda
commit 8e070d6
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 23 deletions.
diff --git a/core/trino-main/src/test/java/io/trino/block/TestBlockBuilder.java b/core/trino-main/src/test/java/io/trino/block/TestBlockBuilder.java
@@ -25,6 +25,7 @@
 
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.IntStream;
 
 import static io.trino.block.BlockAssertions.assertBlockEquals;
 import static io.trino.spi.type.BigintType.BIGINT;
@@ -72,13 +73,15 @@ public void testNewBlockBuilderLike()
             pageBuilder.declarePosition();
         }
 
-        PageBuilder newPageBuilder = pageBuilder.newPageBuilderLike();
+        List<BlockBuilder> originalBlockBuilders = IntStream.range(0, channels.size()).mapToObj(pageBuilder::getBlockBuilder).toList();
+        pageBuilder.reset();
         for (int i = 0; i < channels.size(); i++) {
             // we should get new block builder instances
-            assertThat(pageBuilder.getBlockBuilder(i))
-                    .isNotEqualTo(newPageBuilder.getBlockBuilder(i));
-            assertThat(newPageBuilder.getBlockBuilder(i).getPositionCount()).isEqualTo(0);
-            assertThat(newPageBuilder.getBlockBuilder(i).getRetainedSizeInBytes() < pageBuilder.getBlockBuilder(i).getRetainedSizeInBytes()).isTrue();
+            assertThat(originalBlockBuilders.get(i))
+                    .isNotEqualTo(pageBuilder.getBlockBuilder(i));
+            assertThat(pageBuilder.getBlockBuilder(i).getPositionCount()).isEqualTo(0);
+            assertThat(pageBuilder.getBlockBuilder(i).getSizeInBytes()).isLessThan(originalBlockBuilders.get(i).getSizeInBytes());
+            assertThat(pageBuilder.getBlockBuilder(i).getRetainedSizeInBytes()).isLessThan(originalBlockBuilders.get(i).getRetainedSizeInBytes());
         }
     }
 

diff --git a/core/trino-spi/pom.xml b/core/trino-spi/pom.xml
@@ -342,6 +342,10 @@
                                     <old>method io.trino.spi.block.BlockBuilder io.trino.spi.type.UuidType::createBlockBuilder(io.trino.spi.block.BlockBuilderStatus, int, int)</old>
                                     <new>method io.trino.spi.block.BlockBuilder io.trino.spi.type.Type::createBlockBuilder(io.trino.spi.block.BlockBuilderStatus, int, int) @ io.trino.spi.type.UuidType</new>
                                 </item>
+                                <item>
+                                    <code>java.method.removed</code>
+                                    <old>method io.trino.spi.PageBuilder io.trino.spi.PageBuilder::newPageBuilderLike()</old>
+                                </item>
                             </differences>
                         </revapi.differences>
                     </analysisConfiguration>

diff --git a/core/trino-spi/src/main/java/io/trino/spi/PageBuilder.java b/core/trino-spi/src/main/java/io/trino/spi/PageBuilder.java
@@ -71,17 +71,6 @@ private PageBuilder(int initialExpectedEntries, int maxPageBytes, List<? extends
         }
     }
 
-    private PageBuilder(int maxPageBytes, BlockBuilder[] templateBlockBuilders)
-    {
-        pageBuilderStatus = new PageBuilderStatus(maxPageBytes);
-
-        // Stream API should not be used since constructor can be called in performance sensitive sections
-        blockBuilders = new BlockBuilder[templateBlockBuilders.length];
-        for (int i = 0; i < this.blockBuilders.length; i++) {
-            this.blockBuilders[i] = templateBlockBuilders[i].newBlockBuilderLike(pageBuilderStatus.createBlockBuilderStatus());
-        }
-    }
-
     public void reset()
     {
         if (isEmpty()) {
@@ -96,11 +85,6 @@ public void reset()
         }
     }
 
-    public PageBuilder newPageBuilderLike()
-    {
-        return new PageBuilder(pageBuilderStatus.getMaxPageSizeInBytes(), blockBuilders);
-    }
-
     public BlockBuilder getBlockBuilder(int channel)
     {
         return blockBuilders[channel];

diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/line/LinePageSource.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/line/LinePageSource.java
@@ -41,7 +41,7 @@ public class LinePageSource
     private final LineBuffer lineBuffer;
     private final Location filePath;
 
-    private PageBuilder pageBuilder;
+    private final PageBuilder pageBuilder;
     private long completedPositions;
 
     public LinePageSource(LineReader lineReader, LineDeserializer deserializer, LineBuffer lineBuffer, Location filePath)
@@ -63,7 +63,7 @@ public Page getNextPage()
             }
             Page page = pageBuilder.build();
             completedPositions += page.getPositionCount();
-            pageBuilder = pageBuilder.newPageBuilderLike();
+            pageBuilder.reset();
             return page;
         }
         catch (TrinoException e) {