danny0405 commented on a change in pull request #13289:
URL: https://github.com/apache/flink/pull/13289#discussion_r482002500
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/calcite/FlinkPlannerImpl.scala
##########
@@ -163,7 +163,35 @@ class FlinkPlannerImpl(
sqlValidator.getCatalogReader.unwrap(classOf[CalciteCatalogReader]),
cluster,
convertletTable,
- sqlToRelConverterConfig)
+ sqlToRelConverterConfig) {
+ // override convertFrom() to support flexible Temporal Table Syntax,
+ // this can be revert once FLINK-16579(Upgrade Calcite version to
1.23) resolved.
+ val relBuilder = config.getRelBuilderFactory.create(cluster, null)
+
Review comment:
`val relBuilder: RelBuilder` to eliminate the IDEA warnings.
##########
File path:
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/plan/stream/sql/join/LookupJoinTest.scala
##########
@@ -489,9 +493,12 @@ class LookupJoinTest(legacyTableSource: Boolean) extends
TableTestBase with Seri
}
object LookupJoinTest {
- @Parameterized.Parameters(name = "LegacyTableSource={0}")
+ @Parameterized.Parameters(name = "LegacyTableSource={0},
useComputedColumn={1}")
def parameters(): JCollection[Array[Object]] = {
- Seq[Array[AnyRef]](Array(JBoolean.TRUE), Array(JBoolean.FALSE))
+ Seq[Array[AnyRef]](
Review comment:
No need to add another param `useComputedColumn` i think. Just add new
tables and new cases you want to test.
##########
File path:
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/runtime/batch/sql/join/LookupJoinITCase.scala
##########
@@ -208,15 +247,19 @@ class LookupJoinITCase(legacyTableSource: Boolean,
isAsyncMode: Boolean) extends
@Test
def testLeftJoinTemporalTable(): Unit = {
- val sql = s"SELECT T.id, T.len, D.name, D.age FROM T LEFT JOIN userTable "
+
+ if (legacyTableSource) {
+ //Computed column do not support in legacyTableSource.
Review comment:
You can use junit `Assume.assumeTrue()` instead.
PS, can we just create another table with computed column there and add test
cases for this patch ? It is not necessary to add another test param which
makes the test looks verbose and complex.
----------------------------------------------------------------
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:
[email protected]
