You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "kotwal-itpro (via GitHub)" <gi...@apache.org> on 2026/10/07 18:24:43 UTC
[PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
kotwal-itpro opened a new pull request, #8795:
URL: https://github.com/apache/hop/pull/8795
Fixes #8794.
On the native Spark engine, Iceberg tables in PATH mode were never written at their path. They ended up under `${java.io.tmpdir}/hop-iceberg-path-catalog-warehouse/`, in a sub-folder named after the whole table URI. Reads looked in the same place, so a write followed by a read worked, which is why the existing tests didn't notice.
## Cause
A PATH table was addressed as `` hop_iceberg.`file:///data/orders` `` in one Hadoop catalog whose warehouse is in the temp folder. Iceberg's `SparkCatalog` only treats an identifier as a location when it is a `PathIdentifier`, which Spark creates for `spark.read.format("iceberg").load(path)`, not for a quoted name in SQL or `writeTo()`. So the Hadoop catalog saw a table called `file:///data/orders` in the default namespace and stored it at `<warehouse>/file:/data/orders`.
## Fix
Each folder that holds PATH tables gets its own Hadoop catalog, `hop_iceberg_<hash of the folder>`, with that folder as its warehouse, and the table is addressed by its folder name: `` hop_iceberg_1a2b3c4d5e6f.`orders` ``. A Hadoop catalog stores a table of the default namespace at `<warehouse>/<name>`, which is exactly the table path. The catalog is registered on the session the first time a table in that folder is used.
Because every PATH caller goes through `SparkLakeTableSupport.icebergPathSqlIdentifier()` and `resolveLakeTargetSqlId()`, this covers reads (including time travel), writes in every save mode, merges, and the maintenance procedures (`CALL hop_iceberg_<hash>.system.expire_snapshots(table => '`orders`')` etc.).
I looked at reading with `load(path)` instead, but writes have no path equivalent that can also create a table, and merge and maintenance need a SQL identifier anyway, so a catalog per folder keeps one code path for everything.
## Upgrading
Tables written with 2.19 are still in the temp folder. The lakehouse guide now has a note on copying such a table to its path. They aren't moved automatically: the temp folder may already have been cleaned, and silently moving data around felt wrong. Happy to add an automatic fallback (read from the old location when the table isn't at the path) if you think it's worth it.
## Testing
- `SparkLakeTableIcebergPathTest` now also checks that the table's metadata is at the table path and that nothing is written to the old warehouse. That assertion fails on main and passes with this change.
- New unit tests for the folder/table mapping: the parent folder is the warehouse, tables in one folder share a catalog, other folders get another, and a path without a parent folder is rejected.
- All 195 tests of `hop-engines-spark` pass.
- Built the full client (`-Pfull`) and ran `integration-tests/iceberg` (round trip, overwrite and time travel, merge) with `hop-run`: all pass, the tables are at `/tmp/hop-it-iceberg/...`, and `hop-iceberg-path-catalog-warehouse` is no longer created.
The docs that described `hop_iceberg.uri` (lakehouse guide, output, merge, maintenance, catalog pages) are updated.
- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on code in PR #8795:
URL: https://github.com/apache/hop/pull/8795#discussion_r4229727129
##########
plugins/engines/spark/src/main/java/org/apache/hop/spark/table/SparkLakeTableSupport.java:
##########
@@ -107,15 +108,63 @@ public static String toTableLocationUri(String path) {
}
/**
- * Spark SQL multi-part identifier for an Iceberg path-based table under the hop path catalog:
- * {@code hop_iceberg.`file:///path/to/table`}.
+ * An Iceberg table at a path, seen through the Hadoop catalog whose warehouse is the table's
+ * parent folder. A Hadoop catalog keeps a table of the default namespace in {@code
+ * <warehouse>/<table>}, so the table is read and written exactly at its path.
+ *
+ * <p>A quoted path in a single shared catalog, like {@code hop_iceberg.`file:///data/orders`},
+ * doesn't work: Iceberg only treats an identifier as a location when Spark creates it from {@code
+ * DataFrameReader.load(path)}. Otherwise the Hadoop catalog takes the whole URI as a table name
+ * and stores the table under its own warehouse.
*/
- @SuppressWarnings("javabugs:S2259") // toTableLocationUri() never returns null for a resolved path
+ public record IcebergPathTable(String catalogName, String warehouse, String tableName) {
+
+ /** Spark SQL identifier of the table, e.g. {@code hop_iceberg_1a2b3c4d5e6f.`orders`}. */
+ public String sqlIdentifier() {
+ return catalogName + "." + procedureTableRef();
+ }
+
+ /** The table as named in an Iceberg procedure call of {@link #catalogName()}. */
+ public String procedureTableRef() {
+ return "`" + tableName.replace("`", "``") + "`";
+ }
+ }
+
+ /**
+ * The path catalog and table name for an Iceberg table at {@code path}. Tables in the same folder
+ * share a catalog.
+ */
+ public static IcebergPathTable icebergPathTable(String path) {
+ String uri = StringUtils.removeEnd(toTableLocationUri(path), "/");
+ int slash = uri == null ? -1 : uri.lastIndexOf('/');
+ String parent = slash < 0 ? "" : uri.substring(0, slash);
+ String name = slash < 0 ? "" : uri.substring(slash + 1);
+ if (name.isEmpty() || parent.isEmpty() || parent.endsWith(":") || parent.endsWith("/")) {
Review Comment:
**[bug]** The “needs a parent folder” check treats every parent that ends with `:` or `/` as invalid. That rejects paths which do have a parent directory:
- A table in a Windows drive root (`C:\orders`, `C:/orders`, or `file:///C:/orders`) becomes parent `file:///C:` and throws `IllegalArgumentException`. `C:\data\orders` still works, because that parent is `file:///C:/data`.
- A URI with an empty segment before the table name (`s3a://bucket/lake//orders`, `file:///data//orders`) becomes a parent that ends with `/` and throws, even though it is the same location as the single-slash path.
`file:///t` and `s3a://bucket` are correctly rejected, and those cases are tested. `endsWith(":")` / `endsWith("/")` are wider than that rule, and the failure is an unchecked `IllegalArgumentException` rather than a `HopException` naming the transform.
**Suggestion:** Normalize the URI before the split (collapse empty segments, resolve `.` and `..`). Treat a Windows drive root (`file:///C:`) as a valid warehouse. Only reject a scheme-only parent (`file:`, `file://`, `s3a:`, `s3a:/`). Add tests for `file:///C:/orders` and `s3a://bucket/lake//orders`.
##########
plugins/engines/spark/src/main/java/org/apache/hop/spark/table/SparkLakeTableSupport.java:
##########
@@ -107,15 +108,63 @@ public static String toTableLocationUri(String path) {
}
/**
- * Spark SQL multi-part identifier for an Iceberg path-based table under the hop path catalog:
- * {@code hop_iceberg.`file:///path/to/table`}.
+ * An Iceberg table at a path, seen through the Hadoop catalog whose warehouse is the table's
+ * parent folder. A Hadoop catalog keeps a table of the default namespace in {@code
+ * <warehouse>/<table>}, so the table is read and written exactly at its path.
+ *
+ * <p>A quoted path in a single shared catalog, like {@code hop_iceberg.`file:///data/orders`},
+ * doesn't work: Iceberg only treats an identifier as a location when Spark creates it from {@code
+ * DataFrameReader.load(path)}. Otherwise the Hadoop catalog takes the whole URI as a table name
+ * and stores the table under its own warehouse.
*/
- @SuppressWarnings("javabugs:S2259") // toTableLocationUri() never returns null for a resolved path
+ public record IcebergPathTable(String catalogName, String warehouse, String tableName) {
+
+ /** Spark SQL identifier of the table, e.g. {@code hop_iceberg_1a2b3c4d5e6f.`orders`}. */
+ public String sqlIdentifier() {
+ return catalogName + "." + procedureTableRef();
+ }
+
+ /** The table as named in an Iceberg procedure call of {@link #catalogName()}. */
+ public String procedureTableRef() {
+ return "`" + tableName.replace("`", "``") + "`";
+ }
+ }
+
+ /**
+ * The path catalog and table name for an Iceberg table at {@code path}. Tables in the same folder
+ * share a catalog.
+ */
+ public static IcebergPathTable icebergPathTable(String path) {
+ String uri = StringUtils.removeEnd(toTableLocationUri(path), "/");
Review Comment:
**[suggestion]** The catalog name is a hash of the parent string after removing only one trailing slash, and scheme URIs are not canonicalized (`toTableLocationUri` returns them unchanged). `File.toURI()` yields `file:/data/lake` while `Path.toUri()` yields `file:///data/lake`. Those become two `hop_iceberg_<hash>` catalogs whose warehouses are the same directory. `SparkCatalog` wraps the Hadoop catalog in `CachingCatalog` by default, so a write through one spelling does not refresh the other catalog’s cached snapshot.
**Suggestion:** Canonicalize the location before hashing and before setting `spark.sql.catalog.<name>.warehouse`: lowercase the scheme, rewrite `file` URIs to `file:///…`, collapse empty segments, and resolve `.` / `..`. Then one folder is one catalog.
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "bamaer (via GitHub)" <gi...@apache.org>.
bamaer commented on PR #8795:
URL: https://github.com/apache/hop/pull/8795#issuecomment-6053885190
Built the branch and ran the `hop-engines-spark` tests: all pass. The new assertion in `SparkLakeTableIcebergPathTest` fails against main's `SparkLakeTableSupport`, so it covers the bug.
I also checked Iceberg PATH maintenance, which the tests don't cover: in a fresh session, `REWRITE_MANIFESTS` and `EXPIRE_SNAPSHOTS` hit the table at its path. Looks good to me.
Optional leftovers: the old Javadoc block above `ensureIcebergPathCatalog(SparkSession, String)`, the `hop_iceberg` mention in the `readIcebergPath` error message, and `lakehouse.adoc:295`.
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "kotwal-itpro (via GitHub)" <gi...@apache.org>.
kotwal-itpro commented on PR #8795:
URL: https://github.com/apache/hop/pull/8795#issuecomment-6076144830
Rebased onto main now that #8802 is in (it moved `SparkLakeTableSupport`'s neighbours into the lakehouse plugin). The conflict was the one I'd rehearsed; no change to the fix itself. I also corrected the `ICEBERG_PATH_CATALOG_NAME` Javadoc, which still showed the old `hop_iceberg.\`file:///…\`` form. Spark tests: 182 pass.
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on code in PR #8795:
URL: https://github.com/apache/hop/pull/8795#discussion_r4233072979
##########
plugins/engines/spark/src/main/java/org/apache/hop/spark/table/SparkLakeTableSupport.java:
##########
@@ -107,15 +110,140 @@ public static String toTableLocationUri(String path) {
}
/**
- * Spark SQL multi-part identifier for an Iceberg path-based table under the hop path catalog:
- * {@code hop_iceberg.`file:///path/to/table`}.
+ * An Iceberg table at a path, seen through the Hadoop catalog whose warehouse is the table's
+ * parent folder. A Hadoop catalog keeps a table of the default namespace in {@code
+ * <warehouse>/<table>}, so the table is read and written exactly at its path.
+ *
+ * <p>A quoted path in a single shared catalog, like {@code hop_iceberg.`file:///data/orders`},
+ * doesn't work: Iceberg only treats an identifier as a location when Spark creates it from {@code
+ * DataFrameReader.load(path)}. Otherwise the Hadoop catalog takes the whole URI as a table name
+ * and stores the table under its own warehouse.
+ */
+ public record IcebergPathTable(String catalogName, String warehouse, String tableName) {
+
+ /** Spark SQL identifier of the table, e.g. {@code hop_iceberg_1a2b3c4d5e6f.`orders`}. */
+ public String sqlIdentifier() {
+ return catalogName + "." + procedureTableRef();
+ }
+
+ /** The table as named in an Iceberg procedure call of {@link #catalogName()}. */
+ public String procedureTableRef() {
+ return "`" + tableName.replace("`", "``") + "`";
+ }
+ }
+
+ /**
+ * The path catalog and table name for an Iceberg table at {@code path}. Tables in the same folder
+ * share a catalog.
+ */
+ public static IcebergPathTable icebergPathTable(String path) {
+ String uri = canonicalLocation(toTableLocationUri(path));
+ int slash = uri == null ? -1 : uri.lastIndexOf('/');
+ String parent = slash < 0 ? "" : uri.substring(0, slash);
+ String name = slash < 0 ? "" : uri.substring(slash + 1);
+ if (name.isEmpty() || isSchemeOnly(parent)) {
+ throw new IllegalArgumentException(
+ "Iceberg table path '" + path + "' needs a parent folder, e.g. file:///data/orders");
+ }
+ return new IcebergPathTable(
+ SparkLakeFormats.ICEBERG_PATH_CATALOG_NAME + "_" + shortHash(parent), parent, name);
+ }
+
+ /**
+ * Like {@link #icebergPathTable(String)}, but reports an invalid path as a {@link HopException}
+ * that names the transform.
+ */
+ public static IcebergPathTable icebergPathTable(String path, String transformName)
+ throws HopException {
+ try {
+ return icebergPathTable(path);
+ } catch (IllegalArgumentException e) {
+ throw new HopException("Spark Lake Table '" + transformName + "': " + e.getMessage(), e);
+ }
+ }
+
+ private static final Pattern URI_SCHEME = Pattern.compile("^([A-Za-z][A-Za-z0-9+.-]*):(.*)$");
+
+ /**
+ * One spelling per location, so that one folder is always one catalog: the scheme is lowercased,
+ * {@code file:/x}, {@code file://localhost/x} and {@code file:///x} all become {@code file:///x},
+ * empty path segments are collapsed, and {@code .} and {@code ..} are resolved. A trailing slash
+ * is removed.
+ */
+ static String canonicalLocation(String uri) {
+ if (StringUtils.isEmpty(uri)) {
+ return uri;
+ }
+ Matcher matcher = URI_SCHEME.matcher(uri.trim());
+ if (!matcher.matches() || matcher.group(1).length() == 1) {
+ // No scheme (or a Windows drive letter): leave it as it is.
+ return StringUtils.removeEnd(uri.trim(), "/");
+ }
+ String scheme = matcher.group(1).toLowerCase(java.util.Locale.ROOT);
+ String rest = matcher.group(2);
+ String authority = "";
+ String pathPart = rest;
+ if (rest.startsWith("//")) {
+ int end = rest.indexOf('/', 2);
+ authority = end < 0 ? rest.substring(2) : rest.substring(2, end);
+ pathPart = end < 0 ? "" : rest.substring(end);
+ }
+ if ("file".equals(scheme) && "localhost".equalsIgnoreCase(authority)) {
+ authority = "";
+ }
+ java.util.Deque<String> segments = new java.util.ArrayDeque<>();
+ for (String segment : pathPart.split("/")) {
+ if (segment.isEmpty() || ".".equals(segment)) {
+ continue;
+ }
+ if ("..".equals(segment)) {
+ segments.pollLast();
+ } else {
+ segments.addLast(segment);
Review Comment:
**[suggestion]** A Windows drive letter is still two folders. `Path.toUri()` produces `file:///C:/data/orders`, while a pasted `file:///c:/data/orders` keeps the lowercase `c:` segment, so the same directory gets two `hop_iceberg_<hash>` catalogs. When the scheme is `file`, uppercase a single-letter drive segment (`c:` → `C:`) before it is hashed.
##########
plugins/engines/spark/src/main/java/org/apache/hop/spark/table/SparkLakeTableSupport.java:
##########
@@ -107,15 +110,140 @@ public static String toTableLocationUri(String path) {
}
/**
- * Spark SQL multi-part identifier for an Iceberg path-based table under the hop path catalog:
- * {@code hop_iceberg.`file:///path/to/table`}.
+ * An Iceberg table at a path, seen through the Hadoop catalog whose warehouse is the table's
+ * parent folder. A Hadoop catalog keeps a table of the default namespace in {@code
+ * <warehouse>/<table>}, so the table is read and written exactly at its path.
+ *
+ * <p>A quoted path in a single shared catalog, like {@code hop_iceberg.`file:///data/orders`},
+ * doesn't work: Iceberg only treats an identifier as a location when Spark creates it from {@code
+ * DataFrameReader.load(path)}. Otherwise the Hadoop catalog takes the whole URI as a table name
+ * and stores the table under its own warehouse.
+ */
+ public record IcebergPathTable(String catalogName, String warehouse, String tableName) {
+
+ /** Spark SQL identifier of the table, e.g. {@code hop_iceberg_1a2b3c4d5e6f.`orders`}. */
+ public String sqlIdentifier() {
+ return catalogName + "." + procedureTableRef();
+ }
+
+ /** The table as named in an Iceberg procedure call of {@link #catalogName()}. */
+ public String procedureTableRef() {
+ return "`" + tableName.replace("`", "``") + "`";
+ }
+ }
+
+ /**
+ * The path catalog and table name for an Iceberg table at {@code path}. Tables in the same folder
+ * share a catalog.
+ */
+ public static IcebergPathTable icebergPathTable(String path) {
+ String uri = canonicalLocation(toTableLocationUri(path));
+ int slash = uri == null ? -1 : uri.lastIndexOf('/');
+ String parent = slash < 0 ? "" : uri.substring(0, slash);
+ String name = slash < 0 ? "" : uri.substring(slash + 1);
+ if (name.isEmpty() || isSchemeOnly(parent)) {
+ throw new IllegalArgumentException(
+ "Iceberg table path '" + path + "' needs a parent folder, e.g. file:///data/orders");
+ }
+ return new IcebergPathTable(
+ SparkLakeFormats.ICEBERG_PATH_CATALOG_NAME + "_" + shortHash(parent), parent, name);
+ }
+
+ /**
+ * Like {@link #icebergPathTable(String)}, but reports an invalid path as a {@link HopException}
+ * that names the transform.
+ */
+ public static IcebergPathTable icebergPathTable(String path, String transformName)
+ throws HopException {
+ try {
+ return icebergPathTable(path);
+ } catch (IllegalArgumentException e) {
+ throw new HopException("Spark Lake Table '" + transformName + "': " + e.getMessage(), e);
+ }
+ }
+
+ private static final Pattern URI_SCHEME = Pattern.compile("^([A-Za-z][A-Za-z0-9+.-]*):(.*)$");
+
+ /**
+ * One spelling per location, so that one folder is always one catalog: the scheme is lowercased,
+ * {@code file:/x}, {@code file://localhost/x} and {@code file:///x} all become {@code file:///x},
+ * empty path segments are collapsed, and {@code .} and {@code ..} are resolved. A trailing slash
+ * is removed.
+ */
+ static String canonicalLocation(String uri) {
+ if (StringUtils.isEmpty(uri)) {
+ return uri;
+ }
+ Matcher matcher = URI_SCHEME.matcher(uri.trim());
+ if (!matcher.matches() || matcher.group(1).length() == 1) {
+ // No scheme (or a Windows drive letter): leave it as it is.
+ return StringUtils.removeEnd(uri.trim(), "/");
+ }
+ String scheme = matcher.group(1).toLowerCase(java.util.Locale.ROOT);
+ String rest = matcher.group(2);
+ String authority = "";
+ String pathPart = rest;
+ if (rest.startsWith("//")) {
+ int end = rest.indexOf('/', 2);
+ authority = end < 0 ? rest.substring(2) : rest.substring(2, end);
+ pathPart = end < 0 ? "" : rest.substring(end);
+ }
+ if ("file".equals(scheme) && "localhost".equalsIgnoreCase(authority)) {
+ authority = "";
+ }
+ java.util.Deque<String> segments = new java.util.ArrayDeque<>();
+ for (String segment : pathPart.split("/")) {
+ if (segment.isEmpty() || ".".equals(segment)) {
+ continue;
+ }
+ if ("..".equals(segment)) {
+ segments.pollLast();
+ } else {
+ segments.addLast(segment);
+ }
+ }
+ String normalizedPath = segments.isEmpty() ? "" : "/" + String.join("/", segments);
+ return scheme + "://" + authority + normalizedPath;
Review Comment:
**[bug]** `canonicalLocation` is the string `icebergPathTable` uses as the Hadoop warehouse and the table name, and it never percent-decodes. Scheme-less paths go through `toTableLocationUri` → `Path.toUri()`, so `/data/my lake/orders` becomes `file:///data/my%20lake/orders` (a table folder `my orders` becomes `my%20orders`). Hadoop's `Path` does not treat `%` as an escape; `new Path("file:///data/my%20lake")` is the directory `/data/my%20lake` (`file:///data/my%2520lake`), not `/data/my lake`. The same split of `file:///data/my lake/orders` and `file:///data/my%20lake/orders` also hashes to two catalogs. `SparkLakeTableIcebergPathTest` uses a temp path with no such characters, so it stays green.
**Suggestion:** Percent-decode each path segment and the authority before hashing and before setting `spark.sql.catalog.<name>.warehouse`. Decode with `URI.getPath()` (not `URLDecoder`, so `+` stays `+`); leave a segment as-is when it has a raw space or a broken `%` escape. `/data/my lake/my orders`, `file:///data/my%20lake/my%20orders`, and `file:///data/my lake/my orders` should all yield warehouse `file:///data/my lake` and table name `my orders`. Extend `oneFolderIsOneCatalogWhateverTheSpelling` with that case.
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "kotwal-itpro (via GitHub)" <gi...@apache.org>.
kotwal-itpro commented on PR #8795:
URL: https://github.com/apache/hop/pull/8795#issuecomment-6053969755
Thanks @bamaer, and for checking maintenance on a real path too, that's the part the tests don't cover.
I pushed a8b0877 with the leftovers: the Javadoc is back on the right `ensureIcebergPathCatalog` method (the old one now says it's for two-part TABLE identifiers), the class and resolver Javadoc describe `hop_iceberg_<hash>.\`table\``, the PATH read and write errors no longer mention `hop_iceberg`, and the troubleshooting row at `lakehouse.adoc:295` is updated. The `hop_iceberg` hint in the TABLE-mode error stays, since two-part TABLE identifiers still use that catalog. `hop-engines-spark` tests still pass (195).
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters merged PR #8795:
URL: https://github.com/apache/hop/pull/8795
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "kotwal-itpro (via GitHub)" <gi...@apache.org>.
kotwal-itpro commented on PR #8795:
URL: https://github.com/apache/hop/pull/8795#issuecomment-6086067170
Thanks @mattcasters, both fixed in db477871f2 (rebased onto main):
**Parent check.** The path is now canonicalized before it's split, and only a parent that is a scheme alone (`file:`, `file://`, `s3a:/`) is rejected. A table in a Windows drive root works (`file:///C:/orders` has warehouse `file:///C:`), and so does `s3a://bucket/lake//orders`, which now resolves to the same table as the single-slash path. `file:///t` and `s3a://bucket` are still rejected. On the read, write and maintenance paths an invalid path now comes back as a `HopException` that names the transform, e.g. `Spark Lake Table 'write orders': Iceberg table path 'file:///t' needs a parent folder ...`.
**One folder, one catalog.** `canonicalLocation` lowercases the scheme, turns `file:/x`, `file://localhost/x` and `file:///x` into `file:///x`, collapses empty segments and resolves `.` and `..`. The canonical parent is what gets hashed into `hop_iceberg_<hash>` and what's set as the catalog's warehouse, so the `CachingCatalog` problem you described can't happen through different spellings. `toTableLocationUri` itself is unchanged, since Delta and the other formats use it.
New tests in `SparkLakeTableSupportTest`: the drive root, `lake//orders` plus `./` and `../`, five spellings of the same folder mapping to one catalog, and the transform name in the error. Spark tests: 186 (was 182), RAT clean.
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Issue #8794 : Write Iceberg PATH tables at their path on the Spark engine (hop)
Posted by "kotwal-itpro (via GitHub)" <gi...@apache.org>.
kotwal-itpro commented on PR #8795:
URL: https://github.com/apache/hop/pull/8795#issuecomment-6087616761
Thanks @mattcasters, both fixed in 664246451f (rebased onto main):
**Percent-decoding.** `canonicalLocation` now decodes `%XX` escapes in each path segment and in the authority as UTF-8, with `+` left as `+` and a segment with a broken escape kept as it is. `/data/my lake/my orders`, `file:///data/my%20lake/my%20orders` and `file:///data/my lake/my orders` all give warehouse `file:///data/my lake` and table `my orders`. I also added a Spark round trip into `my lake/my orders` to `SparkLakeTableIcebergPathTest`: it checks the metadata lands in that folder and no `my%20lake` folder appears, and it failed before the change, as you predicted.
**Drive letter.** For `file` URIs a single-letter drive segment is uppercased, so `file:///c:/data/orders` and `file:///C:/data/orders` are one catalog with warehouse `file:///C:/data`.
Spark tests: 190, RAT clean.
--
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@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org