You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/06/09 19:39:39 UTC

[GitHub] [ignite-3] AMashenkov opened a new pull request #168: IGNITE-14743: Row layout. Add large values support.

AMashenkov opened a new pull request #168:
URL: https://github.com/apache/ignite-3/pull/168


   


-- 
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.

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



[GitHub] [ignite-3] ascherbakoff commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
ascherbakoff commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r655492113



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.put(vartblOff, (byte)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readByte(vartblOff) & 0xFF;
+        }
+    };
+
+    /** Writer factory for med-sized chunks. */
+    private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /** Writer factory for large-sized chunks. */
+    private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readInteger(vartblOff + vartableEntryOffset(entryIdx));
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /**
+     * Return chunk formatter.
+     *
+     * @param payloadLen Payload size in bytes.
+     * @return Chunk formatter.
+     */
+    static ChunkFormat formatter(int payloadLen) {

Review comment:
       This approach can create different binary representations of a key, depending on the payloadLen. It should be avoided. For example, we can always use LARGE format for a key.




-- 
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.

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



[GitHub] [ignite-3] ascherbakoff commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
ascherbakoff commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r655978177



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/Row.java
##########
@@ -343,65 +355,55 @@ private boolean hasFlag(int flag) {
      */
     protected long findColumn(int colIdx, NativeTypeSpec type) throws InvalidTypeException {
         // Get base offset (key start or value start) for the given column.
-        boolean keyCol = schema.isKeyColumn(colIdx);
-
-        final short flags = readShort(FLAGS_FIELD_OFFSET);
+        boolean isKeyCol = schema.isKeyColumn(colIdx);
 
-        int off = KEY_CHUNK_OFFSET;
-
-        if (!keyCol) {
-            assert (flags & RowFlags.NO_VALUE_FLAG) == 0;
-
-            // Jump to the next chunk, the size of the first chunk is written at the chunk start.
-            off += readInteger(off);
-
-            // Adjust the column index according to the number of key columns.
+        // Adjust the column index according to the number of key columns.
+        if (!isKeyCol)
             colIdx -= schema.keyColumns().length();
-        }
 
-        Columns cols = keyCol ? schema.keyColumns() : schema.valueColumns();
+        ChunkReader reader = isKeyCol ? keyReader : valueReader();
+        Columns cols = isKeyCol ? schema.keyColumns() : schema.valueColumns();
 
         if (cols.column(colIdx).type().spec() != type)
             throw new InvalidTypeException("Invalid column type requested [requested=" + type +
                 ", column=" + cols.column(colIdx) + ']');
 
-        boolean hasVarTable = ((keyCol ? RowFlags.OMIT_KEY_VARTBL_FLAG : RowFlags.OMIT_VAL_VARTBL_FLAG) & flags) == 0;
-        boolean hasNullMap = ((keyCol ? RowFlags.OMIT_KEY_NULL_MAP_FLAG : RowFlags.OMIT_VAL_NULL_MAP_FLAG) & flags) == 0;
+        assert reader != null;
 
-        if (hasNullMap && isNull(off, colIdx))
+        if (reader.isNull(colIdx))
             return -1;
 
         return type.fixedLength() ?
-            fixlenColumnOffset(cols, off, colIdx, hasVarTable, hasNullMap) :
-            varlenColumnOffsetAndLength(cols, off, colIdx, hasVarTable, hasNullMap);
+            reader.fixlenColumnOffset(cols, colIdx) :

Review comment:
       It seems to me the reader should not implement these methods. Instread, it should be passed as an argument to the corresponding methods for offset calculation.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/Row.java
##########
@@ -343,65 +355,55 @@ private boolean hasFlag(int flag) {
      */
     protected long findColumn(int colIdx, NativeTypeSpec type) throws InvalidTypeException {
         // Get base offset (key start or value start) for the given column.
-        boolean keyCol = schema.isKeyColumn(colIdx);
-
-        final short flags = readShort(FLAGS_FIELD_OFFSET);
+        boolean isKeyCol = schema.isKeyColumn(colIdx);
 
-        int off = KEY_CHUNK_OFFSET;
-
-        if (!keyCol) {
-            assert (flags & RowFlags.NO_VALUE_FLAG) == 0;
-
-            // Jump to the next chunk, the size of the first chunk is written at the chunk start.
-            off += readInteger(off);
-
-            // Adjust the column index according to the number of key columns.
+        // Adjust the column index according to the number of key columns.
+        if (!isKeyCol)
             colIdx -= schema.keyColumns().length();
-        }
 
-        Columns cols = keyCol ? schema.keyColumns() : schema.valueColumns();
+        ChunkReader reader = isKeyCol ? keyReader : valueReader();
+        Columns cols = isKeyCol ? schema.keyColumns() : schema.valueColumns();
 
         if (cols.column(colIdx).type().spec() != type)
             throw new InvalidTypeException("Invalid column type requested [requested=" + type +
                 ", column=" + cols.column(colIdx) + ']');
 
-        boolean hasVarTable = ((keyCol ? RowFlags.OMIT_KEY_VARTBL_FLAG : RowFlags.OMIT_VAL_VARTBL_FLAG) & flags) == 0;
-        boolean hasNullMap = ((keyCol ? RowFlags.OMIT_KEY_NULL_MAP_FLAG : RowFlags.OMIT_VAL_NULL_MAP_FLAG) & flags) == 0;
+        assert reader != null;
 
-        if (hasNullMap && isNull(off, colIdx))
+        if (reader.isNull(colIdx))
             return -1;
 
         return type.fixedLength() ?
-            fixlenColumnOffset(cols, off, colIdx, hasVarTable, hasNullMap) :
-            varlenColumnOffsetAndLength(cols, off, colIdx, hasVarTable, hasNullMap);
+            reader.fixlenColumnOffset(cols, colIdx) :

Review comment:
       It seems to me the reader should not implement these methods. Instread, it should be makde stateless abd passed as an argument to the corresponding methods for offset calculation. This can help to avoid unnecessary object creation.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/Row.java
##########
@@ -343,65 +355,55 @@ private boolean hasFlag(int flag) {
      */
     protected long findColumn(int colIdx, NativeTypeSpec type) throws InvalidTypeException {
         // Get base offset (key start or value start) for the given column.
-        boolean keyCol = schema.isKeyColumn(colIdx);
-
-        final short flags = readShort(FLAGS_FIELD_OFFSET);
+        boolean isKeyCol = schema.isKeyColumn(colIdx);
 
-        int off = KEY_CHUNK_OFFSET;
-
-        if (!keyCol) {
-            assert (flags & RowFlags.NO_VALUE_FLAG) == 0;
-
-            // Jump to the next chunk, the size of the first chunk is written at the chunk start.
-            off += readInteger(off);
-
-            // Adjust the column index according to the number of key columns.
+        // Adjust the column index according to the number of key columns.
+        if (!isKeyCol)
             colIdx -= schema.keyColumns().length();
-        }
 
-        Columns cols = keyCol ? schema.keyColumns() : schema.valueColumns();
+        ChunkReader reader = isKeyCol ? keyReader : valueReader();
+        Columns cols = isKeyCol ? schema.keyColumns() : schema.valueColumns();
 
         if (cols.column(colIdx).type().spec() != type)
             throw new InvalidTypeException("Invalid column type requested [requested=" + type +
                 ", column=" + cols.column(colIdx) + ']');
 
-        boolean hasVarTable = ((keyCol ? RowFlags.OMIT_KEY_VARTBL_FLAG : RowFlags.OMIT_VAL_VARTBL_FLAG) & flags) == 0;
-        boolean hasNullMap = ((keyCol ? RowFlags.OMIT_KEY_NULL_MAP_FLAG : RowFlags.OMIT_VAL_NULL_MAP_FLAG) & flags) == 0;
+        assert reader != null;
 
-        if (hasNullMap && isNull(off, colIdx))
+        if (reader.isNull(colIdx))
             return -1;
 
         return type.fixedLength() ?
-            fixlenColumnOffset(cols, off, colIdx, hasVarTable, hasNullMap) :
-            varlenColumnOffsetAndLength(cols, off, colIdx, hasVarTable, hasNullMap);
+            reader.fixlenColumnOffset(cols, colIdx) :

Review comment:
       It seems to me the reader should not implement these methods. Instead, it should be made stateless and passed as an argument to the corresponding methods for offset calculation. This can help to avoid unnecessary object creation.




-- 
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.

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



[GitHub] [ignite-3] AMashenkov merged pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
AMashenkov merged pull request #168:
URL: https://github.com/apache/ignite-3/pull/168


   


-- 
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



[GitHub] [ignite-3] ascherbakoff commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
ascherbakoff commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r655509325



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {

Review comment:
       I think it will be good for common understanding of how fields are packed to give several examples of how different rows are represented in this scheme.




-- 
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.

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



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r670557621



##########
File path: modules/schema/src/test/java/org/apache/ignite/internal/schema/RowAssemblerAdvancedSchemaTest.java
##########
@@ -54,8 +55,8 @@ public void fixedNullableColumns() {
 
         // Last col null
         assertRowBytesEquals(
-            new byte[] {42, 0, 24, 0, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1},
-            new RowAssembler(schema, 0, 0, 0)
+            new byte[] {42, 0, 0, -103, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1},

Review comment:
       We already have RowTest covered this case.
   
   




-- 
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



[GitHub] [ignite-3] tledkov-gridgain commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r669571612



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
##########
@@ -499,86 +575,89 @@ private void checkType(NativeType type) {
     }
 
     /**
-     * Sets null flag in the null map for the given column.
+     * Sets null flag in the null-map for the given column.
      *
      * @param colIdx Column index.
      */
     private void setNull(int colIdx) {
-        assert (flags & (baseOff == BinaryRow.KEY_CHUNK_OFFSET ? RowFlags.OMIT_KEY_NULL_MAP_FLAG : RowFlags.OMIT_VAL_NULL_MAP_FLAG)) == 0 :
-            "Illegal writing 'null' value when 'omit null-map' flag is set for a chunk.";
+        assert nullMapOff < varTblOff : "Null-map is omitted.";
 
-        int byteInMap = colIdx / 8;
-        int bitInByte = colIdx % 8;
+        int byteInMap = colIdx >> 3; // Equivalent expression for: colIidx / 8
+        int bitInByte = colIdx & 7; // Equivalent expression for: colIdx % 8
 
         buf.ensureCapacity(nullMapOff + byteInMap + 1);
 
-        buf.put(nullMapOff + byteInMap, (byte)(buf.get(nullMapOff + byteInMap) | (1 << bitInByte)));
+        buf.put(nullMapOff + byteInMap, (byte)((Byte.toUnsignedInt(buf.get(nullMapOff + byteInMap))) | (1 << bitInByte)));
     }
 
     /**
-     * Must be called after an append of fixlen column.
-     *
-     * @param type Type of the appended column.
+     * Shifts current column indexes as necessary, also
+     * switch to value chunk writer when moving from key to value columns.
      */
-    private void shiftColumn(NativeType type) {
-        assert type.spec().fixedLength() : "Varlen types should provide field length to shift column: " + type;
-
-        shiftColumn(type.sizeInBytes(), false);
-    }
-
-    /**
-     * Shifts current offsets and column indexes as necessary, also changes the chunk base offsets when
-     * moving from key to value columns.
-     *
-     * @param size Size of the appended column.
-     * @param varlen {@code true} if appended column was varlen.
-     */
-    private void shiftColumn(int size, boolean varlen) {
+    private void shiftColumn(int size) {
         curCol++;
         curOff += size;
 
-        if (varlen)
-            curVarlenTblEntry++;
-
         if (curCol == curCols.length()) {
-            int chunkLen = curOff - baseOff;
+            if (curVartblEntry > 1) {
+                assert varTblOff < dataOff : "Illegal writing of varlen when 'omit vartable' flag is set for a chunk.";
+                assert varTblOff + varTableChunkLength(curVartblEntry, Integer.BYTES) == dataOff : "Vartable overlow: size=" + curVartblEntry;
+
+                final VarTableFormat format = VarTableFormat.format( curOff - dataOff);

Review comment:
       Remove space after open brace




-- 
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



[GitHub] [ignite-3] ascherbakoff commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
ascherbakoff commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r655224609



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {

Review comment:
       Nested class declarations should be put to the end of class file

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.put(vartblOff, (byte)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readByte(vartblOff) & 0xFF;
+        }
+    };
+
+    /** Writer factory for med-sized chunks. */
+    private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /** Writer factory for large-sized chunks. */
+    private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readInteger(vartblOff + vartableEntryOffset(entryIdx));
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /**
+     * Return chunk formatter.
+     *
+     * @param payloadLen Payload size in bytes.
+     * @return Chunk formatter.
+     */
+    static ChunkFormat formatter(int payloadLen) {

Review comment:
       This approach can create different binary representations of a key, depending on the payloadLen. It should be avoided. For example, we can always use LARGE format for a key.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/Columns.java
##########
@@ -58,6 +58,11 @@
      */
     private final int nullMapSize;
 
+    /**
+     * Estimated max length of fixed-size columns.
+     */
+    private int fixsizeMaxLen;

Review comment:
       ```suggestion
       private int fixedSizeMaxLen;
   ```

##########
File path: modules/schema/src/test/java/org/apache/ignite/internal/schema/RowAssemblerAdvancedSchemaTest.java
##########
@@ -54,8 +55,8 @@ public void fixedNullableColumns() {
 
         // Last col null
         assertRowBytesEquals(
-            new byte[] {42, 0, 24, 0, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1},
-            new RowAssembler(schema, 0, 0, 0)
+            new byte[] {42, 0, 0, -103, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1},

Review comment:
       Can we avoid comparing with byte array and instead compare expected values, like
   row0 -> serialize -> deserialize -> row1
   assert row0 equals row1

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
##########
@@ -121,80 +106,101 @@ else if (Character.isHighSurrogate(ch)) {
     }
 
     /**
-     * @param keyCols Key columns.
+     * Creates RowAssembler for chunks of unknown size.
+     * <p>
+     * RowAssembler will use adaptive buffer size and omit some optimizations for small key/value chunks.
+     *
+     * @param schema Row schema.
      * @param nonNullVarlenKeyCols Number of non-null varlen columns in key chunk.
-     * @param nonNullVarlenKeySize Size of non-null varlen columns in key chunk.
-     * @param valCols Value columns.
      * @param nonNullVarlenValCols Number of non-null varlen columns in value chunk.
-     * @param nonNullVarlenValSize Size of non-null varlen columns in value chunk.
-     * @return Total row size.
      */
-    public static int rowSize(
-        Columns keyCols,
+    public RowAssembler(
+        SchemaDescriptor schema,
         int nonNullVarlenKeyCols,
-        int nonNullVarlenKeySize,
-        Columns valCols,
-        int nonNullVarlenValCols,
-        int nonNullVarlenValSize
+        int nonNullVarlenValCols
     ) {
-        return BinaryRow.KEY_CHUNK_OFFSET /* Header size */ +
-            rowChunkSize(keyCols, nonNullVarlenKeyCols, nonNullVarlenKeySize) +
-            rowChunkSize(valCols, nonNullVarlenValCols, nonNullVarlenValSize);
-    }
-
-    /**
-     * @param cols Columns.
-     * @param nonNullVarlenCols Number of non-null varlen columns in chunk.
-     * @param nonNullVarlenSize Size of non-null varlen columns in chunk.
-     * @return Row's chunk size.
-     */
-    static int rowChunkSize(Columns cols, int nonNullVarlenCols, int nonNullVarlenSize) {
-        int size = BinaryRow.CHUNK_LEN_FIELD_SIZE + cols.nullMapSize() +
-            varlenTableChunkSize(nonNullVarlenCols);
-
-        for (int i = 0; i < cols.numberOfFixsizeColumns(); i++)
-            size += cols.column(i).type().sizeInBytes();
-
-        return size + nonNullVarlenSize;
+        this(schema,
+            0,
+            schema.keyColumns().nullMapSize() > 0,
+            nonNullVarlenKeyCols,
+            0,
+            schema.valueColumns().nullMapSize() > 0,
+            nonNullVarlenValCols);
     }
 
     /**
+     * Creates RowAssembler for chunks with estimated sizes.
+     * <p>
+     * RowAssembler will apply optimizations based on chunks sizes estimations.
+     *
      * @param schema Row schema.
-     * @param size Target row size. If the row size is known in advance, it should be provided upfront to avoid
-     * unnecessary arrays copy.
+     * @param keyDataSize Key payload size. Estimated upper-bound or zero if unknown.
      * @param nonNullVarlenKeyCols Number of non-null varlen columns in key chunk.
+     * @param valDataSize Value data size. Estimated upper-bound or zero if unknown.
      * @param nonNullVarlenValCols Number of non-null varlen columns in value chunk.
      */
     public RowAssembler(
         SchemaDescriptor schema,
-        int size,
+        int keyDataSize,
         int nonNullVarlenKeyCols,
+        int valDataSize,
         int nonNullVarlenValCols
+    ) {
+        this(

Review comment:
       Inconsistent formatting with line 122

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkReader.java
##########
@@ -0,0 +1,212 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.Columns;
+
+/**
+ * Chunk reader.
+ */
+class ChunkReader {
+    /** Row. */
+    protected final BinaryRow row;
+
+    /** Base offset. */
+    protected final int baseOff;
+
+    /** Chunk format. */
+    private final ChunkFormat format;
+
+    /** Vartable offset. */
+    protected int varTblOff;
+
+    /** Data offset. */
+    protected int dataOff;
+
+    /**
+     * @param row Row.
+     * @param baseOff Chunk base offset.
+     * @param nullMapLen Null-map size in bytes.
+     * @param hasVarTable {@code true} if chunk has vartable, {@code false} otherwise.
+     * @param format Chunk format.
+     */
+    ChunkReader(BinaryRow row, int baseOff, int nullMapLen, boolean hasVarTable, ChunkFormat format) {
+        this.row = row;
+        this.baseOff = baseOff;
+        this.format = format;
+        varTblOff = nullmapOff() + nullMapLen;
+        dataOff = varTblOff + (hasVarTable ? format.vartableLength(format.readVartableSize(row, varTblOff)) : 0);
+    }
+
+    /**
+     * Reads chunk total length.
+     *
+     * @return Chunk length in bytes
+     */
+    int chunkLength() {
+        return row.readInteger(baseOff);
+    }
+
+    /**
+     * Checks the row's null-map for the given column index in the chunk.
+     *
+     * @param idx Offset of the column in the chunk.
+     * @return {@code true} if the column value is {@code null}, {@code false} otherwise.
+     */
+    protected boolean isNull(int idx) {
+        if (!hasNullmap())
+            return false;
+
+        int nullByte = idx / 8;
+        int posInByte = idx % 8;

Review comment:
       This should be rewritten using bit opts

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.put(vartblOff, (byte)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readByte(vartblOff) & 0xFF;
+        }
+    };
+
+    /** Writer factory for med-sized chunks. */
+    private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /** Writer factory for large-sized chunks. */
+    private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readInteger(vartblOff + vartableEntryOffset(entryIdx));
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /**
+     * Return chunk formatter.
+     *
+     * @param payloadLen Payload size in bytes.
+     * @return Chunk formatter.
+     */
+    static ChunkFormat formatter(int payloadLen) {
+        if (payloadLen > 0) {
+            if (payloadLen < 256)
+                return TINY;
+
+            if (payloadLen < 64 * 1024)
+                return MEDIUM;
+        }
+
+        return LARGE;
+    }
+
+    /**
+     * Creates chunk reader.
+     *
+     * @param row Binary row.
+     * @param offset Chunk offset.
+     * @param nullMapSize Default chunk null-map size.
+     * @param chunkFlags Chunk flags. First 4-bits are meaningful.
+     * @return Chunk reader.
+     */
+    static ChunkReader createReader(BinaryRow row, int offset, int nullMapSize, byte chunkFlags) {
+        return fromFlags(chunkFlags).reader(row, offset, nullMapSize, chunkFlags);
+    }
+
+    /**
+     * Chunk format factory method.
+     *
+     * @param chunkFlags Chunk specific flags. Only first 4-bits are meaningful.
+     * @return Chunk formatter regarding the provided flags.
+     */
+    private static ChunkFormat fromFlags(byte chunkFlags) {
+        final int mode = chunkFlags & FORMAT_CODE_MASK;

Review comment:
       This variable is not necessary

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {

Review comment:
       I think it will be good for later understanding of how fields are packed to give several examples of how different rows are represented in this scheme.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkReader.java
##########
@@ -0,0 +1,212 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.Columns;
+
+/**
+ * Chunk reader.
+ */
+class ChunkReader {
+    /** Row. */
+    protected final BinaryRow row;
+
+    /** Base offset. */
+    protected final int baseOff;
+
+    /** Chunk format. */
+    private final ChunkFormat format;
+
+    /** Vartable offset. */
+    protected int varTblOff;
+
+    /** Data offset. */
+    protected int dataOff;
+
+    /**
+     * @param row Row.
+     * @param baseOff Chunk base offset.
+     * @param nullMapLen Null-map size in bytes.
+     * @param hasVarTable {@code true} if chunk has vartable, {@code false} otherwise.
+     * @param format Chunk format.
+     */
+    ChunkReader(BinaryRow row, int baseOff, int nullMapLen, boolean hasVarTable, ChunkFormat format) {
+        this.row = row;
+        this.baseOff = baseOff;
+        this.format = format;
+        varTblOff = nullmapOff() + nullMapLen;
+        dataOff = varTblOff + (hasVarTable ? format.vartableLength(format.readVartableSize(row, varTblOff)) : 0);
+    }
+
+    /**
+     * Reads chunk total length.
+     *
+     * @return Chunk length in bytes
+     */
+    int chunkLength() {
+        return row.readInteger(baseOff);
+    }
+
+    /**
+     * Checks the row's null-map for the given column index in the chunk.
+     *
+     * @param idx Offset of the column in the chunk.
+     * @return {@code true} if the column value is {@code null}, {@code false} otherwise.
+     */
+    protected boolean isNull(int idx) {
+        if (!hasNullmap())
+            return false;
+
+        int nullByte = idx / 8;
+        int posInByte = idx % 8;
+
+        int map = row.readByte(nullmapOff() + nullByte);
+
+        return (map & (1 << posInByte)) != 0;
+    }
+
+    /**
+     * @return Null-map offset
+     */
+    private int nullmapOff() {
+        return baseOff + Integer.BYTES;
+    }
+
+    /**
+     * @return {@code True} if chunk has vartable, {@code false} otherwise.
+     */
+    protected boolean hasVartable() {
+        return dataOff > varTblOff;
+    }
+
+    /**
+     * @return {@code True} if chunk has null-map, {@code false} otherwise.
+     */
+    protected boolean hasNullmap() {
+        return varTblOff > nullmapOff();
+    }
+
+    /**
+     * Calculates the offset of the fixlen column with the given index in the row. It essentially folds the null-map
+     * with the column lengths to calculate the size of non-null columns preceding the requested column.
+     *
+     * @param cols Columns chunk.
+     * @param idx Column index in the chunk.
+     * @return Encoded offset (from the row start) of the requested fixlen column.
+     */
+    int fixlenColumnOffset(Columns cols, int idx) {

Review comment:
       Avoid abbrevs in method names

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {

Review comment:
       I think it will be good for common understanding of how fields are packed to give several examples of how different rows are represented in this scheme.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.put(vartblOff, (byte)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readByte(vartblOff) & 0xFF;
+        }
+    };
+
+    /** Writer factory for med-sized chunks. */
+    private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /** Writer factory for large-sized chunks. */
+    private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readInteger(vartblOff + vartableEntryOffset(entryIdx));
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /**
+     * Return chunk formatter.
+     *
+     * @param payloadLen Payload size in bytes.
+     * @return Chunk formatter.
+     */
+    static ChunkFormat formatter(int payloadLen) {

Review comment:
       This approach can create different binary representations of a key, depending on the payloadLen. It should be avoided. For example, we can always use LARGE format for a key.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {

Review comment:
       I think it will be good for common understanding of how fields are packed to give several examples of how different rows are represented in this scheme.
   Also. this class look more like a VartableFormat




-- 
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.

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



[GitHub] [ignite-3] ascherbakoff commented on a change in pull request #168: IGNITE-14743: Row layout. Support compact formats for vartable.

Posted by GitBox <gi...@apache.org>.
ascherbakoff commented on a change in pull request #168:
URL: https://github.com/apache/ignite-3/pull/168#discussion_r655224609



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {

Review comment:
       Nested class declarations should be put to the end of class file

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.put(vartblOff, (byte)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readByte(vartblOff) & 0xFF;
+        }
+    };
+
+    /** Writer factory for med-sized chunks. */
+    private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /** Writer factory for large-sized chunks. */
+    private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readInteger(vartblOff + vartableEntryOffset(entryIdx));
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /**
+     * Return chunk formatter.
+     *
+     * @param payloadLen Payload size in bytes.
+     * @return Chunk formatter.
+     */
+    static ChunkFormat formatter(int payloadLen) {

Review comment:
       This approach can create different binary representations of a key, depending on the payloadLen. It should be avoided. For example, we can always use LARGE format for a key.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/Columns.java
##########
@@ -58,6 +58,11 @@
      */
     private final int nullMapSize;
 
+    /**
+     * Estimated max length of fixed-size columns.
+     */
+    private int fixsizeMaxLen;

Review comment:
       ```suggestion
       private int fixedSizeMaxLen;
   ```

##########
File path: modules/schema/src/test/java/org/apache/ignite/internal/schema/RowAssemblerAdvancedSchemaTest.java
##########
@@ -54,8 +55,8 @@ public void fixedNullableColumns() {
 
         // Last col null
         assertRowBytesEquals(
-            new byte[] {42, 0, 24, 0, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1},
-            new RowAssembler(schema, 0, 0, 0)
+            new byte[] {42, 0, 0, -103, -11, 43, 0, 0, 8, 0, 0, 0, 4, 11, 22, 0, 8, 0, 0, 0, 4, -44, -66, -1},

Review comment:
       Can we avoid comparing with byte array and instead compare expected values, like
   row0 -> serialize -> deserialize -> row1
   assert row0 equals row1

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
##########
@@ -121,80 +106,101 @@ else if (Character.isHighSurrogate(ch)) {
     }
 
     /**
-     * @param keyCols Key columns.
+     * Creates RowAssembler for chunks of unknown size.
+     * <p>
+     * RowAssembler will use adaptive buffer size and omit some optimizations for small key/value chunks.
+     *
+     * @param schema Row schema.
      * @param nonNullVarlenKeyCols Number of non-null varlen columns in key chunk.
-     * @param nonNullVarlenKeySize Size of non-null varlen columns in key chunk.
-     * @param valCols Value columns.
      * @param nonNullVarlenValCols Number of non-null varlen columns in value chunk.
-     * @param nonNullVarlenValSize Size of non-null varlen columns in value chunk.
-     * @return Total row size.
      */
-    public static int rowSize(
-        Columns keyCols,
+    public RowAssembler(
+        SchemaDescriptor schema,
         int nonNullVarlenKeyCols,
-        int nonNullVarlenKeySize,
-        Columns valCols,
-        int nonNullVarlenValCols,
-        int nonNullVarlenValSize
+        int nonNullVarlenValCols
     ) {
-        return BinaryRow.KEY_CHUNK_OFFSET /* Header size */ +
-            rowChunkSize(keyCols, nonNullVarlenKeyCols, nonNullVarlenKeySize) +
-            rowChunkSize(valCols, nonNullVarlenValCols, nonNullVarlenValSize);
-    }
-
-    /**
-     * @param cols Columns.
-     * @param nonNullVarlenCols Number of non-null varlen columns in chunk.
-     * @param nonNullVarlenSize Size of non-null varlen columns in chunk.
-     * @return Row's chunk size.
-     */
-    static int rowChunkSize(Columns cols, int nonNullVarlenCols, int nonNullVarlenSize) {
-        int size = BinaryRow.CHUNK_LEN_FIELD_SIZE + cols.nullMapSize() +
-            varlenTableChunkSize(nonNullVarlenCols);
-
-        for (int i = 0; i < cols.numberOfFixsizeColumns(); i++)
-            size += cols.column(i).type().sizeInBytes();
-
-        return size + nonNullVarlenSize;
+        this(schema,
+            0,
+            schema.keyColumns().nullMapSize() > 0,
+            nonNullVarlenKeyCols,
+            0,
+            schema.valueColumns().nullMapSize() > 0,
+            nonNullVarlenValCols);
     }
 
     /**
+     * Creates RowAssembler for chunks with estimated sizes.
+     * <p>
+     * RowAssembler will apply optimizations based on chunks sizes estimations.
+     *
      * @param schema Row schema.
-     * @param size Target row size. If the row size is known in advance, it should be provided upfront to avoid
-     * unnecessary arrays copy.
+     * @param keyDataSize Key payload size. Estimated upper-bound or zero if unknown.
      * @param nonNullVarlenKeyCols Number of non-null varlen columns in key chunk.
+     * @param valDataSize Value data size. Estimated upper-bound or zero if unknown.
      * @param nonNullVarlenValCols Number of non-null varlen columns in value chunk.
      */
     public RowAssembler(
         SchemaDescriptor schema,
-        int size,
+        int keyDataSize,
         int nonNullVarlenKeyCols,
+        int valDataSize,
         int nonNullVarlenValCols
+    ) {
+        this(

Review comment:
       Inconsistent formatting with line 122

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkReader.java
##########
@@ -0,0 +1,212 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.Columns;
+
+/**
+ * Chunk reader.
+ */
+class ChunkReader {
+    /** Row. */
+    protected final BinaryRow row;
+
+    /** Base offset. */
+    protected final int baseOff;
+
+    /** Chunk format. */
+    private final ChunkFormat format;
+
+    /** Vartable offset. */
+    protected int varTblOff;
+
+    /** Data offset. */
+    protected int dataOff;
+
+    /**
+     * @param row Row.
+     * @param baseOff Chunk base offset.
+     * @param nullMapLen Null-map size in bytes.
+     * @param hasVarTable {@code true} if chunk has vartable, {@code false} otherwise.
+     * @param format Chunk format.
+     */
+    ChunkReader(BinaryRow row, int baseOff, int nullMapLen, boolean hasVarTable, ChunkFormat format) {
+        this.row = row;
+        this.baseOff = baseOff;
+        this.format = format;
+        varTblOff = nullmapOff() + nullMapLen;
+        dataOff = varTblOff + (hasVarTable ? format.vartableLength(format.readVartableSize(row, varTblOff)) : 0);
+    }
+
+    /**
+     * Reads chunk total length.
+     *
+     * @return Chunk length in bytes
+     */
+    int chunkLength() {
+        return row.readInteger(baseOff);
+    }
+
+    /**
+     * Checks the row's null-map for the given column index in the chunk.
+     *
+     * @param idx Offset of the column in the chunk.
+     * @return {@code true} if the column value is {@code null}, {@code false} otherwise.
+     */
+    protected boolean isNull(int idx) {
+        if (!hasNullmap())
+            return false;
+
+        int nullByte = idx / 8;
+        int posInByte = idx % 8;

Review comment:
       This should be rewritten using bit opts

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {
+    /** First two flag bits reserved for format code. */
+    public static final int FORMAT_CODE_MASK = 0x03;
+
+    /** Flag indicates key chunk omits vartable. */
+    public static final int OMIT_NULL_MAP_FLAG = 1 << 2;
+
+    /** Flag indicates value chunk omits null map. */
+    public static final int OMIT_VARTBL_FLAG = 1 << 3;
+
+    /** Writer factory for tiny-sized chunks. */
+    private static final ChunkFormat TINY = new ChunkFormat(Byte.BYTES, Byte.BYTES, (byte)1) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 8) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.put(vartblOff + vartableEntryOffset(entryIdx), (byte)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readByte(vartblOff + vartableEntryOffset(entryIdx)) & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 8) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.put(vartblOff, (byte)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readByte(vartblOff) & 0xFF;
+        }
+    };
+
+    /** Writer factory for med-sized chunks. */
+    private static final ChunkFormat MEDIUM = new ChunkFormat(Short.BYTES, Short.BYTES, (byte)2) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            assert off < (1 << 16) && off >= 0 : "Varlen offset overflow: offset=" + off;
+
+            buf.putShort(vartblOff + vartableEntryOffset(entryIdx), (short)off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readShort(vartblOff + vartableEntryOffset(entryIdx)) & 0xFFFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /** Writer factory for large-sized chunks. */
+    private static final ChunkFormat LARGE = new ChunkFormat(Short.BYTES, Integer.BYTES, (byte)0) {
+        /** {@inheritDoc} */
+        @Override void writeVarlenOffset(ExpandableByteBuf buf, int vartblOff, int entryIdx, int off) {
+            buf.putInt(vartblOff + vartableEntryOffset(entryIdx), off);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVarlenOffset(BinaryRow row, int vartblOff, int entryIdx) {
+            return row.readInteger(vartblOff + vartableEntryOffset(entryIdx));
+        }
+
+        /** {@inheritDoc} */
+        @Override void writeVartableSize(ExpandableByteBuf buf, int vartblOff, int size) {
+            assert size < (1 << 16) && size >= 0 : "Vartable size overflow: size=" + size;
+
+            buf.putShort(vartblOff, (short)size);
+        }
+
+        /** {@inheritDoc} */
+        @Override int readVartableSize(BinaryRow row, int vartblOff) {
+            return row.readShort(vartblOff) & 0xFFFF;
+        }
+    };
+
+    /**
+     * Return chunk formatter.
+     *
+     * @param payloadLen Payload size in bytes.
+     * @return Chunk formatter.
+     */
+    static ChunkFormat formatter(int payloadLen) {
+        if (payloadLen > 0) {
+            if (payloadLen < 256)
+                return TINY;
+
+            if (payloadLen < 64 * 1024)
+                return MEDIUM;
+        }
+
+        return LARGE;
+    }
+
+    /**
+     * Creates chunk reader.
+     *
+     * @param row Binary row.
+     * @param offset Chunk offset.
+     * @param nullMapSize Default chunk null-map size.
+     * @param chunkFlags Chunk flags. First 4-bits are meaningful.
+     * @return Chunk reader.
+     */
+    static ChunkReader createReader(BinaryRow row, int offset, int nullMapSize, byte chunkFlags) {
+        return fromFlags(chunkFlags).reader(row, offset, nullMapSize, chunkFlags);
+    }
+
+    /**
+     * Chunk format factory method.
+     *
+     * @param chunkFlags Chunk specific flags. Only first 4-bits are meaningful.
+     * @return Chunk formatter regarding the provided flags.
+     */
+    private static ChunkFormat fromFlags(byte chunkFlags) {
+        final int mode = chunkFlags & FORMAT_CODE_MASK;

Review comment:
       This variable is not necessary

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkFormat.java
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+
+/**
+ * Chunk format.
+ */
+abstract class ChunkFormat {

Review comment:
       I think it will be good for later understanding of how fields are packed to give several examples of how different rows are represented in this scheme.

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/row/ChunkReader.java
##########
@@ -0,0 +1,212 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.row;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.Columns;
+
+/**
+ * Chunk reader.
+ */
+class ChunkReader {
+    /** Row. */
+    protected final BinaryRow row;
+
+    /** Base offset. */
+    protected final int baseOff;
+
+    /** Chunk format. */
+    private final ChunkFormat format;
+
+    /** Vartable offset. */
+    protected int varTblOff;
+
+    /** Data offset. */
+    protected int dataOff;
+
+    /**
+     * @param row Row.
+     * @param baseOff Chunk base offset.
+     * @param nullMapLen Null-map size in bytes.
+     * @param hasVarTable {@code true} if chunk has vartable, {@code false} otherwise.
+     * @param format Chunk format.
+     */
+    ChunkReader(BinaryRow row, int baseOff, int nullMapLen, boolean hasVarTable, ChunkFormat format) {
+        this.row = row;
+        this.baseOff = baseOff;
+        this.format = format;
+        varTblOff = nullmapOff() + nullMapLen;
+        dataOff = varTblOff + (hasVarTable ? format.vartableLength(format.readVartableSize(row, varTblOff)) : 0);
+    }
+
+    /**
+     * Reads chunk total length.
+     *
+     * @return Chunk length in bytes
+     */
+    int chunkLength() {
+        return row.readInteger(baseOff);
+    }
+
+    /**
+     * Checks the row's null-map for the given column index in the chunk.
+     *
+     * @param idx Offset of the column in the chunk.
+     * @return {@code true} if the column value is {@code null}, {@code false} otherwise.
+     */
+    protected boolean isNull(int idx) {
+        if (!hasNullmap())
+            return false;
+
+        int nullByte = idx / 8;
+        int posInByte = idx % 8;
+
+        int map = row.readByte(nullmapOff() + nullByte);
+
+        return (map & (1 << posInByte)) != 0;
+    }
+
+    /**
+     * @return Null-map offset
+     */
+    private int nullmapOff() {
+        return baseOff + Integer.BYTES;
+    }
+
+    /**
+     * @return {@code True} if chunk has vartable, {@code false} otherwise.
+     */
+    protected boolean hasVartable() {
+        return dataOff > varTblOff;
+    }
+
+    /**
+     * @return {@code True} if chunk has null-map, {@code false} otherwise.
+     */
+    protected boolean hasNullmap() {
+        return varTblOff > nullmapOff();
+    }
+
+    /**
+     * Calculates the offset of the fixlen column with the given index in the row. It essentially folds the null-map
+     * with the column lengths to calculate the size of non-null columns preceding the requested column.
+     *
+     * @param cols Columns chunk.
+     * @param idx Column index in the chunk.
+     * @return Encoded offset (from the row start) of the requested fixlen column.
+     */
+    int fixlenColumnOffset(Columns cols, int idx) {

Review comment:
       Avoid abbrevs in method names




-- 
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.

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