sk0x50 commented on a change in pull request #73: URL: https://github.com/apache/ignite-3/pull/73#discussion_r609746143
########## File path: modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java ########## @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) undeBinaryRow one oBinaryRow more + * contributoBinaryRow license agreements. See the NOTICE file distributed with + * this work foBinaryRow additional information regarding copyright ownership. + * The ASF licenses this file to You undeBinaryRow 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 oBinaryRow agreed to in writing, software + * distributed undeBinaryRow the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OBinaryRow CONDITIONS OF ANY KIND, eitheBinaryRow express oBinaryRow implied. + * See the License foBinaryRow the specific language governing permissions and + * limitations undeBinaryRow the License. + */ + +package org.apache.ignite.internal.table; + +import java.util.Collection; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.schema.BinaryRow; +import org.jetbrains.annotations.NotNull; + +/** + * Internal table facade provides low-level methods foBinaryRow table operations. + * The facade hides TX/replication protocol oBinaryRow table storage abstractions. + */ +public interface InternalTable { + /** + * Asynchronously gets a record with same key columns values as given one from the table. Review comment: It seems to me that `record` should be changed to `row`. ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/RowAssembler.java ########## @@ -338,6 +340,17 @@ public void appendBitmask(BitSet bitSet) { * @return Serialized row. */ public byte[] build() { + if (schema.keyColumns() == curCols) + throw new IllegalStateException("Key column missed: colIdx=" + curCol); + else { + if (curCol == 0) + flags |= BinaryRow.RowFlags.NULL_VALUE; + else if (schema.valueColumns().length() != curCol) + throw new IllegalStateException("Value column missed: colIdx=" + curCol); Review comment: Does it make sense to change `IllegalStateException` to a new `IgniteInternalException`? ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryRow.java ########## @@ -0,0 +1,144 @@ +/* + * 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; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.ByteBuffer; + +/** + * Binary row interface. + * The class contains low-level methods to read row data. + */ +public interface BinaryRow { + /** */ + int SCHEMA_VERSION_OFFSET = 0; + /** */ + int FLAGS_FIELD_OFFSET = SCHEMA_VERSION_OFFSET + 2; + /** */ + int KEY_HASH_FIELD_OFFSET = FLAGS_FIELD_OFFSET + 2; + /** */ + int KEY_CHUNK_OFFSET = KEY_HASH_FIELD_OFFSET + 4; + /** */ + int TOTAL_LEN_FIELD_SIZE = 4; + /** */ + int VARLEN_TABLE_SIZE_FIELD_SIZE = 2; + /** */ + int VARLEN_COLUMN_OFFSET_FIELD_SIZE = 2; + + /** + * @return Row schema version. + */ + int schemaVersion(); + + /** + * @return {@code True} if row has non-null value, {@code false} otherwise. + */ + boolean hasValue(); + + // TODO: IGNITE-14199. Add row version. + //GridRowVersion version(); + + /** + * Row hash code is a result of hash function applied to the row affinity columns values. + * + * @return Row hash code. + */ + int hash(); + + /** + * @return ByteBuffer slice representing the key chunk. + */ + ByteBuffer keySlice(); + + /** + * @return ByteBuffer slice representing the value chunk. + */ + ByteBuffer valueSlice(); + + /** + * Writes binary row to given stream. + * + * @throws IOException If write operation fails. + */ + // TODO: support other target types in writeTo() if needed. Review comment: It would be nice to have a Jira ticket for all TODOs ########## File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/Row.java ########## @@ -17,69 +17,55 @@ package org.apache.ignite.internal.schema; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.ByteBuffer; import java.util.BitSet; import java.util.UUID; /** + * Schema-aware row. + * * The class contains non-generic methods to read boxed and unboxed primitives based on the schema column types. * Any type conversions and coercions should be implemented outside the row by the key-value or query runtime. * When a non-boxed primitive is read from a null column value, it is converted to the primitive type default value. */ -public abstract class Row { - /** */ - public static final int SCHEMA_VERSION_OFFSET = 0; - - /** */ - public static final int FLAGS_FIELD_OFFSET = SCHEMA_VERSION_OFFSET + 2; - - /** */ - public static final int KEY_HASH_FIELD_OFFSET = FLAGS_FIELD_OFFSET + 2; - - /** */ - public static final int KEY_CHUNK_OFFSET = KEY_HASH_FIELD_OFFSET + 4; - - /** */ - public static final int TOTAL_LEN_FIELD_SIZE = 4; - - /** */ - public static final int VARLEN_TABLE_SIZE_FIELD_SIZE = 2; - - /** */ - public static final int VARLEN_COLUMN_OFFSET_FIELD_SIZE = 2; +public class Row implements BinaryRow { + /** Schema descriptor. */ + private final SchemaDescriptor schema; - /** */ - public static final class RowFlags { - /** Tombstone flag. */ - public static final int TOMBSTONE = 1; + /** Binary row. */ + private final BinaryRow row; - /** Null-value flag. */ - public static final int NULL_VALUE = 1 << 1; + /** + * Constructor. + * + * @param schema Schema. + * @param row Binary row representation. + */ + public Row(SchemaDescriptor schema, BinaryRow row) { + assert row.schemaVersion() == schema.version(); - /** Stub. */ - private RowFlags() { - } + this.row = row; + this.schema = schema; } - /** Schema descriptor for which this row was created. */ - private final SchemaDescriptor schema; - /** - * @param schema Schema instance. + * @return Row schema. */ - protected Row(SchemaDescriptor schema) { - this.schema = schema; + public SchemaDescriptor rowSchema() { + return schema; } /** * @return {@code True} if row has non-null value, {@code false} otherwise. */ - public boolean hasValue() { - short flags = readShort(FLAGS_FIELD_OFFSET); - - return (flags & (RowFlags.NULL_VALUE | RowFlags.TOMBSTONE)) == 0; + @Override public boolean hasValue() { + return row.hasValue(); } /** + * Review comment: Please describe the contract of this method. What happens if the required column has a different type. What happens if the row does not have a value. ########## File path: modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java ########## @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) undeBinaryRow one oBinaryRow more + * contributoBinaryRow license agreements. See the NOTICE file distributed with + * this work foBinaryRow additional information regarding copyright ownership. + * The ASF licenses this file to You undeBinaryRow 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 oBinaryRow agreed to in writing, software + * distributed undeBinaryRow the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OBinaryRow CONDITIONS OF ANY KIND, eitheBinaryRow express oBinaryRow implied. + * See the License foBinaryRow the specific language governing permissions and + * limitations undeBinaryRow the License. + */ + +package org.apache.ignite.internal.table; + +import java.util.Collection; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.schema.BinaryRow; +import org.jetbrains.annotations.NotNull; + +/** + * Internal table facade provides low-level methods foBinaryRow table operations. Review comment: What do `foBinaryRow` and `oBinaryRow` mean? -- 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: [email protected]
