wuchong commented on a change in pull request #15195:
URL: https://github.com/apache/flink/pull/15195#discussion_r594399049
##########
File path:
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/plan/stream/sql/join/WindowJoinTest.scala
##########
@@ -562,11 +538,8 @@ class WindowJoinTest extends TableTestBase {
thrown.expect(classOf[TableException])
thrown.expectMessage(
- s"""
- |Currently, window starts equality and window ends equality are both
required for
- |window join. In the future, we could support join clause which only
includes window
- |starts equality or window ends equality for TUMBLE or HOP window.
- |""".stripMargin)
+ "Currently, window join requires JOIN ON condition must contain both
window starts " +
+ "equality of input tables and window ends equality of input tables.")
Review comment:
Please also verify the full exception message.
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/utils/WindowJoinUtil.scala
##########
@@ -185,25 +186,22 @@ object WindowJoinUtil {
if (
leftWindowProperties.getTimeAttributeType !=
rightWindowProperties.getTimeAttributeType) {
throw new TableException(
- s"""
- |Currently, time attribute type of left and right inputs should
be both row-time or
- |both proc-time. In the future, we could support different time
attribute type.
- |""".stripMargin)
+ "Currently, window join doesn't support different time attribute
type of left and " +
+ "right inputs.\n" +
+ s"The left time attribute type is
${leftWindowProperties.getTimeAttributeType}.\n" +
+ s"The right time attribute type is
${rightWindowProperties.getTimeAttributeType}.")
} else if (leftWindowProperties.getWindowSpec !=
rightWindowProperties.getWindowSpec) {
throw new TableException(
- s"""
- |Currently, the windowing TVFs must be the same of left and right
inputs.
- |In the future, we could support different window TVFs, for
example, tumbling windows
- | join sliding windows with the same window size.
- |""".stripMargin)
+ "Currently, window join doesn't support different window table
function of left and " +
+ "right inputs.\n" +
+ s"The left window table function is ${leftWindowProperties}.\n" +
Review comment:
We just need to print the windowSpec instead of the total
`leftWindowProperties`. Besides, we need to implement `toString` method for
WindowSpecs.
##########
File path:
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/plan/stream/sql/join/WindowJoinTest.scala
##########
@@ -562,11 +538,8 @@ class WindowJoinTest extends TableTestBase {
thrown.expect(classOf[TableException])
thrown.expectMessage(
- s"""
- |Currently, window starts equality and window ends equality are both
required for
- |window join. In the future, we could support join clause which only
includes window
- |starts equality or window ends equality for TUMBLE or HOP window.
- |""".stripMargin)
+ "Currently, window join requires JOIN ON condition must contain both
window starts " +
+ "equality of input tables and window ends equality of input tables.")
Review comment:
Please also verify the full exception message.
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/utils/WindowJoinUtil.scala
##########
@@ -185,25 +186,22 @@ object WindowJoinUtil {
if (
leftWindowProperties.getTimeAttributeType !=
rightWindowProperties.getTimeAttributeType) {
throw new TableException(
- s"""
- |Currently, time attribute type of left and right inputs should
be both row-time or
- |both proc-time. In the future, we could support different time
attribute type.
- |""".stripMargin)
+ "Currently, window join doesn't support different time attribute
type of left and " +
+ "right inputs.\n" +
+ s"The left time attribute type is
${leftWindowProperties.getTimeAttributeType}.\n" +
+ s"The right time attribute type is
${rightWindowProperties.getTimeAttributeType}.")
} else if (leftWindowProperties.getWindowSpec !=
rightWindowProperties.getWindowSpec) {
throw new TableException(
- s"""
- |Currently, the windowing TVFs must be the same of left and right
inputs.
- |In the future, we could support different window TVFs, for
example, tumbling windows
- | join sliding windows with the same window size.
- |""".stripMargin)
+ "Currently, window join doesn't support different window table
function of left and " +
+ "right inputs.\n" +
+ s"The left window table function is ${leftWindowProperties}.\n" +
+ s"The right window table function is ${rightWindowProperties}.")
}
} else if (windowStartEqualityLeftKeys.nonEmpty ||
windowEndEqualityLeftKeys.nonEmpty) {
throw new TableException(
- s"""
- |Currently, window starts equality and window ends equality are
both required for
- |window join. In the future, we could support join clause which
only includes window
- |starts equality or window ends equality for TUMBLE or HOP window.
- |""".stripMargin)
+ "Currently, window join requires JOIN ON condition must contain both
window starts " +
+ "equality of input tables and window ends equality of input
tables.\n" +
+ s"But the current JOIN ON condition is ${join.getCondition}.")
Review comment:
Currently, it prints:
```
But the current JOIN ON condition is AND(=($2, $7), =($0, $5)).
```
I think it unreadable and not helpful for users. Would be better to display
field names.
----------------------------------------------------------------
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:
us...@infra.apache.org
