korlov42 commented on code in PR #2800: URL: https://github.com/apache/ignite-3/pull/2800#discussion_r1386132964
########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java: ########## @@ -104,4 +99,39 @@ interface RowFactory<RowT> { */ RowT create(InternalTuple tuple); } + + /** + * A builder to create rows. It uses the schema provided by an instance of row factory that created it. + * + * <pre> + * // Create a row builder + * var rowBuilder = rowFactory.rowBuilder(); + * + * // Call newRow() to start construction of a new row. + * rowBuilder.newRow(); + * ... + * // Call build() after all fields have been set. + * var row1 = rowBuilder.build(); + * </pre> + */ + interface RowBuilder<RowT> { + + /** + * Initiates construction of a new row. This method should be called prior to the first call to {@link #addField(Object)}. + * + * @return this. + */ + RowBuilder<RowT> newRow(); + + /** + * Add a field to the current row. + * + * @param value Field value. + * @return this. + */ + RowBuilder<RowT> addField(Object value); Review Comment: let's mark `value` as `@Nullable` ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java: ########## @@ -440,4 +424,56 @@ RowSchema rowSchema() { } } } + + private static class RowBuilderImpl implements RowBuilder<RowWrapper> { + + private final int schemaLen; + + private final RowSchema rowSchema; + + Object[] data; + + int fieldIdx; + + RowBuilderImpl(RowSchema rowSchema) { + this.rowSchema = rowSchema; + this.schemaLen = rowSchema.fields().size(); + fieldIdx = 0; + } + + /** {@inheritDoc} */ + @Override + public RowBuilder<RowWrapper> newRow() { Review Comment: nitpicking: for me, a bit natural seems to have following set of methods for builder: `addField`, `build`, and `reset`. Just created builder is always initialised and ready to accept next fields (no need to check state). You may create several similar rows by invoking `build` method consequently (invocation of `build` method does not reset builder's state). If you want to clear state of a builder -- just invoke `reset` ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java: ########## @@ -34,14 +34,6 @@ public interface RowHandler<RowT> { */ @Nullable Object get(int field, RowT row); - /** Set incoming row field. - * - * @param field Field position to be processed. - * @param row Row which field need to be changed. - * @param val Value which should be set. - */ - void set(int field, RowT row, @Nullable Object val); Review Comment: class-level javadoc need to be adjusted (it says `Universal accessor and *mutator* for rows`) ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java: ########## @@ -226,8 +234,7 @@ private static boolean hasConvertableFields(RelDataType resultType) { } /** - * ToInternal. - * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859 + * ToInternal. Converts the given value to its presentation used by the execution engine. Review Comment: ```suggestion * Converts the given value to its presentation used by the execution engine. ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org