Add non-callback based entry builder for row and array block builders

[dain]

Description

There are cases where the row or array can not be completely build in a single
callback. For example, when remapping json paths to multiple columns,
the fields do not arrive in a single neat bundle.

Release notes

I'm not sure this is worth adding a release note.

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(X) Release notes are required, with the following suggested text:

## SPI
* Add non-callback based entry builder to RowBlockBuilder. ({issue}`issuenumber`)
* Add non-callback based entry builder to ArrayBlockBuilder. ({issue}`issuenumber`)

[sourcery-ai]

Reviewer's Guide

Introduces a non-callback based entry builder by defining a RowEntryBuilder interface and integrating it into RowBlockBuilder alongside existing callback-based API, updates tests to cover both construction styles and error handling, and adjusts Revapi config to suppress the new interface from API checks.

Class diagram for RowBlockBuilder and RowEntryBuilder changes

classDiagram
    class RowBlockBuilder {
        +buildEntry(RowValueBuilder<E> builder)
        +buildEntry() RowEntryBuilder
        -fieldBlockBuilders: BlockBuilder[]
        -currentEntryOpened: boolean
    }
    class RowEntryBuilder {
        +getFieldBuilder(int fieldId) BlockBuilder
        +build()
    }
    class RowEntryBuilderImplementation {
        +RowEntryBuilderImplementation()
        +getFieldBuilder(int fieldId) BlockBuilder
        +build()
    }
    RowBlockBuilder --> RowEntryBuilder
    RowEntryBuilder <|.. RowEntryBuilderImplementation
    RowEntryBuilderImplementation ..> BlockBuilder : uses
    RowBlockBuilder ..> BlockBuilder : uses

Loading

File-Level Changes

Change	Details	Files
Add non-callback entry builder support to RowBlockBuilder	Define RowEntryBuilder interface Add buildEntry() factory method Implement RowEntryBuilderImplementation inner class with getFieldBuilder and build() Enforce single open entry via currentEntryOpened flag	core/trino-spi/src/main/java/io/trino/spi/block/RowBlockBuilder.java core/trino-spi/src/main/java/io/trino/spi/block/RowEntryBuilder.java
Extend TestRowBlockBuilder with new entry builder tests	Add tests for callback-based buildEntry Add tests for non-callback buildEntry usage Verify IllegalStateException when building nested entries	core/trino-spi/src/test/java/io/trino/spi/block/TestRowBlockBuilder.java
Update Revapi configuration for new interface	Add ignore rule for RowEntryBuilder in Revapi differences	core/trino-spi/pom.xml

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `core/trino-spi/src/main/java/io/trino/spi/block/RowBlockBuilder.java:132-136` </location>
<code_context>
+        }
+
+        @Override
+        public BlockBuilder getFieldBuilder(int fieldId)
+        {
+            return fieldBlockBuilders[fieldId];
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Validate fieldId to prevent out-of-bounds access.

Add a bounds check for fieldId in getFieldBuilder to avoid IndexOutOfBoundsException and enhance robustness.

```suggestion
        @Override
        public BlockBuilder getFieldBuilder(int fieldId)
        {
            if (fieldId < 0 || fieldId >= fieldBlockBuilders.length) {
                throw new IndexOutOfBoundsException("fieldId " + fieldId + " is out of bounds (0, " + (fieldBlockBuilders.length - 1) + ")");
            }
            return fieldBlockBuilders[fieldId];
        }
```
</issue_to_address>

### Comment 2
<location> `core/trino-spi/src/main/java/io/trino/spi/block/RowBlockBuilder.java:138-143` </location>
<code_context>
+        }
+
+        @Override
+        public void build()
+        {
+            entryAdded(false);
</code_context>

<issue_to_address>
**suggestion:** Consider handling double build() calls gracefully.

Currently, repeated build() calls reset currentEntryOpened and trigger entryAdded(false) multiple times. Consider adding a guard to prevent unintended behavior from double builds.

```suggestion
        @Override
        public void build()
        {
            if (!currentEntryOpened) {
                // Already built, ignore repeated build() calls
                return;
            }
            entryAdded(false);
            currentEntryOpened = false;
        }
```
</issue_to_address>

### Comment 3
<location> `core/trino-spi/src/test/java/io/trino/spi/block/TestRowBlockBuilder.java:85-88` </location>
<code_context>
+        }
+        assertThat(blockToValues(blockBuilder.buildValueBlock())).isEqualTo(values);
+
+        blockBuilder = new RowBlockBuilder(List.of(VARCHAR, INTEGER, BOOLEAN), null, 1);
+        blockBuilder.buildEntry();
+        assertThatThrownBy(blockBuilder::buildEntry).isInstanceOf(IllegalStateException.class);
+    }
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for incomplete entry (no build call).

