You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/03/18 10:23:14 UTC

[GitHub] [flink] twalthr commented on a change in pull request #19148: [FLINK-26704][table] Remove string expressions

twalthr commented on a change in pull request #19148:
URL: https://github.com/apache/flink/pull/19148#discussion_r829858048



##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/plan/stream/table/validation/CalcValidationTest.scala
##########
@@ -108,7 +108,7 @@ class CalcValidationTest extends TableTestBase {
   def testDropColumnsWithValueLiteral(): Unit = {
     val util = streamTestUtil()
     val tab = util.addTableSource[(Int, Long, String)]("Table3",'a, 'b, 'c)
-    tab.dropColumns("'a'")
+    tab.dropColumns("a")

Review comment:
       why is this still a string and not a ref?

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/ExpressionTestBase.scala
##########
@@ -304,23 +296,23 @@ abstract class ExpressionTestBase {
   }
 
   private def addTableApiTestExpr(
-      tableApiExpr: Expression,
-      expected: String,
-      exprsContainer: mutable.ArrayBuffer[_],
-      exceptionClass: Class[_ <: Throwable] = null): Unit = {
+       tableApiExpr: Expression,

Review comment:
       double space indention

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/ExpressionTestBase.scala
##########
@@ -214,23 +211,23 @@ abstract class ExpressionTestBase {
   }
 
   def testExpectedTableApiException(
-      expr: Expression,

Review comment:
       double space indention

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/MapTypeTestBase.scala
##########
@@ -59,16 +59,16 @@ abstract class MapTypeTestBase extends ExpressionTestBase {
 
   override def typeInfo: RowTypeInfo = {
     new RowTypeInfo(
-      /* 0 */  new MapTypeInfo(BasicTypeInfo.STRING_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO),

Review comment:
       undo changes in this file for now

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/ExpressionTestBase.scala
##########
@@ -304,23 +296,23 @@ abstract class ExpressionTestBase {
   }
 
   private def addTableApiTestExpr(
-      tableApiExpr: Expression,
-      expected: String,
-      exprsContainer: mutable.ArrayBuffer[_],
-      exceptionClass: Class[_ <: Throwable] = null): Unit = {
+       tableApiExpr: Expression,
+       expected: String,
+       exprsContainer: mutable.ArrayBuffer[_],
+       exceptionClass: Class[_ <: Throwable] = null): Unit = {
     // create RelNode from Table API expression
     val relNode = relBuilder
-        .queryOperation(tEnv.from(tableName).select(tableApiExpr).getQueryOperation).build()
+      .queryOperation(tEnv.from(tableName).select(tableApiExpr).getQueryOperation).build()
 
     addTestExpr(relNode, expected, tableApiExpr.asSummaryString(), null, exprsContainer)
   }
 
   private def addTestExpr(
-      relNode: RelNode,
-      expected: String,
-      summaryString: String,
-      exceptionClass: Class[_ <: Throwable],
-      exprs: mutable.ArrayBuffer[_]): Unit = {
+       relNode: RelNode,

Review comment:
       double space indention

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/RowTypeTestBase.scala
##########
@@ -57,9 +57,9 @@ abstract class RowTypeTestBase extends ExpressionTestBase {
       /* 2 */ Types.ROW(Types.INT, Types.STRING, Types.BOOLEAN),
       /* 3 */ Types.ROW(Types.INT, Types.ROW(Types.INT, Types.STRING, Types.BOOLEAN)),
       /* 4 */ Types.ROW(
-                Types.LOCAL_DATE,

Review comment:
       undo changes in this file for now

##########
File path: flink-python/pyflink/table/tests/test_pandas_udaf.py
##########
@@ -107,10 +107,10 @@ def test_group_aggregate_with_aux_group(self):
                                                      result_type=DataTypes.INT(),
                                                      func_type="pandas"))
         self.t_env.create_temporary_system_function("mean_udaf", mean_udaf)
-        t.group_by("a") \
-            .select("a, a + 1 as b, a + 2 as c") \
-            .group_by("a, b") \
-            .select("a, b, mean_udaf(b), max_add(b, c, 1)") \
+        t.group_by(t.a) \
+            .select(t.a,  (t.a + 1).alias("b"), (t.a + 2).alias("c")) \
+            .group_by(t.a, t.b) \
+            .select(t.a, t.b, mean_udaf(t.b), expr.call("max_add", t.b, t.c, 1)) \

Review comment:
       can't we import `expr` as we did in `flink-end-to-end-tests/flink-python-test/python/python_job.py` to call `call` and `col` directly?

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/ScalarOperatorsTestBase.scala
##########
@@ -59,32 +59,32 @@ abstract class ScalarOperatorsTestBase extends ExpressionTestBase {
 
   override def testDataType: AbstractDataType[_] = {
     DataTypes.ROW(
-        DataTypes.FIELD("f0", DataTypes.TINYINT()),
-        DataTypes.FIELD("f1", DataTypes.SMALLINT()),
-        DataTypes.FIELD("f2", DataTypes.INT()),
-        DataTypes.FIELD("f3", DataTypes.BIGINT()),
-        DataTypes.FIELD("f4", DataTypes.FLOAT()),
-        DataTypes.FIELD("f5", DataTypes.DOUBLE()),
-        DataTypes.FIELD("f6", DataTypes.BOOLEAN()),
-        DataTypes.FIELD("f7", DataTypes.DOUBLE()),
-        DataTypes.FIELD("f8", DataTypes.INT()),
-        DataTypes.FIELD("f9", DataTypes.INT()),
-        DataTypes.FIELD("f10", DataTypes.STRING()),
-        DataTypes.FIELD("f11", DataTypes.BOOLEAN()),
-        DataTypes.FIELD("f12", DataTypes.BOOLEAN()),
-        DataTypes.FIELD("f13", DataTypes.ROW(
-            DataTypes.FIELD("f0", DataTypes.STRING()),
-            DataTypes.FIELD("f1", DataTypes.STRING()))
-        ),
-        DataTypes.FIELD("f14", DataTypes.STRING()),
-        DataTypes.FIELD("f15", DataTypes.DATE()),
-        DataTypes.FIELD("f16", DataTypes.DECIMAL(19, 8)),
-        DataTypes.FIELD("f17", DataTypes.DECIMAL(19, 1)),
-        DataTypes.FIELD("f18", DataTypes.BINARY(200)),
-        DataTypes.FIELD("f19", DataTypes.VARBINARY(200)),
-        DataTypes.FIELD("f20", DataTypes.VARBINARY(200)),
-        DataTypes.FIELD("f21", DataTypes.TIME()),
-        DataTypes.FIELD("f22", DataTypes.TIMESTAMP())
+      DataTypes.FIELD("f0", DataTypes.TINYINT()),
+      DataTypes.FIELD("f1", DataTypes.SMALLINT()),
+      DataTypes.FIELD("f2", DataTypes.INT()),
+      DataTypes.FIELD("f3", DataTypes.BIGINT()),
+      DataTypes.FIELD("f4", DataTypes.FLOAT()),
+      DataTypes.FIELD("f5", DataTypes.DOUBLE()),
+      DataTypes.FIELD("f6", DataTypes.BOOLEAN()),
+      DataTypes.FIELD("f7", DataTypes.DOUBLE()),
+      DataTypes.FIELD("f8", DataTypes.INT()),
+      DataTypes.FIELD("f9", DataTypes.INT()),
+      DataTypes.FIELD("f10", DataTypes.STRING()),
+      DataTypes.FIELD("f11", DataTypes.BOOLEAN()),
+      DataTypes.FIELD("f12", DataTypes.BOOLEAN()),
+      DataTypes.FIELD("f13", DataTypes.ROW(
+        DataTypes.FIELD("f0", DataTypes.STRING()),
+        DataTypes.FIELD("f1", DataTypes.STRING()))
+      ),
+      DataTypes.FIELD("f14", DataTypes.STRING()),
+      DataTypes.FIELD("f15", DataTypes.DATE()),
+      DataTypes.FIELD("f16", DataTypes.DECIMAL(19, 8)),
+      DataTypes.FIELD("f17", DataTypes.DECIMAL(19, 1)),
+      DataTypes.FIELD("f18", DataTypes.BINARY(200)),
+      DataTypes.FIELD("f19", DataTypes.VARBINARY(200)),
+      DataTypes.FIELD("f20", DataTypes.VARBINARY(200)),
+      DataTypes.FIELD("f21", DataTypes.TIME()),
+      DataTypes.FIELD("f22", DataTypes.TIMESTAMP())

Review comment:
       undo changes in this file for now

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/userDefinedScalarFunctions.scala
##########
@@ -203,8 +203,8 @@ object Func24 extends ScalarFunction {
 
 
 /**
-  * A scalar function that always returns TRUE if opened correctly.
-  */
+ * A scalar function that always returns TRUE if opened correctly.
+ */
 @SerialVersionUID(1L)

Review comment:
       undo changes in this file for now

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/CompositeTypeTestBase.scala
##########
@@ -47,16 +47,16 @@ class CompositeTypeTestBase extends ExpressionTestBase {
 
   override def typeInfo: RowTypeInfo = {
     new RowTypeInfo(
-      /* 0 */  createTypeInformation[MyCaseClass],

Review comment:
       undo changes in this file for now

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/ScalarTypesTestBase.scala
##########
@@ -96,16 +96,16 @@ abstract class ScalarTypesTestBase extends ExpressionTestBase {
 
   override def typeInfo: RowTypeInfo = {
     new RowTypeInfo(
-      /* 0 */  Types.STRING,

Review comment:
       undo changes in this file for now

##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/utils/ExpressionTestBase.scala
##########
@@ -179,9 +175,9 @@ abstract class ExpressionTestBase {
   }
 
   def testAllApis(
-      expr: Expression,

Review comment:
       double space indention




-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org