KurtYoung commented on a change in pull request #11727:
URL: https://github.com/apache/flink/pull/11727#discussion_r411395660
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java
##########
@@ -142,7 +144,7 @@
"Unsupported SQL query! sqlUpdate() only accepts a
single SQL statement of type " +
"INSERT, CREATE TABLE, DROP TABLE, ALTER TABLE, USE
CATALOG, USE [CATALOG.]DATABASE, " +
"CREATE DATABASE, DROP DATABASE, ALTER DATABASE, CREATE
FUNCTION, " +
- "DROP FUNCTION, ALTER FUNCTION, CREATE CATALOG.";
+ "DROP FUNCTION, ALTER FUNCTION, CREATE CATALOG, CREATE
VIEW, DROP VIEW.";
private static final String UNSUPPORTED_QUERY_IN_EXECUTE_SQL_MSG =
Review comment:
I believe this also should be updated since you added create/drop view
support to method `executeOperation`
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java
##########
@@ -770,7 +803,7 @@ private TableResult executeOperation(Operation operation) {
} else if (operation instanceof ShowFunctionsOperation) {
return buildShowResult(listFunctions());
} else {
- throw new
TableException(UNSUPPORTED_QUERY_IN_EXECUTE_SQL_MSG);
+ throw new
TableException(UNSUPPORTED_QUERY_IN_SQL_UPDATE_MSG);
Review comment:
We should avoid to use `UNSUPPORTED_QUERY_IN_SQL_UPDATE_MSG` anymore
since `sqlUpdate` will be deprecated soon.
##########
File path:
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/catalog/CatalogTableITCase.scala
##########
@@ -946,6 +946,485 @@ class CatalogTableITCase(isStreamingMode: Boolean)
extends AbstractTestBase {
expectedProperty.put("k2", "b")
assertEquals(expectedProperty, database.getProperties)
}
+
Review comment:
Can you move most of these newly introduced tests to
`TableEnvironmentTest`? I'm not sure why we need IT cases for testing view
support, and I also don't get the idea why we put view related tests in this
`CatalogTableITCase`
##########
File path:
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/catalog/CatalogTableITCase.scala
##########
@@ -946,6 +946,485 @@ class CatalogTableITCase(isStreamingMode: Boolean)
extends AbstractTestBase {
expectedProperty.put("k2", "b")
assertEquals(expectedProperty, database.getProperties)
}
+
Review comment:
And please use `executeSql` instead of `sqlUpdate` if possible
----------------------------------------------------------------
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]
