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]

Reply via email to