Copilot commented on code in PR #11349:
URL: https://github.com/apache/gravitino/pull/11349#discussion_r3356122909
##########
flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/integration/test/FlinkCommonIT.java:
##########
@@ -812,4 +829,285 @@ && defaultValueWithNullLiterals()) {
Assertions.assertEquals(expected[i].nullable(), actual[i].nullable());
}
}
+
+ @Test
+ @EnabledIf("supportViewOperation")
+ public void testCreateView() {
+ String schemaName = "test_create_view_db";
+ String viewName = "test_view_create";
+ String tableName = "test_view_base_table";
+ doWithSchema(
+ currentCatalog(),
+ schemaName,
+ catalog -> {
+ TestUtils.assertTableResult(
+ sql("CREATE TABLE %s (id INT, name STRING)%s", tableName,
baseTableConnectorClause()),
+ ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql(
+ "CREATE VIEW %s COMMENT 'view comment' AS SELECT id, name
FROM %s",
+ viewName, tableName),
+ ResultKind.SUCCESS);
+
+ ViewCatalog viewCatalog = catalog.asViewCatalog();
+ View view = viewCatalog.loadView(NameIdentifier.of(schemaName,
viewName));
+ Assertions.assertEquals(viewName, view.name());
+ Assertions.assertEquals("view comment", view.comment());
+ Assertions.assertEquals(1, view.representations().length);
+ Assertions.assertInstanceOf(SQLRepresentation.class,
view.representations()[0]);
Review Comment:
`GravitinoPaimonCatalog` now stores multiple SQL representations for a view
(e.g., FLINK + QUERY). This test currently asserts the view has exactly one
representation, which will fail for backends that legitimately store more than
one representation.
##########
flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/integration/test/FlinkCommonIT.java:
##########
@@ -812,4 +829,285 @@ && defaultValueWithNullLiterals()) {
Assertions.assertEquals(expected[i].nullable(), actual[i].nullable());
}
}
+
+ @Test
+ @EnabledIf("supportViewOperation")
+ public void testCreateView() {
+ String schemaName = "test_create_view_db";
+ String viewName = "test_view_create";
+ String tableName = "test_view_base_table";
+ doWithSchema(
+ currentCatalog(),
+ schemaName,
+ catalog -> {
+ TestUtils.assertTableResult(
+ sql("CREATE TABLE %s (id INT, name STRING)%s", tableName,
baseTableConnectorClause()),
+ ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql(
+ "CREATE VIEW %s COMMENT 'view comment' AS SELECT id, name
FROM %s",
+ viewName, tableName),
+ ResultKind.SUCCESS);
+
+ ViewCatalog viewCatalog = catalog.asViewCatalog();
+ View view = viewCatalog.loadView(NameIdentifier.of(schemaName,
viewName));
+ Assertions.assertEquals(viewName, view.name());
+ Assertions.assertEquals("view comment", view.comment());
+ Assertions.assertEquals(1, view.representations().length);
+ Assertions.assertInstanceOf(SQLRepresentation.class,
view.representations()[0]);
+
+ Optional<org.apache.flink.table.catalog.Catalog> flinkCatalog =
+ tableEnv.getCatalog(catalog.name());
+ Assertions.assertTrue(flinkCatalog.isPresent());
+ try {
+ CatalogBaseTable flinkTable =
+ flinkCatalog.get().getTable(new ObjectPath(schemaName,
viewName));
+ Assertions.assertEquals(CatalogBaseTable.TableKind.VIEW,
flinkTable.getTableKind());
+ } catch (TableNotExistException e) {
+ Assertions.fail("view should exist in Flink catalog: " +
e.getMessage());
+ }
+ },
+ true,
+ supportDropCascade());
+ }
+
+ @Test
+ @EnabledIf("supportViewOperation")
+ public void testListViews() {
+ String schemaName = "test_list_views_db";
+ String tableName = "test_list_view_base";
+ String view1 = "test_list_view_1";
+ String view2 = "test_list_view_2";
+ doWithSchema(
+ currentCatalog(),
+ schemaName,
+ catalog -> {
+ TestUtils.assertTableResult(
+ sql("CREATE TABLE %s (id INT)%s", tableName,
baseTableConnectorClause()),
+ ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql("CREATE VIEW %s AS SELECT id FROM %s", view1, tableName),
ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql("CREATE VIEW %s AS SELECT id FROM %s", view2, tableName),
ResultKind.SUCCESS);
+
+ List<String> views = Arrays.asList(tableEnv.listViews());
+ Assertions.assertTrue(views.contains(view1), "view1 not found in
SHOW VIEWS");
+ Assertions.assertTrue(views.contains(view2), "view2 not found in
SHOW VIEWS");
+ Assertions.assertFalse(views.contains(tableName), "table should not
appear in listViews");
+
+ ViewCatalog viewCatalog = catalog.asViewCatalog();
+ NameIdentifier[] gravitinoViews =
viewCatalog.listViews(Namespace.of(schemaName));
+ List<String> gravitinoViewNames =
+
Arrays.stream(gravitinoViews).map(NameIdentifier::name).collect(Collectors.toList());
+ Assertions.assertTrue(gravitinoViewNames.contains(view1));
+ Assertions.assertTrue(gravitinoViewNames.contains(view2));
+ },
+ true,
+ supportDropCascade());
+ }
+
+ @Test
+ @EnabledIf("supportViewOperation")
+ public void testDropView() {
+ String schemaName = "test_drop_view_db";
+ String tableName = "test_drop_view_base";
+ String viewName = "test_view_drop";
+ doWithSchema(
+ currentCatalog(),
+ schemaName,
+ catalog -> {
+ TestUtils.assertTableResult(
+ sql("CREATE TABLE %s (id INT)%s", tableName,
baseTableConnectorClause()),
+ ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql("CREATE VIEW %s AS SELECT id FROM %s", viewName, tableName),
ResultKind.SUCCESS);
+
+ ViewCatalog viewCatalog = catalog.asViewCatalog();
+ Assertions.assertTrue(
+ viewCatalog.viewExists(NameIdentifier.of(schemaName, viewName)),
+ "view should exist before drop");
+
+ TestUtils.assertTableResult(sql("DROP VIEW %s", viewName),
ResultKind.SUCCESS);
+
+ Assertions.assertFalse(
+ viewCatalog.viewExists(NameIdentifier.of(schemaName, viewName)),
+ "view should not exist after drop");
+ },
+ true,
+ supportDropCascade());
+ }
+
+ @Test
+ @EnabledIf("supportViewOperation")
+ public void testAlterViewRename() {
+ String schemaName = "test_rename_view_db";
+ String tableName = "test_rename_view_base";
+ String viewName = "test_view_rename_src";
+ String newViewName = "test_view_rename_dst";
+ doWithSchema(
+ currentCatalog(),
+ schemaName,
+ catalog -> {
+ TestUtils.assertTableResult(
+ sql("CREATE TABLE %s (id INT)%s", tableName,
baseTableConnectorClause()),
+ ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql("CREATE VIEW %s AS SELECT id FROM %s", viewName, tableName),
ResultKind.SUCCESS);
+
+ TestUtils.assertTableResult(
+ sql("ALTER VIEW %s RENAME TO %s", viewName, newViewName),
ResultKind.SUCCESS);
+
+ ViewCatalog viewCatalog = catalog.asViewCatalog();
+ Assertions.assertFalse(
+ viewCatalog.viewExists(NameIdentifier.of(schemaName, viewName)),
+ "old view name should not exist");
+ Assertions.assertTrue(
+ viewCatalog.viewExists(NameIdentifier.of(schemaName,
newViewName)),
+ "new view name should exist");
+ },
+ true,
+ supportDropCascade());
+ }
+
+ @Test
+ @EnabledIf("supportViewOperation")
+ public void testAlterViewReplaceBody() {
+ String schemaName = "test_replace_view_db";
+ String tableName = "test_replace_view_base";
+ String viewName = "test_view_replace";
+ doWithSchema(
+ currentCatalog(),
+ schemaName,
+ catalog -> {
+ TestUtils.assertTableResult(
+ sql("CREATE TABLE %s (id INT, name STRING)%s", tableName,
baseTableConnectorClause()),
+ ResultKind.SUCCESS);
+ TestUtils.assertTableResult(
+ sql("CREATE VIEW %s AS SELECT id FROM %s", viewName, tableName),
ResultKind.SUCCESS);
+
+ TestUtils.assertTableResult(
+ sql("ALTER VIEW %s AS SELECT id, name FROM %s", viewName,
tableName),
+ ResultKind.SUCCESS);
+
+ ViewCatalog viewCatalog = catalog.asViewCatalog();
+ View view = viewCatalog.loadView(NameIdentifier.of(schemaName,
viewName));
+ Assertions.assertEquals(1, view.representations().length);
+ SQLRepresentation rep = (SQLRepresentation)
view.representations()[0];
+ Assertions.assertTrue(
Review Comment:
`GravitinoPaimonCatalog` can return multiple SQL representations for a view
(FLINK + QUERY). This test assumes there is exactly one representation and
casts the first element without checking, which can cause failures when
additional representations are present.
##########
flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/integration/test/paimon/FlinkPaimonJdbcBackendIT.java:
##########
@@ -94,4 +94,10 @@ protected Map<String, String> getPaimonCatalogOptions() {
protected String getWarehouse() {
return warehouseDir.toString();
}
+
+ @Override
+ protected boolean supportViewOperation() {
+ // Paimon JDBC metastore backend does not support view operations.
+ return false;
+ }
Review Comment:
This class disables view tests for the Paimon JDBC backend
(`supportViewOperation()` returns false), but the PR description/issue
explicitly states view support (and tests) should be enabled for the Paimon
JDBC backend. Either the implementation should support views for JDBC, or the
PR description/issue linkage should be updated to reflect that JDBC is
intentionally excluded.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
