korlov42 commented on code in PR #2101:
URL: https://github.com/apache/ignite-3/pull/2101#discussion_r1206561794
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/logical/ExposeIndexRule.java:
##########
@@ -31,6 +31,7 @@
import org.apache.calcite.util.ImmutableBitSet;
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;
Review Comment:
why do you need this import?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/AbstractIndexScan.java:
##########
@@ -111,33 +112,44 @@ public RelNode accept(RexShuttle shuttle) {
return super.accept(shuttle);
}
- /**
- * Get index name.
- * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
- */
+ /** Return index name. */
public String indexName() {
return idxName;
}
+ /** Index type. */
+ public Type indexType() {
+ return type;
+ }
+
/** {@inheritDoc} */
@Override
public RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery
mq) {
double rows = table.getRowCount();
- double cost;
+ double cost = 0;
+
+ if (type == Type.HASH) {
+ boolean notExact = (searchBounds() == null)
+ || (searchBounds().stream().anyMatch(bound -> bound.type()
!= SearchBounds.Type.EXACT));
Review Comment:
I think, `SearchBounds.Type.MULTI` type is legit as well
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/AbstractIndexScan.java:
##########
@@ -111,33 +112,44 @@ public RelNode accept(RexShuttle shuttle) {
return super.accept(shuttle);
}
- /**
- * Get index name.
- * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
- */
+ /** Return index name. */
public String indexName() {
return idxName;
}
+ /** Index type. */
+ public Type indexType() {
Review Comment:
indexType is not used
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -552,14 +559,36 @@ public Double getRowCount() {
continue;
}
- try {
- size += part.rowsCount();
- } catch (StorageRebalanceException ignore) {
- // No-op.
- }
+ long upd = part.lastAppliedIndex();
+
+ size += upd;
Review Comment:
I'm not sure what size you are counting here
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/HashIndexPlannerTest.java:
##########
@@ -88,6 +90,27 @@ public void hashIndexIsNotAppliedForRangeCondition() throws
Exception {
String invalidPlanMsg = "Invalid plan:\n" + RelOptUtil.toString(phys);
assertThat(invalidPlanMsg, scan, notNullValue());
+
+ // Can`t use hash index scan with range.
+ String sqlGT = "SELECT /*+
DISABLE_RULE('LogicalTableScanConverterRule')*/ id FROM test_tbl WHERE val >=
10";
+
+ IgniteTestUtils.assertThrowsWithCause(() -> physicalPlan(sqlGT,
schema), CannotPlanException.class,
Review Comment:
let's split this test in order to have single test case per method
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteTableImpl.java:
##########
@@ -111,6 +114,9 @@ public class IgniteTableImpl extends AbstractTable
implements IgniteTable, Updat
private final PartitionExtractor partitionExtractor;
+ /** Triggers statistic update. */
+ private static AtomicBoolean updateStat = new AtomicBoolean();
Review Comment:
don't quite understand why do we need this flag. It only prevents from
updating stats before first insert, but why?
--
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]
