You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/02/02 14:40:29 UTC
[GitHub] [ignite] timoninmaxim opened a new pull request #8744: IGNITE-13019 Fail on non-colocated join
timoninmaxim opened a new pull request #8744:
URL: https://github.com/apache/ignite/pull/8744
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] slukyano commented on a change in pull request #8744: [WIP] IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
slukyano commented on a change in pull request #8744:
URL: https://github.com/apache/ignite/pull/8744#discussion_r617533937
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
##########
@@ -1269,6 +1279,10 @@ private void splitSelect(GridSqlAst parent, int childIdx) throws IgniteCheckedEx
map.hasSubQueries(SplitterUtils.hasSubQueries(mapQry));
map.hasOuterJoinReplicatedPartitioned(SplitterUtils.hasOuterJoinReplicatedPartitioned(mapQry.from()));
+ // TODO: join in subqueries? Subqueries through this process too.
Review comment:
Need to get rid of the TODO before merging of course
##########
File path: modules/indexing/src/test/java/org/apache/ignite/sqltests/CheckWarnJoinPartitionedTables.java
##########
@@ -0,0 +1,408 @@
+/*
+ * 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.sqltests;
+
+import java.util.List;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.cache.query.FieldsQueryCursor;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** Check that illegal joins of partitioned tables are warned. */
+public class CheckWarnJoinPartitionedTables extends GridCommonAbstractTest {
+ /** */
+ private final ListeningTestLogger testLog = new ListeningTestLogger(log);
+
+ /** */
+ private Ignite crd;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.setGridLogger(testLog);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ crd = startGrid();
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR);"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
+ checkSameTableWithJoinType("JOIN");
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryAffinityKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR) with \"AFFINITY_KEY=ID\";"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
+ checkSameTableWithJoinType("JOIN");
+ }
+
+ /** */
+ private void checkSameTableWithJoinType(String joinType) {
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a2.ID = a1.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 where a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 where a1.ID = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND a1.TITLE = a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND a1.TITLE != a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND (a1.TITLE = a2.TITLE OR a1.ID = a2.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on (a1.TITLE = a2.TITLE OR a1.ID = a2.ID) AND a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2) a3 on a1.ID = a3.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 where a1.ID in (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID) t" +
+ " on a1.ID = t.ID;");
+
+ // Ignite doesn't support this.
+ if (!joinType.contains("RIGHT"))
Review comment:
Ignite doesn't support two RIGHT JOINs in the same statement or what?
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/SplitterUtils.java
##########
@@ -286,6 +290,255 @@ public static boolean hasPartitionedTables(GridSqlAst ast) {
return false;
}
+ /**
+ * Traverse AST while join operation isn't found. Check it if found.
+ */
+ public static void lookForPartitionedJoin(GridSqlAst ast, GridSqlAst upWhere, IgniteLogger log) {
Review comment:
I'm really struggling to get a sense of completeness from this code. The test cases list seems extensive, so this code probably works, but I'm not sure I understand why...
I think structuring might solve it.
How about the following structure?
1. We always start from a SELECT - so let's process it first
2. In a SELECT, we want to check the subselects in the columns (yes!), the FROM, and the subselects in the where
3. In a FROM, we may see either a single table (not interesting), a subselect (go to step 1), or a JOIN
... and so on. I don't insist on a particular structure but I think we need to be able to understand from the code that nothing is missing. BTW splitting into several methods (lookForPartitionJoinInSelect/From/Where/etc) could help.
##########
File path: modules/indexing/src/test/java/org/apache/ignite/sqltests/CheckWarnJoinPartitionedTables.java
##########
@@ -0,0 +1,408 @@
+/*
+ * 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.sqltests;
+
+import java.util.List;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.cache.query.FieldsQueryCursor;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** Check that illegal joins of partitioned tables are warned. */
+public class CheckWarnJoinPartitionedTables extends GridCommonAbstractTest {
+ /** */
+ private final ListeningTestLogger testLog = new ListeningTestLogger(log);
+
+ /** */
+ private Ignite crd;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.setGridLogger(testLog);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ crd = startGrid();
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR);"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
+ checkSameTableWithJoinType("JOIN");
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryAffinityKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR) with \"AFFINITY_KEY=ID\";"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
+ checkSameTableWithJoinType("JOIN");
+ }
+
+ /** */
+ private void checkSameTableWithJoinType(String joinType) {
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a2.ID = a1.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 where a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 where a1.ID = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND a1.TITLE = a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND a1.TITLE != a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND (a1.TITLE = a2.TITLE OR a1.ID = a2.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on (a1.TITLE = a2.TITLE OR a1.ID = a2.ID) AND a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2) a3 on a1.ID = a3.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 where a1.ID in (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID) t" +
+ " on a1.ID = t.ID;");
+
+ // Ignite doesn't support this.
+ if (!joinType.contains("RIGHT"))
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID " + joinType + " A a3 on a1.ID = a3.ID;");
+
+ // Some simple illegal joins.
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on 1 = 1;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID > a2.ID;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID != a2.ID;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.TITLE;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2.TITLE;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.TITLE = a2.TITLE;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID OR a1.TITLE = a2.TITLE;");
+
+ // Actually this is correct query. But this AST is too complex to analyze, and also it could be simplified.
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID OR (a1.ID = a2.ID AND a1.TITLE = a2.TITLE);");
+
+ if (!joinType.contains("RIGHT"))
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.TITLE " + joinType + " A a3 on a1.ID = a3.ID;");
+
+ // This query is invalid. But we don't have info about partitioning in nested joins.
+ if (!joinType.contains("RIGHT")) {
+ // For inner join we can check it because join conditions are parsed to WHERE clause.
+ boolean shouldWarn = !joinType.contains("LEFT");
+
+ checkLogListener(shouldWarn,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID " + joinType + " A a3 on a1.ID = a3.TITLE;");
+ }
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 where a1.ID in (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.TITLE);");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 where a1.ID in (select a2.ID from A a2 " + joinType + " A a3 on a2.ID != a3.ID);");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.TITLE) t" +
+ " on a1.ID = t.ID;");
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithComplexPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT, TITLE VARCHAR, PRICE INT, COMMENT VARCHAR, PRIMARY KEY (ID, TITLE, PRICE));"));
+
+ checkSameTableWithComplexPrimaryKeyWithJoinType("LEFT JOIN");
+ checkSameTableWithComplexPrimaryKeyWithJoinType("RIGHT JOIN");
+ checkSameTableWithComplexPrimaryKeyWithJoinType("INNER JOIN");
+ checkSameTableWithComplexPrimaryKeyWithJoinType("JOIN");
+ }
+
+ /** */
+ private void checkSameTableWithComplexPrimaryKeyWithJoinType(String joinType) {
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a2._KEY = a1._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID and a1.TITLE = a2.TITLE and a1.PRICE = a2.PRICE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID and a1.TITLE = a2.TITLE and a1.PRICE = a2.PRICE and a1.COMMENT = a2.COMMENT;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID and a1.TITLE = a2.TITLE and a1.PRICE = a2.PRICE and a1.COMMENT != a2.COMMENT;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.PRICE = a2.PRICE and a1.TITLE = a2.TITLE and a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID where a1.PRICE = a2.PRICE and a1.TITLE = a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID where a1.PRICE = a2.PRICE and a1.TITLE = a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 where a1.ID in (" +
+ " select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID and a2.TITLE = a3.TITLE and a2.PRICE = a3.PRICE);");
+
+ if (!joinType.contains("RIGHT"))
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY " + joinType + " A a3 on a1._KEY = a3._KEY;");
+
+ // Some simple illegal joins.
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a2._KEY != a1._KEY;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID != a2.ID and a1.TITLE = a2.TITLE and a1.PRICE = a2.PRICE;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID != a2.ID and a1.TITLE != a2.TITLE and a1.PRICE != a2.PRICE;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID and a1.PRICE = a2.PRICE;");
+
+ if (!joinType.contains("RIGHT")) {
+ // For inner join we can check it because join conditions are parsed to WHERE clause.
+ boolean shouldWarn = !joinType.contains("LEFT");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY " + joinType + " A a3 on a1._KEY != a3._KEY;");
+
+ checkLogListener(!shouldWarn,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY != a2._KEY " + joinType + " A a3 on a1._KEY = a3._KEY;");
+ }
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithComplexPrimaryKeySingleAffKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT, TITLE VARCHAR, PRICE INT, COMMENT VARCHAR, PRIMARY KEY (ID, TITLE, PRICE))" +
+ " with \"AFFINITY_KEY=ID\";"));
+
+ checkSameTableWithComplexPrimaryKeySingleAffKeyWithJoinType("LEFT JOIN");
+ checkSameTableWithComplexPrimaryKeySingleAffKeyWithJoinType("RIGHT JOIN");
+ checkSameTableWithComplexPrimaryKeySingleAffKeyWithJoinType("INNER JOIN");
+ checkSameTableWithComplexPrimaryKeySingleAffKeyWithJoinType("JOIN");
+ }
+
+ /** */
+ private void checkSameTableWithComplexPrimaryKeySingleAffKeyWithJoinType(String joinType) {
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID and a1.TITLE != a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.TITLE != a2.TITLE and a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 where a1.ID in (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID) t" +
+ " on a1.ID = t.ID;");
+
+ if (!joinType.contains("RIGHT"))
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID " + joinType + " A a3 on a1.ID = a3.ID;");
+
+ // Some simple illegal joins.
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID != a2.ID;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID != a2.ID and a1.TITLE = a2.TITLE and a1.PRICE = a2.PRICE;");
+
+ checkLogListener(true,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.TITLE = a2.TITLE;");
+
+ if (!joinType.contains("RIGHT")) {
+ // For inner join we can check it because join conditions are parsed to WHERE clause.
+ boolean shouldWarn = !joinType.contains("LEFT");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID " + joinType + " A a3 on a1.ID != a3.ID;");
+
+ checkLogListener(!shouldWarn,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID != a2.ID " + joinType + " A a3 on a1.ID = a3.ID;");
+ }
+ }
+
+ /** */
+ @Test
+ public void joinWithPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR);"));
+
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE B (ID INT PRIMARY KEY, PRICE INT);"));
+
+ checkJoinPrimaryKeyWithJoinType("LEFT JOIN");
+ checkJoinPrimaryKeyWithJoinType("RIGHT JOIN");
+ checkJoinPrimaryKeyWithJoinType("INNER JOIN");
+ checkJoinPrimaryKeyWithJoinType("JOIN");
+ }
+
+ /** */
+ private void checkJoinPrimaryKeyWithJoinType(String joinType) {
+ checkLogListener(false,
+ "SELECT a.* FROM A a " + joinType + " B b on a.ID = b.ID;");
+
+ checkLogListener(false,
+ "SELECT a.* FROM A a " + joinType + " B b on b.ID = a.ID;");
+
+ checkLogListener(false,
+ "SELECT a.* FROM A a " + joinType + " B b on b._KEY = a._KEY;");
+
+ checkLogListener(false,
+ "SELECT a.* FROM A a " + joinType + " B b on b.ID = a.ID and a.TITLE != 'Title';");
+
+ checkLogListener(false,
+ "SELECT a.* FROM A a " + joinType + " B b on b.ID = a.ID and b.PRICE > 100;");
+
+ checkLogListener(false,
+ "SELECT a.* FROM A a " + joinType + " B b on b.ID = a.ID and b.PRICE != a.ID;");
+
+ // Some wrong queries.
+
+ checkLogListener(true,
Review comment:
Need at least some tests with WHERE instead of ON
##########
File path: modules/indexing/src/test/java/org/apache/ignite/sqltests/CheckWarnJoinPartitionedTables.java
##########
@@ -0,0 +1,408 @@
+/*
+ * 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.sqltests;
+
+import java.util.List;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.cache.query.FieldsQueryCursor;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** Check that illegal joins of partitioned tables are warned. */
+public class CheckWarnJoinPartitionedTables extends GridCommonAbstractTest {
+ /** */
+ private final ListeningTestLogger testLog = new ListeningTestLogger(log);
+
+ /** */
+ private Ignite crd;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.setGridLogger(testLog);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ crd = startGrid();
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR);"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
Review comment:
Aren't INNER JOIN and JOIN the same thing?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8744: [WIP] IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8744:
URL: https://github.com/apache/ignite/pull/8744#discussion_r663107316
##########
File path: modules/indexing/src/test/java/org/apache/ignite/sqltests/CheckWarnJoinPartitionedTables.java
##########
@@ -0,0 +1,408 @@
+/*
+ * 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.sqltests;
+
+import java.util.List;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.cache.query.FieldsQueryCursor;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** Check that illegal joins of partitioned tables are warned. */
+public class CheckWarnJoinPartitionedTables extends GridCommonAbstractTest {
+ /** */
+ private final ListeningTestLogger testLog = new ListeningTestLogger(log);
+
+ /** */
+ private Ignite crd;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.setGridLogger(testLog);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ crd = startGrid();
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR);"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
+ checkSameTableWithJoinType("JOIN");
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryAffinityKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR) with \"AFFINITY_KEY=ID\";"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
+ checkSameTableWithJoinType("JOIN");
+ }
+
+ /** */
+ private void checkSameTableWithJoinType(String joinType) {
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a2.ID = a1.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1._KEY = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 where a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 where a1.ID = a2._KEY;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND a1.TITLE = a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND a1.TITLE != a2.TITLE;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on a1.ID = a2.ID AND (a1.TITLE = a2.TITLE OR a1.ID = a2.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " A a2 on (a1.TITLE = a2.TITLE OR a1.ID = a2.ID) AND a1.ID = a2.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2) a3 on a1.ID = a3.ID;");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 where a1.ID in (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID);");
+
+ checkLogListener(false,
+ "SELECT a1.* FROM A a1 " + joinType + " (select a2.ID from A a2 " + joinType + " A a3 on a2.ID = a3.ID) t" +
+ " on a1.ID = t.ID;");
+
+ // Ignite doesn't support this.
+ if (!joinType.contains("RIGHT"))
Review comment:
Yes, SQL engine write a message about unsupported clause.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8744: IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8744:
URL: https://github.com/apache/ignite/pull/8744#discussion_r688434010
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/QueryPartitionsChecker.java
##########
@@ -0,0 +1,355 @@
+/*
+ * 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.h2.sql;
+
+import java.util.HashSet;
+import java.util.Set;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.internal.processors.query.h2.opt.GridH2Table;
+
+/**
+ * Traverse over query AST to find info about partitioned table usage.
+ */
+class QueryPartitionsChecker {
Review comment:
Rename to SqlAstTraverser.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8744: [WIP] IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8744:
URL: https://github.com/apache/ignite/pull/8744#discussion_r663126698
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/SplitterUtils.java
##########
@@ -286,6 +290,255 @@ public static boolean hasPartitionedTables(GridSqlAst ast) {
return false;
}
+ /**
+ * Traverse AST while join operation isn't found. Check it if found.
+ */
+ public static void lookForPartitionedJoin(GridSqlAst ast, GridSqlAst upWhere, IgniteLogger log) {
Review comment:
I added more comments in this method. Now it should be more straightforward. WDYT?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8744: [WIP] IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8744:
URL: https://github.com/apache/ignite/pull/8744#discussion_r663107021
##########
File path: modules/indexing/src/test/java/org/apache/ignite/sqltests/CheckWarnJoinPartitionedTables.java
##########
@@ -0,0 +1,408 @@
+/*
+ * 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.sqltests;
+
+import java.util.List;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.cache.query.FieldsQueryCursor;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** Check that illegal joins of partitioned tables are warned. */
+public class CheckWarnJoinPartitionedTables extends GridCommonAbstractTest {
+ /** */
+ private final ListeningTestLogger testLog = new ListeningTestLogger(log);
+
+ /** */
+ private Ignite crd;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.setGridLogger(testLog);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ crd = startGrid();
+ }
+
+ /** */
+ @Test
+ public void joinSameTableWithPrimaryKey() {
+ execute(crd, new SqlFieldsQuery(
+ "CREATE TABLE A (ID INT PRIMARY KEY, TITLE VARCHAR);"));
+
+ checkSameTableWithJoinType("LEFT JOIN");
+ checkSameTableWithJoinType("RIGHT JOIN");
+ checkSameTableWithJoinType("INNER JOIN");
Review comment:
Actually don't find a docs about equality of JOIN and INNER JOIN. So test them both.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] AMashenkov merged pull request #8744: IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
AMashenkov merged pull request #8744:
URL: https://github.com/apache/ignite/pull/8744
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] AMashenkov commented on a change in pull request #8744: IGNITE-13019 Fail on non-colocated join
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #8744:
URL: https://github.com/apache/ignite/pull/8744#discussion_r667805837
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/QueryPartitionsChecker.java
##########
@@ -0,0 +1,355 @@
+/*
+ * 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.h2.sql;
+
+import java.util.HashSet;
+import java.util.Set;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.internal.processors.query.h2.opt.GridH2Table;
+
+/**
+ * Traverse over query AST to find info about partitioned table usage.
+ */
+class QueryPartitionsChecker {
Review comment:
Irrelevant class name.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org