You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "tkalkirill (via GitHub)" <gi...@apache.org> on 2023/05/25 15:37:47 UTC
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2093: IGNITE-19531 Switch table IDs from UUIDs to integers
tkalkirill commented on code in PR #2093:
URL: https://github.com/apache/ignite-3/pull/2093#discussion_r1205658113
##########
modules/network-api/src/main/java/org/apache/ignite/network/serialization/MessageReader.java:
##########
@@ -95,6 +96,15 @@ public interface MessageReader {
*/
public int readInt(String name, int dflt);
+ /**
+ * Reads an {@code Integer} value.
+ *
+ * @param name Field name.
+ * @return {@code Integer} value.
+ */
+ @Nullable
+ public Integer readInteger(String name);
Review Comment:
```suggestion
public @Nullable Integer readInteger(String name);
```
##########
modules/client/src/test/java/org/apache/ignite/client/fakes/FakeInternalTable.java:
##########
@@ -91,7 +91,7 @@ public int partitions() {
/** {@inheritDoc} */
Review Comment:
```suggestion
```
##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/DirectMessageReader.java:
##########
@@ -124,6 +124,18 @@ public int readInt(String name, int dflt) {
return readInt(name);
}
+ /** {@inheritDoc} */
Review Comment:
```suggestion
```
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/index/IndexValidatorImpl.java:
##########
@@ -127,6 +125,17 @@ private void validate(
}
}
+ @Nullable
+ private static TableView findTableById(int tableId, NamedListView<? extends TableView> tablesView) {
Review Comment:
```suggestion
private static @Nullable TableView findTableById(int tableId, NamedListView<? extends TableView> tablesView) {
```
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/HashIndexSpoolPlannerTest.java:
##########
@@ -91,6 +91,10 @@ public void testSingleKey() throws Exception {
assertNull(searchRow.get(2));
}
+ private static int randomTableId() {
+ return new Random().nextInt();
Review Comment:
Why not 1 ?)
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AbstractAggregatePlannerTest.java:
##########
@@ -541,7 +541,7 @@ private static IgniteSchema schema(IgniteDistribution distribution,
}
private static IgniteDistribution hash() {
- return IgniteDistributions.affinity(0, UUID.randomUUID(), DEFAULT_ZONE_ID);
+ return IgniteDistributions.affinity(0, new Random().nextInt(), DEFAULT_ZONE_ID);
Review Comment:
Why not 1 ?)
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/SetOpPlannerTest.java:
##########
@@ -74,11 +74,15 @@ public void setup() {
createTable(publicSchema, "BROADCAST_TBL2", type, IgniteDistributions.broadcast());
createTable(publicSchema, "SINGLE_TBL1", type, IgniteDistributions.single());
createTable(publicSchema, "SINGLE_TBL2", type, IgniteDistributions.single());
- createTable(publicSchema, "AFFINITY_TBL1", type, IgniteDistributions.affinity(0, UUID.randomUUID(), DEFAULT_ZONE_ID));
+ createTable(publicSchema, "AFFINITY_TBL1", type, IgniteDistributions.affinity(0, randomTableId(), DEFAULT_ZONE_ID));
createTable(publicSchema, "HASH_TBL1", type, IgniteDistributions.hash(List.of(0)));
- createTable(publicSchema, "AFFINITY_TBL2", type, IgniteDistributions.affinity(0, UUID.randomUUID(), DEFAULT_ZONE_ID));
- createTable(publicSchema, "AFFINITY_TBL3", type, IgniteDistributions.affinity(1, UUID.randomUUID(), DEFAULT_ZONE_ID));
- createTable(publicSchema, "AFFINITY_TBL4", type, IgniteDistributions.affinity(0, UUID.randomUUID(), DEFAULT_ZONE_ID + 1));
+ createTable(publicSchema, "AFFINITY_TBL2", type, IgniteDistributions.affinity(0, randomTableId(), DEFAULT_ZONE_ID));
+ createTable(publicSchema, "AFFINITY_TBL3", type, IgniteDistributions.affinity(1, randomTableId(), DEFAULT_ZONE_ID));
+ createTable(publicSchema, "AFFINITY_TBL4", type, IgniteDistributions.affinity(0, randomTableId(), DEFAULT_ZONE_ID + 1));
+ }
+
+ private static int randomTableId() {
+ return new Random().nextInt();
Review Comment:
why not 1 ?)
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/HashIndexDescriptor.java:
##########
@@ -151,6 +153,17 @@ private static List<HashIndexColumnDescriptor> extractIndexColumnsConfiguration(
.collect(toUnmodifiableList());
}
+ @Nullable
+ private static TableView findTableById(int tableId, NamedListView<? extends TableView> tablesView) {
Review Comment:
```suggestion
private static @Nullable TableView findTableById(int tableId, NamedListView<? extends TableView> tablesView) {
```
##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/stream/DirectByteBufferStream.java:
##########
@@ -290,6 +298,14 @@ <K, V> void writeMap(Map<K, V> map, MessageCollectionItemType keyType, MessageCo
*/
int readInt();
+ /**
+ * Reads {@code Integer}.
+ *
+ * @return Value.
+ */
+ @Nullable
+ Integer readInteger();
Review Comment:
```suggestion
@Nullable Integer readInteger();
```
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/HashIndexPlannerTest.java:
##########
@@ -64,13 +65,17 @@ public void hashIndexIsAppliedForEquiCondition() throws Exception {
assertThat(invalidPlanMsg, scan.indexName(), equalTo(indexName));
}
+ private static IgniteDistribution affinity() {
+ return IgniteDistributions.affinity(0, new Random().nextInt(), DEFAULT_ZONE_ID);
Review Comment:
Why not 1 ?)
##########
modules/client/src/test/java/org/apache/ignite/client/PartitionAwarenessTest.java:
##########
@@ -457,7 +456,7 @@ private Table defaultTable() {
private Table table(String name) {
// Create table on both servers with the same ID.
- var tableId = UUID.randomUUID();
Review Comment:
Maybe a random positive number?
##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/stream/DirectByteBufferStreamImplV1.java:
##########
@@ -909,6 +924,24 @@ public int readInt() {
return val;
}
+ /** {@inheritDoc} */
Review Comment:
```suggestion
```
##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/DirectMessageWriter.java:
##########
@@ -116,6 +116,16 @@ public boolean writeInt(String name, int val) {
return stream.lastFinished();
}
+ /** {@inheritDoc} */
Review Comment:
```suggestion
```
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/JoinColocationPlannerTest.java:
##########
@@ -77,14 +77,18 @@ public void joinSameTableSimpleAff() throws Exception {
assertThat(invalidPlanMsg, join.getRight(), instanceOf(IgniteIndexScan.class));
}
+ private static int randomTableId() {
+ return new Random().nextInt();
Review Comment:
why not 1 ?)
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/PlannerTest.java:
##########
@@ -192,6 +192,10 @@ public ColocationGroup colocationGroup(MappingQueryContext ctx) {
assertEquals(2, plan.fragments().size());
}
+ private static int randomTableId() {
+ return new Random().nextInt();
Review Comment:
why not 1 ?)
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/index/IndexValidatorImplTest.java:
##########
@@ -40,13 +39,20 @@
*/
@ExtendWith(ConfigurationExtension.class)
public class IndexValidatorImplTest {
+ private static final int TABLE_ID = 1;
+
@InjectConfiguration("mock.tables.fooTable {columns.column0 {type.type: STRING}}")
private TablesConfiguration tablesConfig;
+ @BeforeEach
+ void setupTableConfig() {
+ tablesConfig.tables().get("fooTable").id().update(TABLE_ID);
Review Comment:
should we wait for the future?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/JoinCommutePlannerTest.java:
##########
@@ -83,12 +83,16 @@ public IgniteDistribution distribution() {
@Override
public IgniteDistribution distribution() {
- return IgniteDistributions.affinity(0, UUID.randomUUID(), DEFAULT_ZONE_ID);
+ return IgniteDistributions.affinity(0, randomTableId(), DEFAULT_ZONE_ID);
}
}
);
}
+ private static int randomTableId() {
+ return new Random().nextInt();
Review Comment:
why not 1 ?)
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/TableSpoolPlannerTest.java:
##########
@@ -82,4 +83,8 @@ public void tableSpoolDistributed() throws Exception {
checkSplitAndSerialization(phys, publicSchema);
}
+
+ private static IgniteDistribution affinity() {
+ return IgniteDistributions.affinity(0, new Random().nextInt(), DEFAULT_ZONE_ID);
Review Comment:
why not 1 ?)
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java:
##########
@@ -180,6 +181,17 @@ private static BinaryTupleSchema createSchema(List<SortedIndexColumnDescriptor>
return BinaryTupleSchema.create(elements);
}
+ @Nullable
+ private static TableView findTableById(int tableId, NamedListView<? extends TableView> tablesView) {
Review Comment:
```suggestion
private static @Nullable TableView findTableById(int tableId, NamedListView<? extends TableView> tablesView) {
```
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/UnionPlannerTest.java:
##########
@@ -109,9 +109,13 @@ private IgniteSchema prepareSchema() {
.add("NAME", f.createJavaType(String.class))
.add("SALARY", f.createJavaType(Double.class))
.build(),
- IgniteDistributions.affinity(0, UUID.randomUUID(), DEFAULT_ZONE_ID)
+ IgniteDistributions.affinity(0, randomTableId(), DEFAULT_ZONE_ID)
);
return publicSchema;
}
+
+ private static int randomTableId() {
+ return new Random().nextInt();
Review Comment:
why not 1 ?)
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/SortedIndexSpoolPlannerTest.java:
##########
@@ -100,6 +101,10 @@ public void testNotColocatedEqJoin() throws Exception {
assertNull(searchBounds.get(1));
}
+ private static IgniteDistribution affinity() {
+ return IgniteDistributions.affinity(0, new Random().nextInt(), DEFAULT_ZONE_ID);
Review Comment:
why not 1 ?)
##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbMvPartitionStorage.java:
##########
@@ -1121,7 +1124,7 @@ static boolean invalid(RocksIterator it) {
*/
private static ReadResult wrapUncommittedValue(RowId rowId, byte[] valueBytes, @Nullable HybridTimestamp newestCommitTs) {
UUID txId = bytesToUuid(valueBytes, TX_ID_OFFSET);
- UUID commitTableId = bytesToUuid(valueBytes, TABLE_ID_OFFSET);
+ int commitTableId = GridUnsafe.getInt(valueBytes, GridUnsafe.BYTE_ARR_OFF + TABLE_ID_OFFSET);
Review Comment:
Please use static import
--
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