alex-plekhanov commented on code in PR #11957:
URL: https://github.com/apache/ignite/pull/11957#discussion_r2017249623
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/JoinCommutePlannerTest.java:
##########
@@ -76,6 +82,100 @@ public class JoinCommutePlannerTest extends
AbstractPlannerTest {
);
}
+ /** Ensures that join commute rules are disabled if joins count is too
high. Checks the tables order in the query. */
+ @Test
+ public void testCommuteDisabledForManyJoins() throws Exception {
Review Comment:
Test will fail after https://issues.apache.org/jira/browse/IGNITE-24624
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/JoinCommutePlannerTest.java:
##########
@@ -76,6 +82,100 @@ public class JoinCommutePlannerTest extends
AbstractPlannerTest {
);
}
+ /** Ensures that join commute rules are disabled if joins count is too
high. Checks the tables order in the query. */
+ @Test
+ public void testCommuteDisabledForManyJoins() throws Exception {
+ IgniteTypeFactory f = new IgniteTypeFactory(IgniteTypeSystem.INSTANCE);
+
+ int maxJoinsToOptimize =
Math.max(PlannerHelper.MAX_JOINS_TO_COMMUTE_INPUTS,
PlannerHelper.MAX_JOINS_TO_COMMUTE);
+
+ int tablesCnt = maxJoinsToOptimize + 3;
+
+ try {
+ for (int i = 0; i < tablesCnt; ++i) {
+ publicSchema.addTable(
+ "TBL" + i,
Review Comment:
Something like:
```
createTable("TBL" + i, (int)Math.pow(100, (1 + (i % 3))),
IgniteDistributions.affinity(0, "TEST_CACHE", "hash"),
"ID", Integer.class)
```
Is more convinient in my opinion.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/JoinCommutePlannerTest.java:
##########
@@ -76,6 +82,100 @@ public class JoinCommutePlannerTest extends
AbstractPlannerTest {
);
}
+ /** Ensures that join commute rules are disabled if joins count is too
high. Checks the tables order in the query. */
+ @Test
+ public void testCommuteDisabledForManyJoins() throws Exception {
+ IgniteTypeFactory f = new IgniteTypeFactory(IgniteTypeSystem.INSTANCE);
+
+ int maxJoinsToOptimize =
Math.max(PlannerHelper.MAX_JOINS_TO_COMMUTE_INPUTS,
PlannerHelper.MAX_JOINS_TO_COMMUTE);
+
+ int tablesCnt = maxJoinsToOptimize + 3;
+
+ try {
+ for (int i = 0; i < tablesCnt; ++i) {
+ publicSchema.addTable(
+ "TBL" + i,
+ new TestTable(
+ new RelDataTypeFactory.Builder(f)
+ .add("ID", f.createJavaType(Integer.class))
+ .build(), Math.pow(100, (1 + (i % 3)))) {
+
+ @Override public IgniteDistribution distribution() {
+ return IgniteDistributions.affinity(0,
"TEST_CACHE", "hash");
+ }
+ }
+ );
+ }
+
+ doTestCommuteDisabledForManyJoins(maxJoinsToOptimize + 1, false,
"TBL");
+
+ doTestCommuteDisabledForManyJoins(maxJoinsToOptimize + 2, true,
"TBL");
+ }
+ finally {
+ for (int i = 0; i < tablesCnt; ++i)
+ publicSchema.removeTable("TBL" + i);
+ }
+ }
+
+ /** */
+ private void doTestCommuteDisabledForManyJoins(int joinsCnt, boolean
withCorrelated, String tblNamePrefix) throws Exception {
+ StringBuilder select = new StringBuilder();
+ StringBuilder joins = new StringBuilder();
+
+ for (int i = 0; i < joinsCnt + 1; ++i) {
+ if (withCorrelated && i == joinsCnt) {
+ String alias = "t" + i;
+
+ select.append("\n(SELECT MAX(").append(alias).append(".id)
FROM TBL").append(i).append(" ").append(alias)
+ .append(" WHERE
").append(alias).append(".id=").append("t0").append(".id + 1) as corr")
+ .append(i).append(", ");
+
+ continue;
+ }
+
+ select.append("\nt").append(i).append(".id, ");
+
+ if (i == 0)
+ joins.append("TBL0 t0");
+ else {
+ joins.append(" JOIN TBL").append(i).append("
t").append(i).append(" ON t").append(i - 1).append(".id=t")
+ .append(i).append(".id");
+ }
+ }
+
+ String sql = "SELECT " + select.substring(0, select.length() - 2) +
"\nfrom " + joins;
+
+ long timing = System.nanoTime();
+
+ IgniteRel plan = physicalPlan(sql, publicSchema);
+
+ timing = U.nanosToMillis(System.nanoTime() - timing);
+
+ if (log.isInfoEnabled())
+ log.info("The planning took " + timing + "ms.");
+
+ RelShuttle shuttle = new RelHomogeneousShuttle() {
+ private int prev = Integer.MIN_VALUE;
+
+ @Override public RelNode visit(TableScan scan) {
+ String tblName = Util.last(scan.getTable().getQualifiedName());
+
+ assert tblName.startsWith(tblNamePrefix);
+
+ int tblNum = Integer.parseInt(tblName.replace(tblNamePrefix,
""));
+
+ if (prev == Integer.MIN_VALUE)
+ prev = tblNum;
Review Comment:
`prev` variable never updated. Looks like you mean here something like:
```
if (tblNum < prev)
throw new IllegalStateException("Wrong table number
found. Current: " + tblNum + ", previous: " + prev + '.');
prev = tblNum;
```
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/JoinCommutePlannerTest.java:
##########
@@ -76,6 +82,100 @@ public class JoinCommutePlannerTest extends
AbstractPlannerTest {
);
}
+ /** Ensures that join commute rules are disabled if joins count is too
high. Checks the tables order in the query. */
+ @Test
+ public void testCommuteDisabledForManyJoins() throws Exception {
+ IgniteTypeFactory f = new IgniteTypeFactory(IgniteTypeSystem.INSTANCE);
+
+ int maxJoinsToOptimize =
Math.max(PlannerHelper.MAX_JOINS_TO_COMMUTE_INPUTS,
PlannerHelper.MAX_JOINS_TO_COMMUTE);
+
+ int tablesCnt = maxJoinsToOptimize + 3;
+
+ try {
+ for (int i = 0; i < tablesCnt; ++i) {
+ publicSchema.addTable(
+ "TBL" + i,
+ new TestTable(
+ new RelDataTypeFactory.Builder(f)
+ .add("ID", f.createJavaType(Integer.class))
+ .build(), Math.pow(100, (1 + (i % 3)))) {
+
+ @Override public IgniteDistribution distribution() {
+ return IgniteDistributions.affinity(0,
"TEST_CACHE", "hash");
+ }
+ }
+ );
+ }
+
+ doTestCommuteDisabledForManyJoins(maxJoinsToOptimize + 1, false,
"TBL");
+
+ doTestCommuteDisabledForManyJoins(maxJoinsToOptimize + 2, true,
"TBL");
Review Comment:
`maxJoinsToOptimize + 2` looks confusing. Lets use the same joinsCnt and add
subquery to "select" variable when withCorrelated is true (outside of for-loop).
Also, we do not check correlate when joinsCnt = maxJoinsToOptimize (if we
want to check that correlate is not counted as join), so check with
maxJoinsToOptimize + 1 joins looks meaningless, since we have checked query
with the same count of joins before.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
