Copilot commented on code in PR #13389:
URL: https://github.com/apache/ignite/pull/13389#discussion_r3645397998
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/MergeJoinNode.java:
##########
@@ -417,8 +395,12 @@ else if (cmp > 0) {
inLoop = false;
}
- if (requested > 0 && (leftFinished() || rightFinished(true)) &&
checkJoinFinished())
+ if (requested > 0 && (leftFinished() || rightFinished(true))) {
+ requested = 0;
+ downstream().end();
+
return;
+ }
Review Comment:
The new early-termination path ends the downstream immediately without the
prior `checkJoinFinished()` coordination/cleanup. In distributed mode this can
violate the upstream/downstream protocol (inputs may still deliver rows after
downstream has ended), and it also stops clearing buffered
state/materialization that was previously released on finishing. Consider
restoring a coordinated finish path that (1) ensures both inputs are fully
ended/cancelled when `distributed == true` before calling `downstream().end()`,
and (2) performs the same buffer/materialization cleanup previously done in
`checkJoinFinished()` to avoid holding large buffers until GC.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/hints/JoinTypeHintPlannerTest.java:
##########
@@ -415,8 +407,10 @@ private void doTestDisableJoinTypeWith(
nodeOrAnyChild(isInstanceOf(joinRel)).negate(), disabledRules);
// Hint with wrong table.
+ // Need to be removed after :
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-28911
assertPlan(String.format(sqlTpl, hintPref + "('UNEXISTING') */"),
schema,
- nodeOrAnyChild(isInstanceOf(joinRel)), disabledRules);
+ p -> true, disabledRules);
Review Comment:
Using `p -> true` makes this assertion vacuous (it will pass even if the
plan is completely unexpected), so the test no longer validates behavior and
may mask regressions unrelated to IGNITE-28911. If the intent is only to ensure
planning succeeds for wrong-table hints, consider asserting a minimal invariant
instead (e.g., that a node of `joinRel` still exists, or that the plan does not
contain the disabled join type), while keeping the TODO linked to IGNITE-28911.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdRowCount.java:
##########
@@ -72,48 +88,252 @@ public class IgniteMdRowCount extends RelMdRowCount {
mq.getRowCount(rel.getLeft()));
}
+ JoinInfo joinInfo = rel.analyzeCondition();
+
+ if (joinInfo.pairs().isEmpty()) {
+ // Fall-back to calcite's implementation.
+ return RelMdUtil.getJoinRowCount(mq, rel, rel.getCondition());
+ }
+
// Row count estimates of 0 will be rounded up to 1.
// So, use maxRowCount where the product is very small.
- final Double left = mq.getRowCount(rel.getLeft());
- final Double right = mq.getRowCount(rel.getRight());
+ final Double leftRowCnt = mq.getRowCount(rel.getLeft());
+ final Double rightRowCnt = mq.getRowCount(rel.getRight());
- if (left == null || right == null)
+ if (leftRowCnt == null || rightRowCnt == null)
return null;
- if (left <= 1D || right <= 1D) {
+ if (leftRowCnt <= 1D || rightRowCnt <= 1D) {
Double max = mq.getMaxRowCount(rel);
if (max != null && max <= 1D)
return max;
}
- JoinInfo joinInfo = rel.analyzeCondition();
+ Map<Integer, KeyColumnOrigin> columnsFromLeft = resolveOrigins(mq,
rel.getLeft(), joinInfo.leftKeys);
+ Map<Integer, KeyColumnOrigin> columnsFromRight = resolveOrigins(mq,
rel.getRight(), joinInfo.rightKeys);
+
+ if (columnsFromLeft.isEmpty() || columnsFromRight.isEmpty())
+ return crudeEstimation(mq, joinInfo, rel, leftRowCnt, rightRowCnt);
+
+ Map<TablesPair, JoinContext> joinCtxts = new HashMap<>();
+ for (IntPair joinKeys : joinInfo.pairs()) {
+ KeyColumnOrigin leftKey = columnsFromLeft.get(joinKeys.source);
+ KeyColumnOrigin rightKey = columnsFromRight.get(joinKeys.target);
+
+ if (leftKey == null || rightKey == null) {
+ continue;
+ }
+
+ joinCtxts.computeIfAbsent(
+ new TablesPair(
+ leftKey.origin.getOriginTable(),
+ rightKey.origin.getOriginTable()
+ ),
+ key -> {
+ IgniteTable leftTable = key.left.unwrap(IgniteTable.class);
+ IgniteTable rightTable =
key.right.unwrap(IgniteTable.class);
+
+ assert leftTable != null && rightTable != null;
+
+ int leftPkSize = leftTable.distribution().getKeys().size();
+ int rightPkSize =
rightTable.distribution().getKeys().size();
+
+ return new JoinContext(leftPkSize, rightPkSize);
+ }
+ ).countKeys(leftKey, rightKey);
+ }
+
+ if (joinCtxts.isEmpty()) {
+ // Fall-back to calcite's implementation.
+ return RelMdUtil.getJoinRowCount(mq, rel, rel.getCondition());
+ }
+
+ Iterator<JoinContext> it = joinCtxts.values().iterator();
+ JoinContext joinCtx = it.next();
+ while (it.hasNext()) {
+ JoinContext nextCtx = it.next();
+ if (nextCtx.joinType().strength > joinCtx.joinType().strength) {
+ joinCtx = nextCtx;
+ }
+
+ if (joinCtx.joinType().strength ==
JoiningRelationType.PK_ON_PK.strength) {
+ break;
+ }
+ }
+
+ if (joinCtx.joinType() == JoiningRelationType.UNKNOWN) {
+ // Fall-back to calcite's implementation.
+ return RelMdUtil.getJoinRowCount(mq, rel, rel.getCondition());
+ }
+
+ double postFiltrationAdjustment = 1.0;
+
+ switch (rel.getJoinType()) {
+ case INNER:
+ case SEMI:
+ // Extra join keys as well as non-equi conditions serves as
post-filtration,
+ // therefore we need to adjust final result with a little
factor.
+ if (joinCtxts.size() != 1 || !joinInfo.isEqui())
+ postFiltrationAdjustment = NON_EQUI_COEFF;
+
+ break;
+ default:
+ break;
+ }
+
+ double baseRowCnt = 0.0;
+ Double percentageAdjustment = null;
+ if (joinCtx.joinType() == JoiningRelationType.PK_ON_PK) {
+ postFiltrationAdjustment = EQUI_COEFF;
+
+ if (rel.getJoinType() == JoinRelType.INNER || rel.getJoinType() ==
JoinRelType.SEMI) {
+ // Assume we have two fact tables SALES and RETURNS sharing
the same primary key. Every item
+ // can be sold, but only items which were sold can be returned
back, therefore
+ // size(SALES) > size(RETURNS). When joining SALES on RETURNS
by primary key, the estimated
+ // result size will be the same as the size of the smallest
table (RETURNS in this case),
+ // adjusted by the percentage of rows of the biggest table
(SALES in this case; percentage
+ // adjustment is required to account for predicates pushed
down to the table, e.g. we are
+ // interested in returns of items with certain category)
+ if (leftRowCnt > rightRowCnt) {
+ baseRowCnt = rightRowCnt;
+ percentageAdjustment =
mq.getPercentageOriginalRows(rel.getLeft());
+ }
+ else {
+ baseRowCnt = leftRowCnt;
+ percentageAdjustment =
mq.getPercentageOriginalRows(rel.getRight());
+ }
+ }
+ else if (rel.getJoinType() == JoinRelType.LEFT) {
+ baseRowCnt = leftRowCnt;
+ }
+ else if (rel.getJoinType() == JoinRelType.RIGHT) {
+ baseRowCnt = rightRowCnt;
+ }
+ else if (rel.getJoinType() == JoinRelType.FULL) {
+ Double selectivity = mq.getSelectivity(rel,
rel.getCondition());
+
+ // Fall-back to calcite's implementation.
+ if (selectivity == null) {
+ return RelMdUtil.getJoinRowCount(mq, rel,
rel.getCondition());
+ }
+
+ baseRowCnt = rightRowCnt + leftRowCnt;
+ percentageAdjustment = 1.0 - selectivity;
+ }
+ }
+ else if (joinCtx.joinType() == JoiningRelationType.FK_ON_PK) {
+ // For foreign key joins the base table is the one which is joined
by non-primary key columns.
+ if (rel.getJoinType() == JoinRelType.INNER || rel.getJoinType() ==
JoinRelType.SEMI) {
+ baseRowCnt = leftRowCnt;
+ percentageAdjustment =
mq.getPercentageOriginalRows(rel.getRight());
+ }
+ else if (rel.getJoinType() == JoinRelType.LEFT ||
rel.getJoinType() == JoinRelType.RIGHT) {
+ baseRowCnt = leftRowCnt;
+ }
+ else if (rel.getJoinType() == JoinRelType.FULL) {
+ Double selectivity = mq.getSelectivity(rel,
rel.getCondition());
+
+ // Fall-back to calcite's implementation.
+ if (selectivity == null) {
+ return RelMdUtil.getJoinRowCount(mq, rel,
rel.getCondition());
+ }
+
+ baseRowCnt = rightRowCnt + leftRowCnt;
+ percentageAdjustment = 1.0 - selectivity;
+ }
+ }
+ else { // PK_ON_FK
+ if (rel.getJoinType() == JoinRelType.INNER || rel.getJoinType() ==
JoinRelType.SEMI) {
+ baseRowCnt = rightRowCnt;
+ percentageAdjustment =
mq.getPercentageOriginalRows(rel.getLeft());
+ }
+ else if (rel.getJoinType() == JoinRelType.RIGHT ||
rel.getJoinType() == JoinRelType.LEFT) {
+ baseRowCnt = rightRowCnt;
+ }
+ else if (rel.getJoinType() == JoinRelType.FULL) {
+ Double selectivity = mq.getSelectivity(rel,
rel.getCondition());
+
+ // Fall-back to calcite's implementation.
+ if (selectivity == null) {
+ return RelMdUtil.getJoinRowCount(mq, rel,
rel.getCondition());
+ }
+
+ baseRowCnt = rightRowCnt + leftRowCnt;
+ percentageAdjustment = 1.0 - selectivity;
+ }
+ }
+
+ if (percentageAdjustment == null) {
+ // No info, let's be conservative.
+ percentageAdjustment = 1.0;
+ }
+
+ return baseRowCnt * percentageAdjustment * postFiltrationAdjustment;
+ }
+
+ /** */
+ private static Map<Integer, KeyColumnOrigin>
resolveOrigins(RelMetadataQuery mq, RelNode joinShoulder, ImmutableIntList
keys) {
+ GridLeanMap<Integer, KeyColumnOrigin> origins = new GridLeanMap<>();
+
+ for (int i : keys) {
+ if (origins.containsKey(i)) {
+ continue;
+ }
+
+ RelColumnOrigin origin = mq.getColumnOrigin(joinShoulder, i);
+ if (origin == null) {
+ continue;
+ }
+
+ IgniteTable table =
origin.getOriginTable().unwrap(IgniteTable.class);
+ if (table == null || !table.distribution().function().affinity())
+ continue;
+
+ // Keys can relate to affinity not pk, just assumption here.
+ ImmutableIntList distKeys = table.distribution().getKeys();
+
+ int idx = distKeys.indexOf(origin.getOriginColumnOrdinal());
+
+ origins.put(i, new KeyColumnOrigin(origin, idx));
+ }
+
+ return origins;
+ }
+
+ /**
+ * @param origin
+ * @param positionInKey
+ */
+ private record KeyColumnOrigin(RelColumnOrigin origin, int positionInKey)
{ }
+ /** This part of estimation is applicable for distributions different from
hash, i.e. broadcast, single. */
+ private static double crudeEstimation(RelMetadataQuery mq, JoinInfo
joinInfo, Join rel, Double leftRowCnt, Double rightRowCnt) {
ImmutableIntList leftKeys = joinInfo.leftKeys;
ImmutableIntList rightKeys = joinInfo.rightKeys;
double selectivity = mq.getSelectivity(rel, rel.getCondition());
if (F.isEmpty(leftKeys) || F.isEmpty(rightKeys))
- return left * right * selectivity;
+ return leftRowCnt * rightRowCnt * selectivity;
Review Comment:
`mq.getSelectivity(...)` returns `Double` (nullable), but this code unboxes
it into a primitive `double`. If selectivity metadata is unavailable for some
relation/condition, this will throw an NPE. Use `Double selectivity = ...` and
fall back to Calcite's `RelMdUtil.getJoinRowCount(...)` (or a conservative
default) when it is `null`, similar to other branches in `joinRowCount()`.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/JoinRowCountEstimationTest.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.planner;
+
+import java.math.BigDecimal;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.sql.SqlExplainFormat;
+import org.apache.calcite.sql.SqlExplainLevel;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteSchema;
+import
org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
+import org.apache.ignite.internal.util.typedef.internal.CU;
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.CoreMatchers;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.junit.Test;
+
+import static
org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.COMPARISON_SELECTIVITY;
+import static
org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.EQUALS_SELECTIVITY;
+import static
org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMdSelectivity.IS_NOT_NULL_SELECTIVITY;
+
+/**
+ * Tests to check row count estimation for join relation.
+ */
+public class JoinRowCountEstimationTest extends AbstractPlannerTest {
+ /** */
+ private static final int CATALOG_SALES_SIZE = 1_441_548;
+
+ /** */
+ private static final int CATALOG_RETURNS_SIZE = 144_067;
+
+ /** */
+ private static final int DATE_DIM_SIZE = 73_049;
+
+ /** */
+ private static final String SELECT = "SELECT /*+ DISABLE_RULE(" +
+ "'JoinCommuteRule', " +
+ "'MergeJoinConverter', " +
+ "'NestedLoopJoinConverter', " +
+ "'CorrelateToNestedLoopRule'" +
+ ") */ * ";
+
+ /** */
+ private IgniteSchema publicSchema;
+
+ /** {@inheritDoc} */
+ @Override public void setup() {
+ super.setup();
+
+ TestTable tbl1 = createTable(
+ "CATALOG_SALES",
+ CATALOG_SALES_SIZE,
+ IgniteDistributions.affinity(ImmutableIntList.of(1, 2),
CU.cacheId("default"), 0),
+ "ID", Integer.class,
+ "CS_ITEM_SK", Integer.class,
+ "CS_ORDER_NUMBER", Integer.class,
+ "CS_PROMO_SK", Integer.class
+ );
+
+ TestTable tbl2 = createTable(
+ "CATALOG_RETURNS",
+ CATALOG_RETURNS_SIZE,
+ IgniteDistributions.affinity(ImmutableIntList.of(1, 2),
CU.cacheId("default"), 0),
+ "ID", Integer.class,
+ "CR_ITEM_SK", Integer.class,
+ "CR_ORDER_NUMBER", Integer.class,
+ "CR_RETURNED_DATE_SK", Integer.class,
+ "CR_RETURN_QUANTITY", Integer.class
+ );
+
+ TestTable tbl3 = createTable(
+ "DATE_DIM",
+ DATE_DIM_SIZE,
+ IgniteDistributions.affinity(ImmutableIntList.of(0),
CU.cacheId("default"), 0),
+ "D_DATE_SK", Integer.class,
+ "D_MOY", Integer.class
+ );
+
+ publicSchema = createSchema(tbl1, tbl2, tbl3);
+ }
+
+ /** */
+ @Test
+ public void joinSameTableSimpleAffMergeJoin() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_sales"
+ + " ,catalog_returns"
+ + " WHERE cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_RETURNS_SIZE)));
+
+ // It needs to return like: CATALOG_RETURNS_SIZE *
IS_NOT_NULL_SELECTIVITY, but it will be done at future
+ // Need to adopt: IGNITE-23969
+ assertPlan(SELECT
+ + " FROM catalog_sales"
+ + " ,catalog_returns"
+ + " WHERE cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cs_promo_sk IS NOT NULL",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_RETURNS_SIZE)));
+ }
+
+ /** */
+ @Test
+ public void joinByPrimaryKeysLeft() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_sales LEFT JOIN catalog_returns ON"
+ + " cs_item_sk = cr_item_sk AND cs_order_number =
cr_order_number",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_SALES_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM catalog_sales LEFT JOIN catalog_returns"
+ + " ON cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cs_promo_sk IS NOT NULL",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_SALES_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM catalog_sales LEFT JOIN catalog_returns"
+ + " ON cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cr_order_number > 1",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_SALES_SIZE)));
+ }
+
+ /** */
+ @Test
+ public void joinByPrimaryKeysRight() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_returns RIGHT JOIN catalog_sales ON"
+ + " cs_item_sk = cr_item_sk AND cs_order_number =
cr_order_number",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_SALES_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM catalog_returns RIGHT JOIN catalog_sales"
+ + " ON cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cs_promo_sk IS NOT NULL",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_SALES_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM catalog_returns RIGHT JOIN catalog_sales"
+ + " ON cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cr_order_number > 1",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_SALES_SIZE)));
+ }
+
+ /** */
+ @Test
+ public void joinByPrimaryKeysFull() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_returns FULL OUTER JOIN catalog_sales ON"
+ + " cs_item_sk = cr_item_sk AND cs_order_number =
cr_order_number",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual((int)((CATALOG_SALES_SIZE + CATALOG_RETURNS_SIZE)
+ * (1.0 - (EQUALS_SELECTIVITY * EQUALS_SELECTIVITY))))));
+
+ assertPlan(SELECT
+ + " FROM catalog_returns FULL OUTER JOIN catalog_sales"
+ + " ON cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cs_promo_sk IS NOT NULL",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual((int)((CATALOG_SALES_SIZE + CATALOG_RETURNS_SIZE)
+ * (1.0 - (EQUALS_SELECTIVITY * EQUALS_SELECTIVITY *
IS_NOT_NULL_SELECTIVITY))))));
+
+ assertPlan(SELECT
+ + " FROM catalog_returns FULL OUTER JOIN catalog_sales"
+ + " ON cs_item_sk = cr_item_sk"
+ + " AND cs_order_number = cr_order_number"
+ + " AND cr_order_number > 1",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual((int)((CATALOG_SALES_SIZE + CATALOG_RETURNS_SIZE)
+ * (1.0 - (EQUALS_SELECTIVITY * EQUALS_SELECTIVITY *
COMPARISON_SELECTIVITY))))));
+ }
+
+ /** */
+ @Test
+ public void joinByForeignKey() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_returns"
+ + " ,date_dim"
+ + " WHERE cr_returned_date_sk = d_date_sk",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual(CATALOG_RETURNS_SIZE)));
+
+ // It needs to return like: CATALOG_RETURNS_SIZE *
COMPARISON_SELECTIVITY, but it will be done at future
+ // Need to adopt: IGNITE-23969
+ assertPlan(SELECT
+ + " FROM date_dim"
+ + " ,catalog_returns"
+ + " WHERE cr_returned_date_sk = d_date_sk"
+ + " AND d_moy > 6",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+ }
+
+ /** */
+ @Test
+ public void joinByForeignKeyLeft() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_returns LEFT JOIN date_dim ON"
+ + " cr_returned_date_sk = d_date_sk",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM date_dim LEFT JOIN catalog_returns ON"
+ + " cr_returned_date_sk = d_date_sk",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM catalog_returns LEFT JOIN date_dim ON"
+ + " cr_returned_date_sk = d_date_sk"
+ + " AND cr_return_quantity > 6",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+ }
+
+ /** */
+ @Test
+ public void joinByForeignKeyRight() throws Exception {
+ assertPlan(SELECT
+ + " FROM catalog_returns RIGHT JOIN date_dim ON"
+ + " cr_returned_date_sk = d_date_sk",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM date_dim RIGHT JOIN catalog_returns ON"
+ + " cr_returned_date_sk = d_date_sk",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+
+ assertPlan(SELECT
+ + " FROM date_dim RIGHT JOIN catalog_returns ON"
+ + " cr_returned_date_sk = d_date_sk"
+ + " AND cr_return_quantity > 6",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
CoreMatchers.is(CATALOG_RETURNS_SIZE)));
+ }
+
+ /** */
+ @Test
+ public void joinByForeignKeyFull() throws Exception {
+ assertPlan(SELECT
+ + " FROM date_dim FULL OUTER JOIN catalog_returns ON"
+ + " cr_returned_date_sk = d_date_sk",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual((int)((DATE_DIM_SIZE + CATALOG_RETURNS_SIZE)
+ * (1.0 - (EQUALS_SELECTIVITY))))));
+
+ assertPlan(SELECT
+ + " FROM date_dim FULL OUTER JOIN catalog_returns ON"
+ + " cr_returned_date_sk = d_date_sk"
+ + " AND cr_return_quantity > 6",
+ publicSchema,
+ nodeRowCount("IgniteHashJoin",
approximatelyEqual((int)((DATE_DIM_SIZE + CATALOG_RETURNS_SIZE)
+ * (1.0 - (EQUALS_SELECTIVITY * COMPARISON_SELECTIVITY))))));
+ }
+
+ /**
+ * Matcher which ensures that node matching the pattern has row count
matching provided matcher.
+ *
+ * @param nodePattern A pattern describing a node of interest.
+ * @param rowCountMatcher Matcher for the estimated row count.
+ * @return Matcher.
+ */
+ static <T extends RelNode> Predicate<RelNode> nodeRowCount(String
nodePattern, Matcher<Integer> rowCountMatcher) {
+ Pattern pattern = Pattern.compile(".*" + nodePattern
+ + "\\(.*?rowcount = (?<rowcount>\\d+).*");
+
+ return node -> {
+ String plan = RelOptUtil.dumpPlan("", node, SqlExplainFormat.TEXT,
SqlExplainLevel.ALL_ATTRIBUTES);
+
+ String sanitized = plan.replace("\n", "");
+ java.util.regex.Matcher matcher = pattern.matcher(sanitized);
Review Comment:
The rowcount regex only matches integers (`\\d+`), but Calcite plan dumps
commonly emit row counts as decimals (e.g. `123.0`) or exponent notation (e.g.
`1.23E6`). This can make the predicate return `false` even when the row count
is correct, causing flaky failures depending on formatter/version. Consider
widening the capture to support decimals/exponents and parsing as `double`
(then comparing via the matcher), and also sanitize `\\r` to be robust across
platforms.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/Commons.java:
##########
@@ -452,6 +452,23 @@ public static Mappings.TargetMapping
inverseMapping(ImmutableBitSet bitSet, int
return mapping;
}
+ /**
+ * Creates mapping from given projection.
+ *
+ * <p>Projection is a list of integers representing an index of element
from source
+ * at desired position.
+ *
+ * @param sourceSize Size of the source.
+ * @param bitSet Desired projection.
+ * @return Mapping for given projection.
+ */
+ public static Mappings.TargetMapping projectedMapping(ImmutableBitSet
bitSet, int sourceSize) {
Review Comment:
The Javadoc is inconsistent with the method signature and terminology: (1)
parameter order in the signature is `(bitSet, sourceSize)` but the `@param`
entries describe `sourceSize` first, and (2) it describes the projection as a
'list of integers' while the API actually takes an `ImmutableBitSet` (ordered
set) which implies projection positions by ascending bit order. Updating the
Javadoc to match the actual inputs/semantics will make the mapping direction
and expected ordering clearer.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LitmusCheckIntegrationTest.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.internal.processors.query.calcite.integration;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configurator;
+import org.junit.Test;
+
+import static org.apache.logging.log4j.Level.DEBUG;
+
+/** Calcite litmus related tests. */
+public class LitmusCheckIntegrationTest extends AbstractBasicIntegrationTest {
+ /** {@inheritDoc} */
+ @Override protected void beforeTestsStarted() throws Exception {
+ super.beforeTestsStarted();
+
+ setCalciteLoggerDebugLevel();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected int nodeCount() {
+ return 1;
+ }
+
+ /** Check no calcite litmus exception is raised. */
+ @Test
+ public void testLitmusLowerCost() {
+ sql("create table t11 (c1 int, c2 int, c3 int)");
+ sql("create table t22 (c1 int, c2 int, c3 int)");
+ sql("create index t11_idx on t11 (c3, c2, c1)");
+ sql("create index t22_idx on t22 (c3, c2, c1)");
+
+ assertQuery("SELECT distinct p.c1 FROM t11 cd left join " +
+ "t22 p ON p.c2 = cd.c2 WHERE cd.c2 = 1;").resultSize(0).check();
+ }
+
+ /**
+ * Sets the log level for logger ({@link #log}) to {@link Level#DEBUG}.
The log level will be resetted to
+ * default in {@link #afterTest()}.
Review Comment:
Fix spelling: 'resetted' should be 'reset'.
--
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]