xtern commented on code in PR #2800:
URL: https://github.com/apache/ignite-3/pull/2800#discussion_r1387814965
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -440,4 +426,69 @@ 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> addField(Object value) {
+ if (fieldIdx == 0 && data == null) {
+ data = new Object[schemaLen];
+ }
+
+ checkIndex();
+
+ data[fieldIdx++] = value;
+ return this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper build() {
+ checkState();
+
+ if (rowSchema.fields().isEmpty()) {
+ return ObjectsArrayRowWrapper.EMPTY_ROW;
+ } else {
+ Object[] row = data;
+ return new ObjectsArrayRowWrapper(rowSchema, row);
+ }
Review Comment:
```suggestion
return schemaLen == 0 ? EMPTY_ROW : new
ObjectsArrayRowWrapper(rowSchema, data);
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -186,15 +185,15 @@ public abstract static class RowWrapper {
abstract @Nullable Object get(int field);
- abstract void set(int field, @Nullable Object value);
-
abstract BinaryTuple toBinaryTuple();
}
/**
* Wrapper over an array of objects.
*/
private static class ObjectsArrayRowWrapper extends RowWrapper {
+ private static final ObjectsArrayRowWrapper EMPTY_ROW = new
ObjectsArrayRowWrapper(RowSchema.builder().build(), new Object[0]);
Review Comment:
I suggest moving it into RowBuilderImpl and replace
`ObjectsArrayRowWrapper.EMPTY_ROW` with `EMPTY_ROW`
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -104,4 +99,50 @@ 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 build() after all fields have been set.
+ * var row1 = rowBuilder.build();
+ * // Call reset() to cleanup builder's state.
+ * rowBuilder.reset();
+ * </pre>
+ */
+ interface RowBuilder<RowT> {
+
+ /**
+ * Adds a field to the current row.
+ *
+ * @param value Field value.
+ * @return this.
+ */
+ RowBuilder<RowT> addField(@Nullable Object value);
+
+ /** Creates a new row from a previously added fields. */
+ RowT build();
+
+ /**
+ * Resets the state of this builder.
+ */
+ void reset();
+
+ /**
+ * Creates a new row and resets the state of this builder. This is a
shorthand for:
+ * <pre>
+ * Row row = builder.build();
+ * builder.reset();
+ * return row;
+ * </pre>
+ */
+ default RowT buildAndReset() {
Review Comment:
Do we use in production code `build` and/or `reset`?
Can we remove `reset` and `buildAndReset` and keep single `build` method?
p.s. I checked such refactoring in tests and it seems to work well, maybe we
can get rid of unnecessary methods?
--
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
