This is an automated email from the ASF dual-hosted git repository. amashenkov pushed a commit to branch ignite-19497 in repository https://gitbox.apache.org/repos/asf/ignite-3.git
commit dc013412a08c55106f53f8e4ea5b761bfeb8e8cb Author: amashenkov <[email protected]> AuthorDate: Tue Aug 15 13:18:24 2023 +0300 Fix table cache key. --- .../sql/engine/exec/ExecutableTableRegistry.java | 5 +++-- .../engine/exec/ExecutableTableRegistryImpl.java | 22 +++++++++++----------- .../exec/ExecutionDependencyResolverImpl.java | 3 ++- .../exec/ExecutableTableRegistrySelfTest.java | 3 ++- .../exec/ExecutionDependencyResolverSelfTest.java | 14 +++++++------- .../engine/exec/TestExecutableTableRegistry.java | 6 +++--- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistry.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistry.java index 9c6113fb1a..184e6ab5c0 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistry.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistry.java @@ -29,12 +29,13 @@ public interface ExecutableTableRegistry { * Returns an table that can be used for both read and write operations. * * @param tableId Table Id. + * @param tableVersion Table version. * @param tableDescriptor Table descriptor. * @return An operation that returns executable table. */ - CompletableFuture<ExecutableTable> getTable(int schemaVersion, int tableId, TableDescriptor tableDescriptor); + CompletableFuture<ExecutableTable> getTable(int tableId, int tableVersion, TableDescriptor tableDescriptor); // TODO IGNITE-19499 use tableId instead of name. @Deprecated(forRemoval = true) - CompletableFuture<ExecutableTable> getTable(int schemaVersion, int tableId, String tableName, TableDescriptor tableDescriptor); + CompletableFuture<ExecutableTable> getTable(int tableId, int tableVersion, String tableName, TableDescriptor tableDescriptor); } diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java index f54654cac5..40a4114f27 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java @@ -69,14 +69,14 @@ public class ExecutableTableRegistryImpl implements ExecutableTableRegistry, Sch /** {@inheritDoc} */ @Override - public CompletableFuture<ExecutableTable> getTable(int schemaVersion, int tableId, TableDescriptor tableDescriptor) { - return tableCache.computeIfAbsent(cacheKey(schemaVersion, tableId), (k) -> loadTable(tableId, tableDescriptor)); + public CompletableFuture<ExecutableTable> getTable(int tableId, int tableVersion, TableDescriptor tableDescriptor) { + return tableCache.computeIfAbsent(cacheKey(tableId, tableVersion), (k) -> loadTable(tableId, tableDescriptor)); } // TODO IGNITE-19499: Drop this temporal method to get table by name. @Override - public CompletableFuture<ExecutableTable> getTable(int schemaVersion, int tableId, String tableName, TableDescriptor tableDescriptor) { - return tableCache.computeIfAbsent(cacheKey(schemaVersion, tableId), (k) -> loadTable(tableName, tableDescriptor)); + public CompletableFuture<ExecutableTable> getTable(int tableId, int tableVersion, String tableName, TableDescriptor tableDescriptor) { + return tableCache.computeIfAbsent(cacheKey(tableId, tableVersion), (k) -> loadTable(tableName, tableDescriptor)); } /** {@inheritDoc} */ @@ -170,16 +170,16 @@ public class ExecutableTableRegistryImpl implements ExecutableTableRegistry, Sch } } - private static CacheKey cacheKey(int schemaVersion, int tableId) { - return new CacheKey(schemaVersion, tableId); + private static CacheKey cacheKey(int tableId, int version) { + return new CacheKey(tableId, version); } private static class CacheKey { - private final int schemaVersion; + private final int version; private final int tableId; - public CacheKey(int schemaVersion, int tableId) { - this.schemaVersion = schemaVersion; + public CacheKey(int tableId, int version) { + this.version = version; this.tableId = tableId; } @@ -192,12 +192,12 @@ public class ExecutableTableRegistryImpl implements ExecutableTableRegistry, Sch return false; } CacheKey cacheKey = (CacheKey) o; - return schemaVersion == cacheKey.schemaVersion && tableId == cacheKey.tableId; + return version == cacheKey.version && tableId == cacheKey.tableId; } @Override public int hashCode() { - return Objects.hash(schemaVersion, tableId); + return Objects.hash(version, tableId); } } } diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverImpl.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverImpl.java index 804f640860..3859307454 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverImpl.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverImpl.java @@ -83,8 +83,9 @@ public class ExecutionDependencyResolverImpl implements ExecutionDependencyResol TableDescriptor tableDescriptor = igniteTable.descriptor(); //TODO IGNITE-19499 Use id instead of name. String tableName = igniteTable.name(); + int tableVersion = igniteTable.version(); - tableMap.computeIfAbsent(tableId, (id) -> registry.getTable(schemaVersion, tableId, tableName, tableDescriptor)); + tableMap.computeIfAbsent(tableId, (id) -> registry.getTable(tableId, tableVersion, tableName, tableDescriptor)); } }; diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistrySelfTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistrySelfTest.java index 740a1cba71..6f49c2ebba 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistrySelfTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistrySelfTest.java @@ -144,13 +144,14 @@ public class ExecutableTableRegistrySelfTest { CompletableFuture<ExecutableTable> getTable(int tableId) { TableImpl table = new TableImpl(internalTable, schemaRegistry, new HeapLockManager()); int schemaVersion = 1; + int tableVersion = 1; SchemaDescriptor schemaDescriptor = newDescriptor(schemaVersion); when(tableManager.tableAsync(tableId)).thenReturn(CompletableFuture.completedFuture(table)); when(schemaManager.schemaRegistry(tableId)).thenReturn(schemaRegistry); when(schemaRegistry.schema()).thenReturn(schemaDescriptor); - return registry.getTable(schemaVersion, tableId, descriptor); + return registry.getTable(tableId, tableVersion, descriptor); } } } diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverSelfTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverSelfTest.java index cc5a2e4d7a..9db847f33a 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverSelfTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ExecutionDependencyResolverSelfTest.java @@ -103,8 +103,8 @@ public class ExecutionDependencyResolverSelfTest extends AbstractPlannerTest { tester.checkDependencies(deps, t1Id); tester.checkDependencies(deps, t2Id); - verify(registry, times(1)).getTable(anyInt(), eq(t1Id), anyString(), same(td1)); - verify(registry, times(1)).getTable(anyInt(), eq(t2Id), anyString(), same(td2)); + verify(registry, times(1)).getTable(eq(t1Id), anyInt(), anyString(), same(td1)); + verify(registry, times(1)).getTable(eq(t2Id), anyInt(), anyString(), same(td2)); } /** @@ -123,7 +123,7 @@ public class ExecutionDependencyResolverSelfTest extends AbstractPlannerTest { CompletableFuture<ResolvedDependencies> f = tester.resolveDependencies("SELECT * FROM test1 WHERE id=1"); tester.checkDependencies(f.join(), t1Id); - verify(registry, times(1)).getTable(anyInt(), eq(t1Id), anyString(), same(td1)); + verify(registry, times(1)).getTable(eq(t1Id), anyInt(), anyString(), same(td1)); } /** @@ -151,8 +151,8 @@ public class ExecutionDependencyResolverSelfTest extends AbstractPlannerTest { tester.checkDependencies(deps, t1Id); tester.checkDependencies(deps, t2Id); - verify(registry, times(1)).getTable(anyInt(), eq(t1Id), anyString(), same(td1)); - verify(registry, times(1)).getTable(anyInt(), eq(t2Id), anyString(), same(td2)); + verify(registry, times(1)).getTable(eq(t1Id), anyInt(), anyString(), same(td1)); + verify(registry, times(1)).getTable(eq(t2Id), anyInt(), anyString(), same(td2)); } /** @@ -261,14 +261,14 @@ public class ExecutionDependencyResolverSelfTest extends AbstractPlannerTest { CompletableFuture<ExecutableTable> f = CompletableFuture.completedFuture(executableTable); - when(registry.getTable(anyInt(), eq(tableId), anyString(), any(TableDescriptor.class))).thenReturn(f); + when(registry.getTable(eq(tableId), anyInt(), anyString(), any(TableDescriptor.class))).thenReturn(f); } void setError(int tableId, Throwable err) { CompletableFuture<ExecutableTable> f = new CompletableFuture<>(); f.completeExceptionally(err); - when(registry.getTable(anyInt(), eq(tableId), anyString(), any(TableDescriptor.class))).thenReturn(f); + when(registry.getTable(eq(tableId), anyInt(), anyString(), any(TableDescriptor.class))).thenReturn(f); } void setColocationGroup(int tableId, CompletableFuture<ColocationGroup> group) { diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/TestExecutableTableRegistry.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/TestExecutableTableRegistry.java index d9731c9d50..0de1a47e0f 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/TestExecutableTableRegistry.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/TestExecutableTableRegistry.java @@ -40,13 +40,13 @@ public final class TestExecutableTableRegistry implements ExecutableTableRegistr /** {@inheritDoc} */ @Override - public CompletableFuture<ExecutableTable> getTable(int schemaVersion, int tableId, TableDescriptor tableDescriptor) { + public CompletableFuture<ExecutableTable> getTable(int tableId, int schemaVersion, TableDescriptor tableDescriptor) { return CompletableFuture.completedFuture(new TestExecutableTable(tableId, colocationGroupProvider)); } @Override - public CompletableFuture<ExecutableTable> getTable(int schemaVersion, int tableId, String tableName, TableDescriptor tableDescriptor) { - return this.getTable(schemaVersion, tableId, tableDescriptor); + public CompletableFuture<ExecutableTable> getTable(int tableId, int tableVersion, String tableName, TableDescriptor tableDescriptor) { + return this.getTable(tableId, tableVersion, tableDescriptor); } private static final class TestExecutableTable implements ExecutableTable {
