nastra commented on code in PR #9582:
URL: https://github.com/apache/iceberg/pull/9582#discussion_r1472630689
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -1362,6 +1362,120 @@ public void showCreateComplexView() {
assertThat(sql("SHOW CREATE TABLE %s",
viewName)).containsExactly(row(expected));
}
+ @Test
+ public void alterViewSetProperties() {
+ String viewName = "viewWithSetProperties";
+
+ sql("CREATE VIEW %s AS SELECT id FROM %s WHERE id <= 3", viewName,
tableName);
+
+ ViewCatalog viewCatalog = viewCatalog();
+ assertThat(viewCatalog.loadView(TableIdentifier.of(NAMESPACE,
viewName)).properties())
+ .doesNotContainKey("key1")
+ .doesNotContainKey("comment");
+
+ sql("ALTER VIEW %s SET TBLPROPERTIES ('key1' = 'val1', 'comment' = 'view
comment')", viewName);
+ assertThat(viewCatalog.loadView(TableIdentifier.of(NAMESPACE,
viewName)).properties())
+ .containsEntry("key1", "val1")
+ .containsEntry("comment", "view comment");
+
+ sql("ALTER VIEW %s SET TBLPROPERTIES ('key1' = 'new_val1')", viewName);
+ assertThat(viewCatalog.loadView(TableIdentifier.of(NAMESPACE,
viewName)).properties())
+ .containsEntry("key1", "new_val1")
+ .containsEntry("comment", "view comment");
+ }
+
+ @Test
+ public void alterViewSetReservedProperties() {
+ String viewName = "viewWithSetReservedProperties";
+
+ sql("CREATE VIEW %s AS SELECT id FROM %s WHERE id <= 3", viewName,
tableName);
+
+ assertThatThrownBy(() -> sql("ALTER VIEW %s SET TBLPROPERTIES ('provider'
= 'val1')", viewName))
+ .isInstanceOf(AnalysisException.class)
+ .hasMessageContaining(
+ "The feature is not supported: provider is a reserved table
property");
+
+ assertThatThrownBy(
+ () -> sql("ALTER VIEW %s SET TBLPROPERTIES ('location' =
'random_location')", viewName))
+ .isInstanceOf(AnalysisException.class)
+ .hasMessageContaining(
+ "The feature is not supported: location is a reserved table
property");
+
+ assertThatThrownBy(
+ () -> sql("ALTER VIEW %s SET TBLPROPERTIES ('format-version' =
'99')", viewName))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessageContaining(
+ "Cannot specify 'format-version' because it's a reserved view
property");
+
+ assertThatThrownBy(
+ () -> sql("ALTER VIEW %s SET TBLPROPERTIES ('queryColumnNames' =
'a,b,c')", viewName))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessageContaining(
+ "Cannot specify 'queryColumnNames' because it's a reserved view
property");
+ }
+
+ @Test
+ public void alterViewUnsetProperties() {
+ String viewName = "viewWithUnsetProperties";
+ sql("CREATE VIEW %s AS SELECT id FROM %s WHERE id <= 3", viewName,
tableName);
+
+ ViewCatalog viewCatalog = viewCatalog();
+ assertThat(viewCatalog.loadView(TableIdentifier.of(NAMESPACE,
viewName)).properties())
+ .doesNotContainKey("key1")
+ .doesNotContainKey("comment");
+
+ sql("ALTER VIEW %s SET TBLPROPERTIES ('key1' = 'val1', 'comment' = 'view
comment')", viewName);
+ assertThat(viewCatalog.loadView(TableIdentifier.of(NAMESPACE,
viewName)).properties())
+ .containsEntry("key1", "val1")
+ .containsEntry("comment", "view comment");
+
+ sql("ALTER VIEW %s UNSET TBLPROPERTIES ('key1')", viewName);
+ assertThat(viewCatalog.loadView(TableIdentifier.of(NAMESPACE,
viewName)).properties())
+ .doesNotContainKey("key1")
+ .containsEntry("comment", "view comment");
+ }
+
+ @Test
+ public void alterViewUnsetUnknownProperty() {
+ String viewName = "viewWithUnsetUnknownProp";
+ sql("CREATE VIEW %s AS SELECT id FROM %s WHERE id <= 3", viewName,
tableName);
+
+ assertThatThrownBy(() -> sql("ALTER VIEW %s UNSET TBLPROPERTIES
('unknown-key')", viewName))
+ .isInstanceOf(AnalysisException.class)
+ .hasMessageContaining("Attempted to unset non-existing property
'unknown-key'");
+
+ assertThatNoException()
+ .isThrownBy(
+ () -> sql("ALTER VIEW %s UNSET TBLPROPERTIES IF EXISTS
('unknown-key')", viewName));
+ }
+
+ @Test
+ public void alterViewUnsetReservedProperties() {
+ String viewName = "viewWithUnsetReservedProperties";
+
+ sql("CREATE VIEW %s AS SELECT id FROM %s WHERE id <= 3", viewName,
tableName);
+
+ assertThatThrownBy(() -> sql("ALTER VIEW %s UNSET TBLPROPERTIES
('provider')", viewName))
+ .isInstanceOf(AnalysisException.class)
+ .hasMessageContaining(
+ "The feature is not supported: provider is a reserved table
property");
+
+ assertThatThrownBy(() -> sql("ALTER VIEW %s UNSET TBLPROPERTIES
('location')", viewName))
+ .isInstanceOf(AnalysisException.class)
+ .hasMessageContaining(
+ "The feature is not supported: location is a reserved table
property");
+
+ sql("ALTER VIEW %s UNSET TBLPROPERTIES ('format-version')", viewName);
Review Comment:
@rdblue it feels like maybe we should also prevent removing reserverd
properties to be more explicit (even though they would be set by default when
the view is loaded)? But I also see that we don't do this when removing
reserved properties from a table
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
