tkalkirill commented on code in PR #2441:
URL: https://github.com/apache/ignite-3/pull/2441#discussion_r1295391314
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java:
##########
@@ -138,4 +169,35 @@ public CompletableFuture<ColocationGroup>
fetchColocationGroup() {
});
}
}
+
+ private static CacheKey cacheKey(int tableId, int version) {
+ return new CacheKey(tableId, version);
+ }
+
+ private static class CacheKey {
+ private final int version;
Review Comment:
`private final int tableVersion;`
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteSchema.java:
##########
@@ -17,135 +17,54 @@
package org.apache.ignite.internal.sql.engine.schema;
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import java.util.Collection;
import java.util.Collections;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.apache.calcite.schema.Table;
import org.apache.calcite.schema.impl.AbstractSchema;
-import org.jetbrains.annotations.Nullable;
+import org.apache.ignite.internal.util.CollectionUtils;
/**
- * Ignite schema.
+ * Schema implementation for sql engine.
*/
public class IgniteSchema extends AbstractSchema {
- static final long INITIAL_VERSION = -1;
- private final String schemaName;
+ private final String name;
- private final Map<String, Table> tblMap;
+ private final int version;
- private final Map<Integer, IgniteIndex> idxMap;
+ private final Map<String, IgniteTable> tableMapByName;
+ private final Int2ObjectMap<IgniteTable> tableMapById;
Review Comment:
I think it will be enough: `tableById`
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteSchema.java:
##########
@@ -17,135 +17,54 @@
package org.apache.ignite.internal.sql.engine.schema;
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import java.util.Collection;
import java.util.Collections;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.apache.calcite.schema.Table;
import org.apache.calcite.schema.impl.AbstractSchema;
-import org.jetbrains.annotations.Nullable;
+import org.apache.ignite.internal.util.CollectionUtils;
/**
- * Ignite schema.
+ * Schema implementation for sql engine.
*/
public class IgniteSchema extends AbstractSchema {
- static final long INITIAL_VERSION = -1;
- private final String schemaName;
+ private final String name;
- private final Map<String, Table> tblMap;
+ private final int version;
- private final Map<Integer, IgniteIndex> idxMap;
+ private final Map<String, IgniteTable> tableMapByName;
Review Comment:
I think it will be enough: `tableByName`
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/TestBuilders.java:
##########
@@ -430,15 +424,18 @@ public TestTable build() {
throw new IllegalArgumentException("Table must contain at
least one column");
}
- TestTable testTable = new TestTable(
- new TableDescriptorImpl(columns, distribution),
- Objects.requireNonNull(name),
- size
- );
+ TableDescriptorImpl tableDescriptor = new
TableDescriptorImpl(columns, distribution);
-
indexBuilders.stream().map(AbstractIndexBuilderImpl::build).forEach(testTable::addIndex);
+ List<IgniteIndex> indexMap = indexBuilders.stream()
Review Comment:
Map ?)
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/CatalogSqlSchemaManager.java:
##########
@@ -66,7 +66,11 @@ public class CatalogSqlSchemaManager implements
SqlSchemaManager {
/** Constructor. */
public CatalogSqlSchemaManager(CatalogManager catalogManager, int
cacheSize) {
this.catalogManager = catalogManager;
- this.cache =
Caffeine.newBuilder().maximumSize(cacheSize).<Map.Entry<String, Integer>,
SchemaPlus>build().asMap();
+ this.cache = Caffeine.newBuilder()
+ .initialCapacity(cacheSize)
+ .maximumSize(cacheSize)
+ .<Map.Entry<String, Integer>, SchemaPlus>build()
Review Comment:
What a pain, what a pain)
Let's use a separate class with fields.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java:
##########
@@ -138,4 +169,35 @@ public CompletableFuture<ColocationGroup>
fetchColocationGroup() {
});
}
}
+
+ private static CacheKey cacheKey(int tableId, int version) {
+ return new CacheKey(tableId, version);
+ }
+
+ private static class CacheKey {
Review Comment:
Maybe it's better to rename it to "TableIdWithVersion"?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/TestBuilders.java:
##########
@@ -351,16 +350,11 @@ public TestCluster build() {
}
}
- Map<String, Table> tableMap = tableBuilders.stream()
+ Map<String, IgniteTable> tableMap = tableBuilders.stream()
Review Comment:
tableByName
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/TestBuilders.java:
##########
@@ -880,11 +879,11 @@ static class TestColocationGroupProvider implements
ColocationGroupProvider {
private final Map<Integer, CompletableFuture<ColocationGroup>> groups
= new HashMap<>();
- TestColocationGroupProvider(List<ClusterTableBuilderImpl>
tableBuilders, Map<String, Table> tableMap, List<String> nodeNames) {
+ TestColocationGroupProvider(List<ClusterTableBuilderImpl>
tableBuilders, Map<String, IgniteTable> tableMap,
+ List<String> nodeNames) {
for (ClusterTableBuilderImpl tableBuilder : tableBuilders) {
- Table table = tableMap.get(tableBuilder.name);
- IgniteTable igniteTable = (IgniteTable) table;
+ IgniteTable table = tableMap.get(tableBuilder.name);
Review Comment:
tableByName
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -118,6 +115,9 @@ public class SqlQueryProcessor implements QueryProcessor {
/** Size of the table access cache. */
private static final int TABLE_CACHE_SIZE = 1024;
+ /** Size of the schema cache. */
Review Comment:
Please specify that this is the size in items.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutableTableRegistryImpl.java:
##########
@@ -77,6 +83,22 @@ public void onSchemaUpdated() {
tableCache.clear();
}
+ private CompletableFuture<ExecutableTable> loadTable(String tableName,
TableDescriptor tableDescriptor) {
+ return tableManager.tableAsyncInternal(tableName.toUpperCase())
+ .thenApply(table -> {
+ InternalTable internalTable = table.internalTable();
+ SchemaRegistry schemaRegistry =
schemaManager.schemaRegistry(table.tableId());
+ SchemaDescriptor schemaDescriptor =
schemaRegistry.schema();
+ TableRowConverter rowConverter = new
TableRowConverterImpl(schemaRegistry, schemaDescriptor, tableDescriptor);
+ ScannableTable scannableTable = new
ScannableTableImpl(internalTable, rowConverter, tableDescriptor);
+
+ UpdatableTableImpl updatableTable = new
UpdatableTableImpl(table.tableId(), tableDescriptor, internalTable.partitions(),
+ replicaService, clock, rowConverter,
schemaDescriptor);
+
+ return new ExecutableTableImpl(internalTable,
scannableTable, updatableTable);
Review Comment:
Let's create a ticket for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
