Skip to content

IGNITE-28899 Calcite. Fix litmus error during join - #13389

Open
zstan wants to merge 28 commits into
apache:masterfrom
zstan:ignite-28899
Open

zstan wants to merge 28 commits into
apache:masterfrom
zstan:ignite-28899

Conversation

@zstan

@zstan zstan commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

continue;

// Keys can relate to affinity not pk, just assumption here.
ImmutableIntList distKeys = table.distribution().getKeys();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course we need to have 'real pk' here, but fix it right now - seems too huge.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Check for columns description with key() is too huge?
  2. For broadcast table and for tables with affinity column defined estimation is wrong. I thing it's a relatively big part of all cases.

inputRows += mq.getRowCount(input);

double mem = 0.5 * inputRows * aggregateFieldsCount() * IgniteCost.AVERAGE_FIELD_SIZE;
double mem = 0.7 * inputRows * aggregateFieldsCount() * IgniteCost.AVERAGE_FIELD_SIZE;

@zstan zstan Jul 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empiric constants need to be fixed cause row count estimation is changed for now and some tests (2-3) are fails, all empirics need to be reevaluated during further (i hope) statistics activity changes.

prepareTables();

String sql = "SELECT sum(i.price * i.amount)" +
String sql = "SELECT /*+ MERGE_JOIN */ sum(i.price * i.amount)" +

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes i observe hashJoin here, i really don`t know what algo is more preferably here and i think it need to be discovered more accurate in further activities.

private static final int DATE_DIM_SIZE = 73_049;

/** */
private static final String SELECT = "SELECT /*+ DISABLE_RULE(" +

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course it work perfectly well without this disablings, i just afraid about some king of flaky...

@zstan
zstan marked this pull request as ready for review July 24, 2026 12:30
@zstan
zstan requested a review from Copilot July 24, 2026 12:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Updates Calcite planner metadata/costing and merge-join execution behavior to address a Calcite litmus failure seen during join planning/execution (IGNITE-28899), and adds regression tests to cover the scenario.

Changes:

  • Reworked join row-count estimation and adjusted join cost formulas/constants to influence plan selection.
  • Modified merge-join runtime node control flow around buffering and early termination.
  • Added new planner/integration tests and updated several existing assertions to match the new behavior.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
modules/calcite/src/test/java/org/apache/ignite/testsuites/PlannerTestSuite.java Registers the new join row-count estimation planner test in the suite.
modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java Adds the new litmus regression integration test to the suite.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/hints/JoinTypeHintPlannerTest.java Relaxes/adjusts hint-related plan assertions (notably around wrong-table hints).
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/UncollectPlannerTest.java Updates plan shape expectations for UNNEST + join planning.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/PlanSplitterTest.java Refactors colocation assignments setup to reuse a single assignments instance.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/JoinRowCountEstimationTest.java New planner tests validating join row-count estimation logic.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/CorrelatedSubqueryPlannerTest.java Updates plan assertion for table scan condition presence.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AggregatePlannerTest.java Removes an early return that skipped part of a test scenario.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/QueryBlockingTaskExecutorIntegrationTest.java Adds hint to force merge join and simplifies plan matching into a single regex check.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LitmusCheckIntegrationTest.java New integration test meant to catch Calcite litmus exceptions/regressions.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DistributedJoinIntegrationTest.java Forces MERGE_JOIN via hint for deterministic join plan coverage.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/Commons.java Adds a helper to build a target mapping from a projected bitset.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/set/IgniteSetOp.java Adjusts memory estimation coefficient used in set-op costing.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteNestedLoopJoin.java Revises NLJ cost computation to use metadata row-count and applies an additional penalty factor.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteMergeJoin.java Revises merge join cost computation to use metadata row-count.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteHashJoin.java Revises hash join cost computation to use metadata row-count.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteCorrelatedNestedLoopJoin.java Revises CNLJ cost computation to use metadata row-count.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/cost/IgniteCost.java Changes the hash lookup cost constant affecting planner decisions.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdSelectivity.java Updates metadata provider wiring and exposes selectivity constants for tests/consumers.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdRowCount.java Implements custom join row-count estimation with PK/FK heuristics and new metadata provider wiring.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdColumnOrigins.java Updates metadata provider wiring and minor indentation cleanup.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/MergeJoinNode.java Reworks buffering/join scheduling and replaces “finishing” cleanup with immediate downstream end conditions.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java Removes a previously-added workaround disabling Correlate assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@zstan
zstan force-pushed the ignite-28899 branch 3 times, most recently from a6dbda5 to eea65ee Compare August 19, 2026 13:54
assertPlan(sql, schema, hasFragmentsCount(3).and(hasChildThat(isInstanceOf(IgniteTrimExchange.class))));
// First table is replicated and planned with Exchange to colocate data, but set of nodes for partitioned
// table is differ, so exchange is added after fragments split for colocation.
assertPlan(sql, schema, hasFragmentsCount(3)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current plan:

IgniteMergeJoin(condition=[=($0, $3)], joinType=[inner]): rowcount = 5.625E9, cumulative cost = IgniteCost [rowCount=5.627075E9, cpu=2.2525784224829422E10, memory=4900000.0, io=0.0, network=4000000.0]
  IgniteSort(sort0=[$0], dir0=[ASC]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=575000.0, cpu=4600679.76331665, memory=900000.0, io=0.0, network=0.0]
    IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=2000000.0, memory=0.0, io=0.0, network=0.0]
  IgniteExchange(distribution=[single]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=1500000.0, cpu=2.118354506610649E7, memory=4000000.0, io=0.0, network=4000000.0]
    IgniteSort(sort0=[$0], dir0=[ASC]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=1000000.0, cpu=2.068354506610649E7, memory=4000000.0, io=0.0, network=0.0]
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=500000.0, memory=0.0, io=0.0, network=0.0]

master branch plan:

IgniteProject(ID=[$2], NAME=[$3], PROJECTID=[$4], ID0=[$0], VER0=[$1]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1672500.0, cpu=8572500.0, memory=1170000.0, io=0.0, network=225000.0]
  IgniteExchange(distribution=[single]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1661250.0, cpu=8561250.0, memory=1170000.0, io=0.0, network=225000.0]
    IgniteHashJoin(condition=[=($2, $0)], joinType=[inner]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1650000.0, cpu=8550000.0, memory=1170000.0, io=0.0, network=0.0]
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=500000.0, memory=0.0, io=0.0, network=0.0]
      IgniteTrimExchange(distribution=[affinity[identity=hash, cacheId=1355342585][0]]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=575000.0, cpu=2300000.0, memory=0.0, io=0.0, network=0.0]
        IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=2000000.0, memory=0.0, io=0.0, network=0.0]

// fragments split to send data to initiator node.
assertPlan(sql, schema, hasFragmentsCount(3).and(hasChildThat(isInstanceOf(IgniteTrimExchange.class))));
// table is differ, so exchange is added after fragments split for colocation.
assertPlan(sql, schema, hasFragmentsCount(2)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current plan:

IgniteMergeJoin(condition=[=($2, $3)], joinType=[inner], variablesSet=[[]], leftCollation=[[2]], rightCollation=[[0]]): rowcount = 5.625E9, cumulative cost = IgniteCost [rowCount=5.627075E9, cpu=2.2525784224829422E10, memory=4900000.0, io=0.0, network=4000000.0], id = 3041
  IgniteSort(sort0=[$2], dir0=[ASC]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=575000.0, cpu=4600679.76331665, memory=900000.0, io=0.0, network=0.0]
    IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=2000000.0, memory=0.0, io=0.0, network=0.0]
  IgniteExchange(distribution=[single]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=1500000.0, cpu=2.118354506610649E7, memory=4000000.0, io=0.0, network=4000000.0]
    IgniteSort(sort0=[$0], dir0=[ASC]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=1000000.0, cpu=2.068354506610649E7, memory=4000000.0, io=0.0, network=0.0]
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=500000.0, memory=0.0, io=0.0, network=0.0]

master branch plan:

IgniteProject(ID=[$2], NAME=[$3], PROJECTID=[$4], ID0=[$0], VER0=[$1]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1672500.0, cpu=8572500.0, memory=1170000.0, io=0.0, network=225000.0]
  IgniteExchange(distribution=[single]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1661250.0, cpu=8561250.0, memory=1170000.0, io=0.0, network=225000.0]
    IgniteHashJoin(condition=[=($4, $0)], joinType=[inner], variablesSet=[[]]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1650000.0, cpu=8550000.0, memory=1170000.0, io=0.0, network=0.0]
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=500000.0, memory=0.0, io=0.0, network=0.0]
      IgniteTrimExchange(distribution=[affinity[identity=hash, cacheId=1355342585][2]]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=575000.0, cpu=2300000.0, memory=0.0, io=0.0, network=0.0]
        IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=2000000.0, memory=0.0, io=0.0, network=0.0]

assertPlan(sql, schema, hasFragmentsCount(3).and(hasChildThat(isInstanceOf(IgniteTrimExchange.class))));
// First table is replicated and planned with Exchange to colocate data, but set of nodes for partitioned
// table is differ, so exchange is added after fragments split for colocation.
assertPlan(sql, schema, hasFragmentsCount(2)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current plan:

IgniteMergeJoin(condition=[=($0, $3)], joinType=[inner], variablesSet=[[]], leftCollation=[[0]], rightCollation=[[0]]): rowcount = 5.625E9, cumulative cost = IgniteCost [rowCount=5.627075E9, cpu=2.2525784224829422E10, memory=4900000.0, io=0.0, network=4000000.0]
  IgniteSort(sort0=[$0], dir0=[ASC]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=575000.0, cpu=4600679.76331665, memory=900000.0, io=0.0, network=0.0]
    IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=2000000.0, memory=0.0, io=0.0, network=0.0]
  IgniteExchange(distribution=[single]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=1500000.0, cpu=2.118354506610649E7, memory=4000000.0, io=0.0, network=4000000.0]
    IgniteSort(sort0=[$0], dir0=[ASC]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=1000000.0, cpu=2.068354506610649E7, memory=4000000.0, io=0.0, network=0.0]
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=500000.0, memory=0.0, io=0.0, network=0.0]

master branch plan:

IgniteProject(ID=[$2], NAME=[$3], PROJECTID=[$4], ID0=[$0], VER0=[$1]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1672500.0, cpu=8572500.0, memory=1170000.0, io=0.0, network=225000.0]
  IgniteExchange(distribution=[single]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1661250.0, cpu=8561250.0, memory=1170000.0, io=0.0, network=225000.0]
    IgniteHashJoin(condition=[=($2, $0)], joinType=[inner], variablesSet=[[]]): rowcount = 11250.0, cumulative cost = IgniteCost [rowCount=1650000.0, cpu=8550000.0, memory=1170000.0, io=0.0, network=0.0]
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 500000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=500000.0, memory=0.0, io=0.0, network=0.0]
      IgniteTrimExchange(distribution=[affinity[identity=hash, cacheId=1355342585][0]]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=575000.0, cpu=2300000.0, memory=0.0, io=0.0, network=0.0]
        IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 75000.0, cumulative cost = IgniteCost [rowCount=500000.0, cpu=2000000.0, memory=0.0, io=0.0, network=0.0]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also want to show that previously expected plan still possible but for lower table size, if we try to change
protected static final int DEFAULT_TBL_SIZE = 10;//500_000;
below plan will be raized:

IgniteProject(ID=[$2], NAME=[$3], PROJECTID=[$4], ID0=[$0], VER0=[$1]): rowcount = 2.25, cumulative cost = IgniteCost [rowCount=28.25, cpu=83.0, memory=23.4, io=0.0, network=45.0], id = 4147
  IgniteExchange(distribution=[single]): rowcount = 2.25, cumulative cost = IgniteCost [rowCount=26.0, cpu=80.75, memory=23.4, io=0.0, network=45.0], id = 4146
    IgniteHashJoin(condition=[=($2, $0)], joinType=[inner], variablesSet=[[]]): rowcount = 2.25, cumulative cost = IgniteCost [rowCount=23.75, cpu=78.5, memory=23.4, io=0.0, network=0.0], id = 4145
      IgniteTableScan(table=[[PUBLIC, PROJECT]], requiredColumns=[{0, 2}]): rowcount = 10.0, cumulative cost = IgniteCost [rowCount=10.0, cpu=10.0, memory=0.0, io=0.0, network=0.0], id = 1784
      IgniteTrimExchange(distribution=[affinity[identity=hash, cacheId=1355342585][0]]): rowcount = 1.5, cumulative cost = IgniteCost [rowCount=11.5, cpu=46.0, memory=0.0, io=0.0, network=0.0], id = 4144
        IgniteTableScan(table=[[PUBLIC, DEVELOPER]], filters=[=(+($t2, 1), ?0)]): rowcount = 1.5, cumulative cost = IgniteCost [rowCount=10.0, cpu=40.0, memory=0.0, io=0.0, network=0.0], id = 1793

Thus i think - if we want a fine tuning here - it need to be done under different issue.

@zstan
zstan force-pushed the ignite-28899 branch 3 times, most recently from 809cdcb to 063753c Compare August 20, 2026 11:08
@zstan

zstan commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Current cut shows:

TpchBenchmark, scale = 0.01, q = 5. (H2 just for comparison here, not related to an issue of course)

PR:
Benchmark             (engine)  (queryId)  (scale)  Mode  Cnt     Score     Error  Units
TpchBenchmark.cached   CALCITE          5     0.01  avgt    3    25.519 ±   4.548  ms/op
TpchBenchmark.cached        H2          5     0.01  avgt    3   193.700 ± 168.796  ms/op
TpchBenchmark.cold     CALCITE          5     0.01    ss       1093.271            ms/op
TpchBenchmark.cold          H2          5     0.01    ss        816.027            ms/op

master 
Benchmark             (engine)  (queryId)  (scale)  Mode  Cnt     Score    Error  Units
TpchBenchmark.cached   CALCITE          5     0.01  avgt    3   566.541 ± 49.195  ms/op
TpchBenchmark.cold     CALCITE          5     0.01    ss       1564.145           ms/op

@tkalkirill

Copy link
Copy Markdown
Contributor

Perhaps it would be more correct to compare before and after your changes?

.and(hasDistribution(broadcast()))
))
)
.and(nodeOrAnyChild(isInstanceOf(IgniteUncollect.class)))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

current plan:

IgniteHashJoin(condition=[=($0, $2)], joinType=[inner], variablesSet=[[]]): rowcount = 1.0, cumulative cost = IgniteCost [rowCount=1000007.0, cpu=1000016.0, memory=30.4, io=0.0, network=4000000.0]
  IgniteExchange(distribution=[single]): rowcount = 500000.0
    IgniteTableScan(table=[[PUBLIC, HASH_TBL]]): rowcount = 500000.0
  IgniteUncollect
    IgniteProject(EXPR$0=[ARRAY(1, 2, 3)])
      IgniteValues(tuples=[[{ 0 }]])

master branch plan:

IgniteProject(ID=[$1], NAME=[$2], EXPR$0=[$0]): rowcount = 4.0, cumulative cost = IgniteCost [rowCount=2300018.0, cpu=9200030.0, memory=0.0, io=0.0, network=48.0], id = 2654
  IgniteExchange(distribution=[single]): 
    IgniteCorrelatedNestedLoopJoin(condition=[=($1, $0)], joinType=[inner], variablesSet=[[$cor0]], variablesSet=[[0]], correlationVariables=[[$cor0]]): rowcount = 4.0
      IgniteTrimExchange(distribution=[hash[0]])
        IgniteUncollect
          IgniteProject(EXPR$0=[ARRAY(1, 2, 3)])
            IgniteValues(tuples=[[{ 0 }]])
      IgniteTableScan(table=[[PUBLIC, HASH_TBL]], filters=[=($t0, $cor0.EXPR$0)]): rowcount = 75000.0

@zstan

zstan commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

@tkalkirill @alex-plekhanov make a review plz ?

// See Apache Calcite ticket for more information, assertion is incorrect.
// FRAMEWORK_CONFIG initialize SqlToRelConverter class. Assertions should be disabled before class initialization.
SqlToRelConverter.class.getClassLoader().setClassAssertionStatus(SqlToRelConverter.class.getName(), false);
// TODO Workaround for https://issues.apache.org/jira/browse/CALCITE-5421 and https://issues.apache.org/jira/browse/CALCITE-7034

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can remove previous workaraund too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return res;
}

return Collections.singleton(rel.columnOriginsByRelLocalRef(iOutputColumn));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the difference with the current implementation?
Let's keep it as is, but replace requiredColumns().toArray()[colIdx] with requiredColumns().indexOf(colIdx) in columnOriginsByRelLocalRef for little optimization.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indexOf is not identical to toArray()[colIdx] but ok, I fix it

private IgniteSchema publicSchema;

/** {@inheritDoc} */
@Override public void setup() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add annotation BeforeClass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* Provides column origins for a subset.
*
* <p>Origins are resolved through the original (logical) expression of the set rather than through the current best
* plan: the best expression changes during optimization, and metadata depending on it makes row count estimates

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metadata depending on it makes row count estimates of the same expression unstable, which in turn leads to inconsistent costs and cyclic best plans in Volcano memo

Column origin (unlike row count estimation) should be calculated identically for all plans.
Here we do not calculate row count. At least comment is incorrect.

continue;

// Keys can relate to affinity not pk, just assumption here.
ImmutableIntList distKeys = table.distribution().getKeys();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Check for columns description with key() is too huge?
  2. For broadcast table and for tables with affinity column defined estimation is wrong. I thing it's a relatively big part of all cases.

postFiltrationAdjustment = 1.0;

if (rel.getJoinType() == JoinRelType.INNER || rel.getJoinType() == JoinRelType.SEMI) {
postFiltrationAdjustment = EQUI_COEFF;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we redefine post filtration here? If we have two tables and first is a subset of second, if no additional (non equi) conditions are defined, no additional EQUI_COEF is required, row count will be firstTableRowCoun * percentOfTotalRowsFromSecondTable (if data from second table are selected randomly). If there are any non-equi conditions, row count should be reduced by NON_EQUI_COEFF (in case of other join types too)

postFiltrationAdjustment = 1.0;

if (rel.getJoinType() == JoinRelType.INNER || rel.getJoinType() == JoinRelType.SEMI) {
postFiltrationAdjustment = EQUI_COEFF;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we redefine post filtration here? If we have two tables and first is a subset of second, if no additional (non equi) conditions are defined, no additional EQUI_COEF is required, row count will be firstTableRowCoun * percentOfTotalRowsFromSecondTable (if data from second table are selected randomly). If there are any non-equi conditions, row count should be reduced by NON_EQUI_COEFF

double baseRowCnt = 0.0;
Double percentageAdjustment = null;
if (joinCtx.joinType() == JoiningRelationType.PK_ON_PK) {
postFiltrationAdjustment = 1.0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redefining here is redundant, it always 1 except for INNER and SEMI join, but for INNER and SEMI join it redefined again below (but shouldn't be).

}

baseRowCnt = rightRowCnt + leftRowCnt;
percentageAdjustment = 1.0 - selectivity;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For full join, if one table is subset of another:
rowCnt = leftRowCnt + rightRowCnt - intersetion,
intersection = min(part of left rows in right table, part of right rows in left table) = min(leftRowCnt * rightPercentage, rightRowCnt * leftPercentage) or intersect = leftRowCnt * rightRowCnt / Math.max(leftOrigCnt, rightOrigCnt);

.and(input(0, nodeOrAnyChild(isTableScan("T1"))))
.and(input(1, nodeOrAnyChild(isTableScan("T3")
.and(ts -> "=($t0, $cor0.REF1)".equals(ts.condition().toString())))))
.and(input(1, nodeOrAnyChild(isTableScan("T2").and(ts -> ts.condition() == null))))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test fails now with:

Caused by: java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 2
	at com.google.common.collect.RegularImmutableList.get(RegularImmutableList.java:80)
	at org.apache.calcite.util.Util$TransformingList.get(Util.java:2924)
	at org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.extactFieldName(IgniteMdSelectivity.java:331)
	at org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.getColumnStatistics(IgniteMdSelectivity.java:312)
	at org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.getTablePredicateBasedSelectivity(IgniteMdSelectivity.java:237)
	at org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.getSelectivity(IgniteMdSelectivity.java:94)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants