You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "tomtongue (via GitHub)" <gi...@apache.org> on 2025/09/29 11:26:41 UTC
[PR] Core: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
tomtongue opened a new pull request, #14212:
URL: https://github.com/apache/iceberg/pull/14212
The context of this PR is https://github.com/apache/iceberg/pull/12017#issuecomment-3238253488.
## Changes
This PR tries to deprecate the `write.metadata.path` property which was added in [PR#12017](https://github.com/apache/iceberg/pull/12017), and use `location` instead to for the view location configuration.
## Background
That PR#12017 added `write.metadata.path` to customize the View location. However, the [Iceberg View Spec](https://iceberg.apache.org/view-spec/#view-metadata) defines `location` as its view location. Using `write.metadata.path` is confusing because it specifies two view locations in its view properties, along with `location`.
## Behavior difference between `location` and `write.metadata.path`
Both properties can configure the view location, however,
* if `write.metadata.path` is specified, the view metadata is put in the path defined `write.metadata.path`
* (e.g.) When `'write.metadata.path'='custom-location/path/to` is specified, the view metadata is put in `custom-location/path/to`
* if `location` is specified, the view metadata is put in the path defined `location` with `/metadata` path
* (e.g.) When `'location='custom-location/path/to` is specified, the view metadata is put in `custom-location/path/to/metadata`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412545300
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
@flyrain @huaxingao Thanks, I believe it's good to support `ALTER TABLE SET TBLPROPERTIES` (I don't think adding a new clause like `LOCATION` into Spark is a good option because only Iceberg needs the location for its view for now) Let me try to make `location` configurable as a non-reserved property, and consider the way to handle `location`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412114925
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
That's understandable. Location is important for a table, which has multiple metadata files and data files, but view doesn't really require a file, Iceberg asks for one metadata file, which also isn't completely necessary when catalog can take care of it. I believe it's fine to not consider view `location` as a reserved property.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2400210802
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2114,6 +2114,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
Review Comment:
Is the reason for not backporting to v3.4 that's deprecated #14099?
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
I think there is a minor difference between `write.metadata.path` and `location` in UNSET/SET TBLPROPERTIES statement.
`write.metadata.path` isn't a reserved property, so it can be modified or unset. In contrast, `location` is a reserved property, so it can no longer be changed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2402529390
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
If we enable to change the location after view creation, I believe at least there is one way to do that like: Keep the `write.metadata.path` property, and make some changes to update `location` value with the `write.metadata.path` property (this can avoid confusing the path maybe compareing to the current situation)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2389473826
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,26 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('%s'='%s') AS SELECT * FROM %s",
+ viewName, "location", customMetadataLocation, tableName);
Review Comment:
nit: `"location"` can be inlined.
##########
docs/docs/view-configuration.md:
##########
@@ -28,7 +28,7 @@ Iceberg views support properties to configure view behavior. Below is an overvie
| Property | Default | Description |
|----------------------------------|---------------------------|------------------------------------------------------------------------------------|
| write.metadata.compression-codec | gzip | Metadata compression codec: `none` or `gzip` |
-| write.metadata.path | view location + /metadata | Base location for metadata files |
+| location | view location + /metadata | Base location for metadata files |
Review Comment:
The "location" isn't actually a property, right?
Readers of the docs might mistakenly think they can set a custom location with `ViewBuilder#withProperty("location", xxx)`. Instead, they should use `ViewBuilder#withLocation`.
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,26 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('%s'='%s') AS SELECT * FROM %s",
+ viewName, "location", customMetadataLocation, tableName);
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+
+ assertThat(sql("DESCRIBE EXTENDED %s", viewName))
+ .contains(
+ row(
+ "View Properties",
+ String.format(
+ "['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
+ customMetadataLocation),
+ ""));
Review Comment:
We could use `SHOW TBLPROPERTIES %s (location)` if `format-version` & `provider` aren't important in this test:
```java
assertThat(sql("SHOW TBLPROPERTIES %s (location)", viewName))
.contains(row("location", customMetadataLocation));
```
##########
core/src/main/java/org/apache/iceberg/view/ViewProperties.java:
##########
@@ -25,7 +25,12 @@ public class ViewProperties {
public static final String METADATA_COMPRESSION = "write.metadata.compression-codec";
public static final String METADATA_COMPRESSION_DEFAULT = "gzip";
- public static final String WRITE_METADATA_LOCATION = "write.metadata.path";
+
+ /**
+ * @deprecated will be removed in 2.0.0, use location instead.
Review Comment:
nit: We could replace `location` with `{@link ViewBuilder#withLocation}`.
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,26 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('%s'='%s') AS SELECT * FROM %s",
+ viewName, "location", customMetadataLocation, tableName);
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
Review Comment:
`location` variable is unused. Perhaps, `assertThat(location).isEqualTo(customMetadataLocation)` is missing?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2400591787
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2114,6 +2114,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
Review Comment:
Yes, I don't think it's necessary to backport to v3.4.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412093761
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
I did a quick test, actually Spark only has ALTER TABLE … SET LOCATION; It doesn't have ALTER VIEW … SET LOCATION
```
sql("ALTER VIEW test_view SET TBLPROPERTIES ('location' = '/updated/path')")
```
issued
```
[UNSUPPORTED_FEATURE.SET_TABLE_PROPERTY] The feature is not supported: location is a reserved table property, please use the LOCATION clause to specify it. SQLSTATE: 0A000
== SQL (line 1, position 1) ==
ALTER VIEW test_view SET TBLPROPERTIES ('location' = '/updated/path')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
but
```
sql("ALTER VIEW test_view SET LOCATION '/new/table/location'")
```
doesn't work, it gave
```
[PARSE_SYNTAX_ERROR] Syntax error at or near 'LOCATION'. SQLSTATE: 42601 (line 1, pos 25)
== SQL ==
ALTER VIEW test_view SET LOCATION '/new/table/location'
-------------------------^^^
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2401880785
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
Thanks for adding that info, yes when you run `ALTER TABLE SET TBLPROPERTIES` or `UNSET TBLPROPERTIES`, they failed with the parce exception with the error showing `location is a reseved property` like Spark.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2391437288
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,26 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('%s'='%s') AS SELECT * FROM %s",
+ viewName, "location", customMetadataLocation, tableName);
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
Review Comment:
Thanks for pointing this, I missed that test, let me add it to Spark 4.0 and 3.5.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412114925
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
That's understandable. Location is important for a table, which has multiple metadata files and data files, but view doesn't really require a file, Iceberg requires a view to have one metadata file, which also isn't completely necessary when catalog can take care of it. I believe it's fine to not consider view `location` as a reserved property.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#issuecomment-3378743570
Thanks @tomtongue for the change. Thanks @huaxingao @ebyhr for the review. I will merge it by EOD if there is no objection.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2402529390
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
If we enable to change the location after view creation, I believe at least there is one way to do that like: Keep the `write.metadata.path` property, and make some changes to update `location` value with the `write.metadata.path` property (this can avoid confusing the path maybe compareing to the current situation, but need to confirm its possibility)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2402529390
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
If we enable to change the location after view creation, I believe at least there is one way to enable that location change like: Keep the `write.metadata.path` property, and make some changes to update `location` value with the `write.metadata.path` property (this can avoid confusing the path maybe compareing to the current situation)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#issuecomment-3379550623
@flyrain Thanks so much for raising this issue, discussion and the review!! @ebyhr @huaxingao Thanks so much for the review and discussion!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412029812
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
I agree to merge this pr, but I prefer to stay consistent with Spark and treat `location` a reserved property (not a TBLPROPERTY). If users need to change it, they should use catalog‑managed CREATE/ALTER VIEW … SET LOCATION, not TBLPROPERTIES.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2391423596
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,26 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('%s'='%s') AS SELECT * FROM %s",
+ viewName, "location", customMetadataLocation, tableName);
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+
+ assertThat(sql("DESCRIBE EXTENDED %s", viewName))
+ .contains(
+ row(
+ "View Properties",
+ String.format(
+ "['format-version' = '1', 'location' = '%s', 'provider' = 'iceberg']",
+ customMetadataLocation),
+ ""));
Review Comment:
Agree, let me update this test along with Spark 3.5 test
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#issuecomment-3357047690
@flyrain could you check the changes? (@ebyhr if you have any additional comments, please add the comments)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2409195459
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
I'm fine with this behavior change, as it's still a deprecation rather than a removal.
We can review it again after receiving feedback from users.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412114925
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
That's understandable. Location is important for a table, which has multiple metadata files and data files, but view doesn't really require a file, Iceberg requires a view has one metadata file, which also isn't completely necessary when catalog can take care of it. I believe it's fine to not consider view `location` as a reserved property.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#issuecomment-3348071466
+1 on consolidating to a single property.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#issuecomment-3346436695
@flyrain Could you review this when you have time? (@nastra @amogh-jahagirdar Could you please share your review comments if you have any comments on this change?)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#issuecomment-3349993913
@ebyhr Thanks for the review, let me apply them.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #14212:
URL: https://github.com/apache/iceberg/pull/14212
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2411061090
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
Agreed, thanks so much for pointing this out.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2412046992
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
Does Spark have a command like this? Or do you suggest to create one?
```
ALTER VIEW … SET LOCATION
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2411944511
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
Thanks for catching this, @ebyhr! I think we should allow users to set the `location`, view is different from table, the location update is low risk, but even for tables, we got REST API for location update. I think it makes sense to enable that in another PR.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2391703147
##########
docs/docs/view-configuration.md:
##########
@@ -28,7 +28,7 @@ Iceberg views support properties to configure view behavior. Below is an overvie
| Property | Default | Description |
|----------------------------------|---------------------------|------------------------------------------------------------------------------------|
| write.metadata.compression-codec | gzip | Metadata compression codec: `none` or `gzip` |
-| write.metadata.path | view location + /metadata | Base location for metadata files |
+| location | view location + /metadata | Base location for metadata files |
Review Comment:
As you're mentioning, actually it's not a property. So, I think this property should be removed, and let me try to add the explanation about specifying the view location in the Spark DDL part.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2401880785
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
Thanks for adding that info, yes when you run `ALTER TABLE SET TBLPROPERTIES` or `UNSET TBLPROPERTIES`, they failed with the parce exception with the error showing `location is a reseved property` like Spark. So as you're mentioning, this would be another limitation that users cannot change the view metadata location after they create a view. I believe there are not so many cases like changing view metadata location after creating the view, but is there any concern on this limitation? @ebyhr
And if you have any concern on the limitation, could you share it? @flyrain @huaxingao
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core, Spark: Deprecate `write.metadata.path` and use `location` to customize view location [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #14212:
URL: https://github.com/apache/iceberg/pull/14212#discussion_r2402529390
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestViews.java:
##########
@@ -2113,6 +2113,22 @@ public void createViewWithCustomMetadataLocation() {
""));
}
+ @TestTemplate
+ public void createViewWithCustomMetadataLocationWithLocation() {
+ String viewName = viewName("v");
+ String customMetadataLocation =
+ Paths.get(temp.toUri().toString(), "custom-metadata-location").toString();
+ sql(
+ "CREATE VIEW %s TBLPROPERTIES ('location'='%s') AS SELECT * FROM %s",
+ viewName, customMetadataLocation, tableName);
+
+ assertThat(sql("SHOW TBLPROPERTIES %s", viewName))
+ .contains(row("location", customMetadataLocation));
+
+ String location = viewCatalog().loadView(TableIdentifier.of(NAMESPACE, viewName)).location();
+ assertThat(location).isEqualTo(customMetadataLocation);
+ }
+
Review Comment:
If we enable to change the location after view creation, I believe at least there is one way like: Keep the `write.metadata.path` property, and make some changes to update `location` value with the `write.metadata.path` property (this can avoid confusing the path maybe compareing to the current situation),
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org