AMashenkov commented on code in PR #2203:
URL: https://github.com/apache/ignite-3/pull/2203#discussion_r1519532335


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/TableRowConverterImpl.java:
##########
@@ -46,66 +41,36 @@ public class TableRowConverterImpl implements 
TableRowConverter {
 
     private final BinaryTupleSchema fullTupleSchema;
 
-    private final int[] fullRowColocationIndexes;
-    private final int[] keyOnlyColocationIndexes;
-    private final NativeType[] colocationColumnTypes;
-
     /**
      * Mapping of required columns to their indexes in physical schema.
      */
     private final int[] requiredColumnsMapping;
 
-    /**
-     * Mapping of all columns to their indexes in physical schema.
-     */
-    private final int[] fullMapping;
-
-    private final boolean skipReshuffling;
+    private final boolean skipTrimming;
 
     /** Constructor. */
     TableRowConverterImpl(
             SchemaRegistry schemaRegistry,
             BinaryTupleSchema fullTupleSchema,
-            int[] fullRowColocationIndexes,
-            int[] keyOnlyColocationIndexes,
-            NativeType[] colocationColumnTypes,
             SchemaDescriptor schemaDescriptor,
             @Nullable BitSet requiredColumns
     ) {
         this.schemaRegistry = schemaRegistry;
         this.schemaDescriptor = schemaDescriptor;
-        this.fullRowColocationIndexes = fullRowColocationIndexes;
-        this.keyOnlyColocationIndexes = keyOnlyColocationIndexes;
-        this.colocationColumnTypes = colocationColumnTypes;
         this.fullTupleSchema = fullTupleSchema;
 
-        this.skipReshuffling = requiredColumns == null && 
schemaDescriptor.physicalOrderMatchesLogical();
+        this.skipTrimming = requiredColumns == null;

Review Comment:
   Maybe it is better make the class package-private and have 2 constructor for 
both cases: `requiredColumns` is null and not null; instead of having bunch of 
`if (requiredColumns == null)`? Or maybe different implementations?
   In fact, the object is always created via factory and it is clear which 
constructor must be used.
   
   It is ok to make a refactoring in a. separate ticket.
   
    



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to