From d107f4825d40998b9e40a5270b8df48dd48b8eb2 Mon Sep 17 00:00:00 2001 From: ramkrishna Date: Mon, 21 Mar 2016 22:03:42 +0530 Subject: [PATCH 1/2] [FLINK-3631] CodeGenerator does not check type compatibility for equality expressions --- .../api/table/codegen/CodeGenerator.scala | 14 ++++++++ .../table/test/StringExpressionsITCase.java | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/codegen/CodeGenerator.scala b/flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/codegen/CodeGenerator.scala index c7d0ed906c11a..f213d4cdffea4 100644 --- a/flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/codegen/CodeGenerator.scala +++ b/flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/codegen/CodeGenerator.scala @@ -652,31 +652,37 @@ class CodeGenerator( case EQUALS => val left = operands.head val right = operands(1) + checkNumericOrString(left, right) generateEquals(nullCheck, left, right) case NOT_EQUALS => val left = operands.head val right = operands(1) + checkNumericOrString(left, right) generateNotEquals(nullCheck, left, right) case GREATER_THAN => val left = operands.head val right = operands(1) + checkNumericOrString(left, right) generateComparison(">", nullCheck, left, right) case GREATER_THAN_OR_EQUAL => val left = operands.head val right = operands(1) + checkNumericOrString(left, right) generateComparison(">=", nullCheck, left, right) case LESS_THAN => val left = operands.head val right = operands(1) + checkNumericOrString(left, right) generateComparison("<", nullCheck, left, right) case LESS_THAN_OR_EQUAL => val left = operands.head val right = operands(1) + checkNumericOrString(left, right) generateComparison("<=", nullCheck, left, right) case IS_NULL => @@ -736,6 +742,14 @@ class CodeGenerator( } } + def checkNumericOrString(left: GeneratedExpression, right: GeneratedExpression): Unit = { + if (isNumeric(left)) { + requireNumeric(right) + } else if (isString(left)) { + requireString(right) + } + } + override def visitOver(over: RexOver): GeneratedExpression = ??? // ---------------------------------------------------------------------------------------------- diff --git a/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java b/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java index d95a5f6054226..646ac17fd68e6 100644 --- a/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java +++ b/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java @@ -18,6 +18,9 @@ package org.apache.flink.api.java.table.test; + +import org.apache.flink.api.java.tuple.Tuple3; +import org.apache.flink.test.javaApiOperators.util.CollectionDataSets; import org.apache.flink.api.table.Row; import org.apache.flink.api.table.Table; import org.apache.flink.api.table.codegen.CodeGenException; @@ -117,4 +120,34 @@ public void testNonWorkingSubstring2() throws Exception { DataSet resultSet = tableEnv.toDataSet(result, Row.class); resultSet.collect(); } + + @Test(expected = CodeGenException.class) + public void testGeneratedCodeForStringComparison() throws Exception { + ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment(); + TableEnvironment tableEnv = new TableEnvironment(); + DataSet> tupleDataSet = CollectionDataSets.get3TupleDataSet(env); + Table in = tableEnv.fromDataSet(tupleDataSet, "a, b, c"); + Table res = in.filter("a = 'Fred'" ); + DataSet resultSet = tableEnv.toDataSet(res, Row.class); + } + + @Test(expected = CodeGenException.class) + public void testGeneratedCodeForIntegerEqualsComparison() throws Exception { + ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment(); + TableEnvironment tableEnv = new TableEnvironment(); + DataSet> tupleDataSet = CollectionDataSets.get3TupleDataSet(env); + Table in = tableEnv.fromDataSet(tupleDataSet, "a, b, c"); + Table res = in.filter("c = 10" ); + DataSet resultSet = tableEnv.toDataSet(res, Row.class); + } + + @Test(expected = CodeGenException.class) + public void testGeneratedCodeForIntegerGreaterComparison() throws Exception { + ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment(); + TableEnvironment tableEnv = new TableEnvironment(); + DataSet> tupleDataSet = CollectionDataSets.get3TupleDataSet(env); + Table in = tableEnv.fromDataSet(tupleDataSet, "a, b, c"); + Table res = in.filter("c > 10" ); + DataSet resultSet = tableEnv.toDataSet(res, Row.class); + } } From 09610f365a9a8041a02a867ac8b31faa47975bfe Mon Sep 17 00:00:00 2001 From: ramkrishna Date: Tue, 22 Mar 2016 10:39:58 +0530 Subject: [PATCH 2/2] Adding comments as why to fail --- .../flink/api/java/table/test/StringExpressionsITCase.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java b/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java index 646ac17fd68e6..65f04708c09c8 100644 --- a/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java +++ b/flink-libraries/flink-table/src/test/java/org/apache/flink/api/java/table/test/StringExpressionsITCase.java @@ -127,6 +127,7 @@ public void testGeneratedCodeForStringComparison() throws Exception { TableEnvironment tableEnv = new TableEnvironment(); DataSet> tupleDataSet = CollectionDataSets.get3TupleDataSet(env); Table in = tableEnv.fromDataSet(tupleDataSet, "a, b, c"); + // Must fail because the comparison here is between Integer(column 'a') and (String 'Fred') Table res = in.filter("a = 'Fred'" ); DataSet resultSet = tableEnv.toDataSet(res, Row.class); } @@ -137,6 +138,7 @@ public void testGeneratedCodeForIntegerEqualsComparison() throws Exception { TableEnvironment tableEnv = new TableEnvironment(); DataSet> tupleDataSet = CollectionDataSets.get3TupleDataSet(env); Table in = tableEnv.fromDataSet(tupleDataSet, "a, b, c"); + // Must fail because the comparison here is between String(column 'c') and (Integer 10) Table res = in.filter("c = 10" ); DataSet resultSet = tableEnv.toDataSet(res, Row.class); } @@ -147,6 +149,7 @@ public void testGeneratedCodeForIntegerGreaterComparison() throws Exception { TableEnvironment tableEnv = new TableEnvironment(); DataSet> tupleDataSet = CollectionDataSets.get3TupleDataSet(env); Table in = tableEnv.fromDataSet(tupleDataSet, "a, b, c"); + // Must fail because the comparison here is between String(column 'c') and (Integer 10) Table res = in.filter("c > 10" ); DataSet resultSet = tableEnv.toDataSet(res, Row.class); }