zstan commented on code in PR #2441:
URL: https://github.com/apache/ignite-3/pull/2441#discussion_r1294661606
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -17,81 +17,55 @@
package org.apache.ignite.internal.sql.engine.schema;
-import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.function.DoubleSupplier;
-import org.apache.calcite.plan.Convention;
+import org.apache.calcite.config.CalciteConnectionConfig;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.plan.RelOptTable;
import org.apache.calcite.plan.RelTraitSet;
-import org.apache.calcite.rel.RelCollation;
-import org.apache.calcite.rel.RelCollations;
+import org.apache.calcite.rel.core.TableScan;
import org.apache.calcite.rel.hint.RelHint;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.schema.Schema;
import org.apache.calcite.schema.Statistic;
import org.apache.calcite.schema.impl.AbstractTable;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.util.ImmutableBitSet;
-import org.apache.ignite.internal.distributionzones.DistributionZoneManager;
-import
org.apache.ignite.internal.sql.engine.rel.logical.IgniteLogicalIndexScan;
import
org.apache.ignite.internal.sql.engine.rel.logical.IgniteLogicalTableScan;
-import org.apache.ignite.internal.sql.engine.schema.IgniteIndex.Type;
import org.apache.ignite.internal.sql.engine.trait.IgniteDistribution;
-import org.apache.ignite.internal.sql.engine.trait.TraitUtils;
import org.apache.ignite.internal.sql.engine.type.IgniteTypeFactory;
-import org.apache.ignite.internal.storage.MvPartitionStorage;
-import org.apache.ignite.internal.storage.StorageRebalanceException;
-import org.apache.ignite.internal.table.InternalTable;
import org.jetbrains.annotations.Nullable;
/**
- * Ignite table implementation.
+ * Table for calcite schema.
Review Comment:
i think no need to mention implementation ?
Table for sql engine. ? or smth similar
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/CatalogSqlSchemaManager.java:
##########
@@ -141,20 +133,20 @@ private static SchemaPlus createSqlSchema(int version,
CatalogSchemaDescriptor s
assert descriptor != null;
IgniteStatistic statistic = new IgniteStatistic(() -> 0.0d,
descriptor.distribution());
- Map<String, IgniteSchemaIndex> tableIndexMap =
schemaTableIndexes.getOrDefault(tableId, Collections.emptyMap());
+ Map<String, IgniteIndex> tableIndexMap =
schemaTableIndexes.getOrDefault(tableId, Collections.emptyMap());
- IgniteSchemaTable schemaTable = new IgniteSchemaTable(tableName,
tableId, version, descriptor, statistic, tableIndexMap);
+ IgniteTable schemaTable = new IgniteTableImpl(tableName, tableId,
version, descriptor, statistic, tableIndexMap);
- schemaTables.put(tableName, schemaTable);
+ schemaTables.add(schemaTable);
}
// create root schema
SchemaPlus rootSchema = Frameworks.createRootSchema(false);
- IgniteCatalogSchema igniteSchema = new IgniteCatalogSchema(schemaName,
version, schemaTables);
+ IgniteSchema igniteSchema = new IgniteSchema(schemaName, version,
schemaTables);
return rootSchema.add(schemaName, igniteSchema);
}
- private static IgniteSchemaIndex createSchemaIndex(CatalogIndexDescriptor
indexDescriptor, TableDescriptor tableDescriptor) {
+ private static IgniteIndex createSchemaIndex(CatalogIndexDescriptor
indexDescriptor, TableDescriptor tableDescriptor) {
Review Comment:
createSchemaIndex is sounds like : index for schema, a little weird as for
me, probably : assemblyIndex(Definition|Description) ? but it`s up to you
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/trait/TraitUtils.java:
##########
@@ -398,6 +397,22 @@ private static boolean fillRecursive(
return processed;
}
+ /**
+ * Creates {@link RelCollation} object from a given collations.
+ *
+ * @param collations Original collation.
+ * @return a {@link RelCollation} object.
+ */
+ public static RelCollation trimmedCollation(List<RelFieldCollation>
collations) {
Review Comment:
fking ass hole, not trim our collations !))
plz rename it somehow ? )
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteSchema.java:
##########
@@ -17,135 +17,63 @@
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.List;
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;
+import org.jetbrains.annotations.TestOnly;
/**
- * Ignite schema.
+ * Schema adapter for apache calcite.
*/
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;
- private final long schemaVersion;
-
- /**
- * Creates a Schema with given tables and indexes.
- *
- * @param schemaName A name of the schema to create.
- * @param tableMap A collection of a tables belonging to the schema.
- * @param indexMap A collection of an indexes belonging to the schema.
- */
- public IgniteSchema(
- String schemaName,
- @Nullable Map<String, Table> tableMap,
- @Nullable Map<Integer, IgniteIndex> indexMap,
- long schemaVersion
- ) {
- this.schemaName = schemaName;
- this.tblMap = tableMap == null ? new ConcurrentHashMap<>() : new
ConcurrentHashMap<>(tableMap);
- this.idxMap = indexMap == null ? new ConcurrentHashMap<>() : new
ConcurrentHashMap<>(indexMap);
- this.schemaVersion = schemaVersion;
+ /** Constructor. */
+ public IgniteSchema(String name, int version, Collection<IgniteTable>
tables) {
+ this.name = name;
+ this.version = version;
+ this.tableMapByName = tables.stream().collect(Collectors.toMap(t ->
t.name().toUpperCase(), Function.identity()));
+ this.tableMapById =
tables.stream().collect(CollectionUtils.toIntMapCollector(IgniteTable::id,
Function.identity()));
}
- /**
- * Creates an empty Schema.
- *
- * @param schemaName A name of the schema to create.
- */
- public IgniteSchema(String schemaName) {
- this(schemaName, null, null, INITIAL_VERSION);
+ /** Constructor. */
+ @TestOnly
+ public IgniteSchema(String name) {
Review Comment:
used only in 3 tests, you can easily change it, i suppose ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -172,7 +173,7 @@ public static <RowT> ExecutionServiceImpl<RowT> create(
dependencyResolver,
(ctx, deps) -> new LogicalRelImplementor<>(
ctx,
- new HashFunctionFactoryImpl<>(sqlSchemaManager,
handler),
+ new
HashFunctionFactoryImpl<>(ctx.getRootSchema().unwrap(IgniteSchema.class),
handler),
Review Comment:
I also talk about this place : public RowHashFunction<T> create(int[]
fields, int tableId) {
...
TableDescriptor tblDesc = schema.getTable(tableId).descriptor();
we need only descriptor in this implementation if we would need smth else we
will change incoming params, no ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -174,103 +139,43 @@ public IgniteDistribution distribution() {
/** {@inheritDoc} */
@Override
- public Map<String, IgniteIndex> indexes() {
- return Collections.unmodifiableMap(indexes);
+ public Schema.TableType getJdbcTableType() {
+ return Schema.TableType.TABLE;
}
/** {@inheritDoc} */
@Override
- public void addIndex(IgniteIndex idxTbl) {
- indexes.put(idxTbl.name(), idxTbl);
+ public boolean isRolledUp(String column) {
+ return false;
}
/** {@inheritDoc} */
@Override
- public IgniteIndex getIndex(String idxName) {
- return indexes.get(idxName);
+ public boolean rolledUpColumnValidInsideAgg(String column, SqlCall call,
+ @Nullable SqlNode parent,
+ @Nullable CalciteConnectionConfig config) {
+ return false;
}
/** {@inheritDoc} */
@Override
- public void removeIndex(String idxName) {
- indexes.remove(idxName);
+ public Statistic getStatistic() {
+ return statistic;
}
/** {@inheritDoc} */
@Override
- public <C> C unwrap(Class<C> cls) {
+ public <C> @Nullable C unwrap(Class<C> cls) {
if (cls.isInstance(desc)) {
return cls.cast(desc);
}
return super.unwrap(cls);
}
- static DoubleSupplier rowCountStatistic(InternalTable table) {
- return new RowCountStatistic(table);
- }
-
- private static final class RowCountStatistic implements DoubleSupplier {
Review Comment:
I can`t found new statistic implementation (
Also you remove informational issue // TODO: need to be refactored
https://issues.apache.org/jira/browse/IGNITE-19558 we need to remember about it
)
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteSchema.java:
##########
@@ -17,135 +17,63 @@
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.List;
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;
+import org.jetbrains.annotations.TestOnly;
/**
- * Ignite schema.
+ * Schema adapter for apache calcite.
Review Comment:
```suggestion
* Schema adapter for sql engine.
```
--
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]
