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 2020/05/05 08:46:42 UTC
[GitHub] [flink] twalthr commented on a change in pull request #11989: [FLINK-16804] Deprecate all Table methods that accept expressions as a String
twalthr commented on a change in pull request #11989:
URL: https://github.com/apache/flink/pull/11989#discussion_r419935875
##########
File path: flink-formats/flink-avro/src/test/java/org/apache/flink/table/runtime/batch/AvroTypesITCase.java
##########
@@ -194,8 +195,9 @@ public void testAvroObjectAccess() throws Exception {
Table t = tEnv.fromDataSet(testData(env));
Table result = t
- .filter("type_nested.isNotNull")
- .select("type_nested.flatten()").as("city, num, state, street, zip");
+ .filter($("type_nested").isNotNull())
+ .select($("type_nested").flatten())
+ .as("city, num, state, street, zip");
Review comment:
still uses deprecated method
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/FlatAggregateTable.java
##########
@@ -56,14 +58,26 @@
* <p><b>Note</b>: You have to close the flatAggregate with a select statement. And the select
* statement does not support aggregate functions.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * TableAggregateFunction tableAggFunc = new MyTableAggregateFunction
+ * tableEnv.registerFunction("tableAggFunc", tableAggFunc);
+ * tab.groupBy("key")
Review comment:
update
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/GroupedTable.java
##########
@@ -71,22 +73,36 @@
* .select("key, f0, f1")
* }
* </pre>
+ * @deprecated use {@link #aggregate(Expression)}
*/
+ @Deprecated
AggregatedTable aggregate(String aggregateFunction);
/**
* Performs an aggregate operation with an aggregate function. You have to close the
* {@link #aggregate(Expression)} with a select statement. The output will be flattened if the
* output type is a composite type.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * AggregateFunction aggFunc = new MyAggregateFunction
Review comment:
missing `;` here and below
##########
File path: flink-examples/flink-examples-table/src/main/java/org/apache/flink/table/examples/java/WordCountTable.java
##########
@@ -48,9 +50,9 @@ public static void main(String[] args) throws Exception {
Table table = tEnv.fromDataSet(input);
Review comment:
Update `org.apache.flink.table.examples.java.WordCountSQL` and classes in `org.apache.flink.table.examples.scala` (with `$""`) as well.
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/GroupedTable.java
##########
@@ -107,21 +123,35 @@
* .select("key, x, y, z")
* }
* </pre>
+ * @deprecated use {@link #flatAggregate(Expression)}
*/
+ @Deprecated
FlatAggregateTable flatAggregate(String tableAggFunction);
/**
* Performs a flatAggregate operation on a grouped table. FlatAggregate takes a
* TableAggregateFunction which returns multiple rows. Use a selection after flatAggregate.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * TableAggregateFunction tableAggFunc = new MyTableAggregateFunction
Review comment:
missing `;` and update groupBy
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -295,7 +347,7 @@
*
* <pre>
* {@code
- * left.join(right).where("a = b && c > 3").select("a, b, d")
+ * left.join(right).where($("a").isEqual($("b")).and($("c").isGreater(3)).select($("a"), $("b"), $("d"))
Review comment:
split line per operation here and below
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/GroupedTable.java
##########
@@ -71,22 +73,36 @@
* .select("key, f0, f1")
* }
* </pre>
+ * @deprecated use {@link #aggregate(Expression)}
*/
+ @Deprecated
AggregatedTable aggregate(String aggregateFunction);
/**
* Performs an aggregate operation with an aggregate function. You have to close the
* {@link #aggregate(Expression)} with a select statement. The output will be flattened if the
* output type is a composite type.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * AggregateFunction aggFunc = new MyAggregateFunction
+ * tableEnv.registerFunction("aggFunc", aggFunc);
+ * tab.groupBy("key")
Review comment:
update
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -111,7 +113,15 @@
*
* <pre>
* {@code
- * tab.select('key, 'value.avg + " The average" as 'average)
+ * tab.select($("key"), $("value").avg(), lit("The average").as("average"))
+ * }
+ * </pre>
+ *
+ * <p>Scala Example:
+ *
+ * <pre>
+ * {@code
+ * tab.select($"key", $"value".avg, "The average" as "average")
Review comment:
should be `+`
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -914,18 +1106,29 @@
* tab.addOrReplaceColumns("a + 1 as a1, concat(b, 'sunny') as b1")
* }
* </pre>
+ * @deprecated use {@link #addOrReplaceColumns(Expression...)}
*/
+ @Deprecated
Table addOrReplaceColumns(String fields);
/**
* Adds additional columns. Similar to a SQL SELECT statement. The field expressions
* can contain complex expressions, but can not contain aggregations. Existing fields will be
* replaced. If the added fields have duplicate field name, then the last one is used.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * tab.addOrReplaceColumns($("a").plus(1).as("a1"), concat($("b"), "sunny").as("b1"))
Review comment:
new line for every column
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -472,14 +564,31 @@
* table.joinLateral("split(c) as (s)").select("a, b, c, s");
* }
* </pre>
+ * @deprecated use {@link #joinLateral(Expression)}
*/
+ @Deprecated
Table joinLateral(String tableFunctionCall);
/**
* Joins this {@link Table} with an user-defined {@link TableFunction}. This join is similar to
* a SQL inner join with ON TRUE predicate but works with a table function. Each row of the
* table is joined with all rows produced by the table function.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * class MySplitUDTF extends TableFunction<String> {
+ * public void eval(String str) {
+ * str.split("#").forEach(this::collect);
+ * }
+ * }
+ *
+ * TableFunction<String> split = new MySplitUDTF()
+ * table.joinLateral(call(split, $("c")).as("s")).select($("a"), $("b"), $("c"), $("s"))
Review comment:
use `MySplitUDTF.class`? we should advocate the shorter approach without an instance
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/OverWindowedTable.java
##########
@@ -42,13 +42,23 @@
* overWindowedTable.select("c, b.count over ow, e.sum over ow")
* }
* </pre>
+ * @deprecated use {@link #select(Expression...)}
*/
+ @Deprecated
Table select(String fields);
/**
* Performs a selection operation on a over windowed table. Similar to an SQL SELECT statement.
* The field expressions can contain complex expressions and aggregations.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * overWindowedTable.select($("c"), $("b").count().over($("ow")), $("e").sum().over($("ow")))
Review comment:
make the example more readable by newline for each select column
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -884,19 +1066,29 @@
* tab.addColumns("a + 1 as a1, concat(b, 'sunny') as b1")
* }
* </pre>
+ * @deprecated use {@link #addColumns(Expression...)}
*/
+ @Deprecated
Table addColumns(String fields);
/**
* Adds additional columns. Similar to a SQL SELECT statement. The field expressions
* can contain complex expressions, but can not contain aggregations. It will throw an exception
* if the added fields already exist.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * tab.addColumns($("a").plus(1).as("a1"), concat($("b"), "sunny").as("b1"))
Review comment:
new line for every column
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -1095,20 +1351,32 @@
* .select("x, y, z")
* }
* </pre>
+ * @deprecated use {@link #flatAggregate(Expression)}
*/
+ @Deprecated
FlatAggregateTable flatAggregate(String tableAggregateFunction);
/**
* Perform a global flatAggregate without groupBy. FlatAggregate takes a TableAggregateFunction
* which returns multiple rows. Use a selection after the flatAggregate.
*
+ * <p>Example:
+ *
+ * <pre>
+ * {@code
+ * TableAggregateFunction tableAggFunc = new MyTableAggregateFunction
Review comment:
missing `;`, use also `.class` instead?
##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Table.java
##########
@@ -111,7 +113,15 @@
*
* <pre>
* {@code
- * tab.select('key, 'value.avg + " The average" as 'average)
+ * tab.select($("key"), $("value").avg(), lit("The average").as("average"))
Review comment:
should be `plus()`
##########
File path: flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/runtime/stream/table/ValuesITCase.java
##########
@@ -291,7 +295,9 @@ public void testProjectionWithValues() throws Exception {
tEnv().createTemporaryFunction("func", new CustomScalarFunction());
Table t = tEnv().fromValues(data)
- .select("func(f0, f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11, f12, f13, f14, f15)");
+ .select(call(
+ "func",
+ IntStream.rangeClosed(0, 15).mapToObj(idx -> $("f" + idx)).toArray(Expression[]::new)));
Review comment:
use our fancy expressions for selecting a range of columns
----------------------------------------------------------------
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