Add a test where buildEntry() is called without a subsequent build(), to confirm RowBlockBuilder's behavior with incomplete entries.

```suggestion
        blockBuilder = new RowBlockBuilder(List.of(VARCHAR, INTEGER, BOOLEAN), null, 1);
        blockBuilder.buildEntry();
        assertThatThrownBy(blockBuilder::buildEntry).isInstanceOf(IllegalStateException.class);

        // Test for incomplete entry (no build call)
        RowBlockBuilder incompleteEntryBuilder = new RowBlockBuilder(List.of(VARCHAR, INTEGER, BOOLEAN), null, 1);
        incompleteEntryBuilder.buildEntry();
        assertThatThrownBy(incompleteEntryBuilder::buildValueBlock)
                .isInstanceOf(IllegalStateException.class)
                .hasMessageContaining("Incomplete entry");
    }
```
</issue_to_address>

### Comment 4
<location> `core/trino-spi/src/test/java/io/trino/spi/block/TestRowBlockBuilder.java:71-83` </location>
<code_context>
+        assertThat(blockToValues(blockBuilder.buildValueBlock())).isEqualTo(values);
+
+        blockBuilder = new RowBlockBuilder(List.of(VARCHAR, INTEGER, BOOLEAN), null, 1);
+        for (TestRow row : values) {
+            RowEntryBuilder rowEntryBuilder = blockBuilder.buildEntry();
+            if (row.name() == null) {
+                rowEntryBuilder.getFieldBuilder(0).appendNull();
+            }
+            else {
+                VARCHAR.writeString(rowEntryBuilder.getFieldBuilder(0), row.name());
+            }
+            INTEGER.writeLong(rowEntryBuilder.getFieldBuilder(1), row.number());
+            BOOLEAN.writeBoolean(rowEntryBuilder.getFieldBuilder(2), row.flag());
+            rowEntryBuilder.build();
+        }
+        assertThat(blockToValues(blockBuilder.buildValueBlock())).isEqualTo(values);
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for out-of-bounds field access in RowEntryBuilder.

Add a test that calls getFieldBuilder with an invalid index (negative or >= number of fields) to verify that RowEntryBuilder throws the correct exception.

```suggestion
        for (TestRow row : values) {
            RowEntryBuilder rowEntryBuilder = blockBuilder.buildEntry();
            if (row.name() == null) {
                rowEntryBuilder.getFieldBuilder(0).appendNull();
            }
            else {
                VARCHAR.writeString(rowEntryBuilder.getFieldBuilder(0), row.name());
            }
            INTEGER.writeLong(rowEntryBuilder.getFieldBuilder(1), row.number());
            BOOLEAN.writeBoolean(rowEntryBuilder.getFieldBuilder(2), row.flag());
            rowEntryBuilder.build();

            // Test out-of-bounds field access
            assertThatThrownBy(() -> rowEntryBuilder.getFieldBuilder(-1))
                    .isInstanceOf(IndexOutOfBoundsException.class);
            assertThatThrownBy(() -> rowEntryBuilder.getFieldBuilder(3))
                    .isInstanceOf(IndexOutOfBoundsException.class);
        }
        assertThat(blockToValues(blockBuilder.buildValueBlock())).isEqualTo(values);
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}