[GitHub] [ignite-3] ptupitsyn commented on a change in pull request #282: IGNITE-15253 Tuple API improvement

[GitBox <gi...@apache.org>]

ptupitsyn commented on a change in pull request #282:
URL: https://github.com/apache/ignite-3/pull/282#discussion_r692969801



##########
File path: modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java
##########
@@ -15,117 +15,102 @@
  * limitations under the License.
  */
 
-package org.apache.ignite.internal.table;
+package org.apache.ignite.table;
 
+import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Objects;
 import java.util.UUID;
 import org.apache.ignite.binary.BinaryObject;
-import org.apache.ignite.binary.BinaryObjects;
-import org.apache.ignite.internal.schema.Column;
-import org.apache.ignite.internal.schema.SchemaAware;
-import org.apache.ignite.internal.schema.SchemaDescriptor;
-import org.apache.ignite.table.Tuple;
-import org.apache.ignite.table.TupleBuilder;
 import org.jetbrains.annotations.NotNull;
 
 /**
- * Buildable tuple.
+ * Simple tuple implementation.
  */
-public class TupleBuilderImpl implements TupleBuilder, Tuple, SchemaAware {
-    /** Columns values. */
-    protected Map<String, Object> map;
+public class TupleImpl implements Tuple {
+    /** Column name -&gt; index. */
+    protected Map<String, Integer> colIdxMap;
+
+    /** Columns names. */
+    private final ArrayList<String> colNames;
 
-    /** Current schema descriptor. */
-    private SchemaDescriptor schemaDesc;
+    /** Columns values. */
+    private final ArrayList<Object> vals;
 
     /**
-     * Creates tuple builder.
-     *
-     * @param schemaDesc Schema descriptor.
+     * Creates tuple.
      */
-    public TupleBuilderImpl(SchemaDescriptor schemaDesc) {
-        Objects.requireNonNull(schemaDesc);
-
-        this.schemaDesc = schemaDesc;
-        map = new HashMap<>(schemaDesc.length());
+    public TupleImpl() {
+        colIdxMap = new HashMap<>();
+        vals = new ArrayList();
+        colNames = new ArrayList();
     }
 
     /** {@inheritDoc} */
-    @Override public TupleBuilder set(String columnName, Object val) {
-        getColumnOrThrow(columnName).validate(val);
+    @Override public Tuple set(String columnName, Object val) {
+        int idx = colIdxMap.computeIfAbsent(columnName, name -> colIdxMap.size());

Review comment:
       Missing `columnName` null check.

##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/RowChunkAdapter.java
##########
@@ -71,6 +71,11 @@
         }
     }
 
+    /** {@inheritDoc} */
+    @Override public Tuple set(String columnName, Object value) {
+        throw new UnsupportedOperationException("Tuple is immutable.");

Review comment:
       We return an immutable tuple to the user, and looks like there is no API to convert it to a mutable one. How is the user supposed to handle "get-change-set" scenario?
   
   Instead of throwing an exception, we can copy the data into a new mutable Tuple and return it.
   




-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org