You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org> on 2026/09/01 20:58:03 UTC
[PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Aggarwal-Raghav opened a new pull request, #6749:
URL: https://github.com/apache/hive/pull/6749
### What changes were proposed in this pull request?
[HIVE-29877](https://issues.apache.org/jira/browse/HIVE-29877)
Previously, the router expected URLs to match an exact length and crashed if an Iceberg client sent a `{prefix}` (like my_catalog). This PR introduces a mathematical offset that dynamically calculates the length of any prefix, allowing the server to gracefully accept and route multi-segment prefixes.
### Why are the changes needed?
- To comply with the OpenAPI specification and HMS Rest catalog exposes `{prefix}` in `../iceberg/v1/config`
- Databricks Unity IRC follows the following standard where `catalogs/<uc_catalog_name>` can be prefix.
```
https://<workspace-instance>/api/2.1/unity-catalog/iceberg-rest/v1/catalogs/<uc_catalog_name>/namespaces/<uc_schema_name>/tables/<uc_table_name>
```
- Spark client can inject prefix using `--conf spark.sql.catalog.local.prefix=my_test_prefix`
### Does this PR introduce _any_ user-facing change?
Yes (Positive Impact): Users can now successfully connect strict Iceberg REST clients that inject custom multi-segment prefixes into the HTTP paths without hitting HTTP 400 errors.
### How was this patch tested?
Write Junit test and spark docker image testing (Screenshot attached below)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4030370612
##########
standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCatalogAdapterRoutes.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.rest;
+
+import java.util.Map;
+import org.apache.iceberg.util.Pair;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.apache.iceberg.rest.HTTPRequest.HTTPMethod;
+
+public class TestHMSCatalogAdapterRoutes {
+
+ @Test
+ public void testPrefixRouting() {
Review Comment:
ACK
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5554033513
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749) **Quality Gate passed**
Issues
 [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024628689
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -189,26 +192,60 @@ enum Route {
}
this.requestClass = requestClass;
-
this.requiredLength = parts.size();
this.requirements = requirementsBuilder.build();
this.variables = variablesBuilder.build();
}
+ /**
+ * Shift index to skip the prefix.
+ */
+ private int mappedIndex(int baseIndex, int offset) {
Review Comment:
could we simplify? index 0 is "v1"; the prefix sits right after it
````
private static int mappedIndex(int templateIndex, int prefixLength) {
return templateIndex == 0 ? 0 : templateIndex + prefixLength;
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024628689
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -189,26 +192,60 @@ enum Route {
}
this.requestClass = requestClass;
-
this.requiredLength = parts.size();
this.requirements = requirementsBuilder.build();
this.variables = variablesBuilder.build();
}
+ /**
+ * Shift index to skip the prefix.
+ */
+ private int mappedIndex(int baseIndex, int offset) {
Review Comment:
````
/** Index of the first prefix segment, right after "v1". */
private static final int PREFIX_START = 1;`
/** Maps a template index to a request-path index, skipping prefix segments. */
private static int mappedIndex(int templateIndex, int prefixLength) {
return templateIndex < PREFIX_START ? templateIndex : templateIndex + prefixLength;
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5703194720
please check
````
@Test
public void testResourceNamedNamespacesIsNotTreatedAsPrefix() {
assertEquals(Route.LOAD_NAMESPACE, route("v1/namespaces/namespaces").first());
assertEquals(Route.LOAD_TABLE, route("v1/namespaces/db/tables/namespaces").first());
assertEquals(Route.UPDATE_TABLE,
Route.from(HTTPMethod.POST, "v1/namespaces/db/tables/namespaces").first());
assertEquals(Route.LOAD_TABLE,
route("v1/catalogs/sales/namespaces/db/tables/namespaces").first());
}
````
I think an exact-length match (no prefix) should always win, and among prefixed matches, the fewest prefix segments should win:
````
public static Pair<Route, Map<String, String>> from(HTTPMethod method, String path) {
List<String> parts = SLASH.splitToList(path);
Route best = null;
for (Route candidate : Route.values()) {
if (candidate.matches(method, parts)
&& (best == null || candidate.prefixLength(parts) < best.prefixLength(parts))) {
best = candidate;
}
}
return best == null ? null : Pair.of(best, best.variables(parts));
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5772980812
> **there might be a better way,** how about we pre-sort (descending order) the routes based on `requiredLength` at startup. `prefixLength = request_path_size - requiredLength` **_The greater the requiredLength the lower the prefixLenght_**. So the first one will be best match. WDYT?
I would keep the explicit best-match loop with the early exit on prefixLength == 0, and skip the pre-sorting
````
/**
* Resolves the route for a request. When several routes match (e.g. a table named
* "namespaces" can make a commit look like CREATE_NAMESPACE with a long prefix), the
* route with the shortest implied prefix wins. This is a heuristic: without knowing
* the server's expected prefix, such paths are inherently ambiguous.
* Ties keep the first-declared route; no current templates can tie.
*/
public static Pair<Route, Map<String, String>> from(HTTPMethod method, String path) {
List<String> parts = SLASH.splitToList(path);
Route best = null;
int bestPrefix = Integer.MAX_VALUE;
for (Route candidate : Route.values()) {
if (!candidate.matches(method, parts)) {
continue;
}
int prefix = candidate.prefixLength(parts);
if (prefix < bestPrefix) {
best = candidate;
bestPrefix = prefix;
if (prefix == 0) {
break; // exact match, can't do better
}
}
}
return best == null ? null : Pair.of(best, best.variables(parts));
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5704342870
> please check
>
> ```
> @Test
> public void testResourceNamedNamespacesIsNotTreatedAsPrefix() {
> assertEquals(Route.LOAD_NAMESPACE, route("v1/namespaces/namespaces").first());
> assertEquals(Route.LOAD_TABLE, route("v1/namespaces/db/tables/namespaces").first());
> assertEquals(Route.UPDATE_TABLE,
> Route.from(HTTPMethod.POST, "v1/namespaces/db/tables/namespaces").first());
> assertEquals(Route.LOAD_TABLE,
> route("v1/catalogs/sales/namespaces/db/tables/namespaces").first());
> }
> ```
>
> I think an exact-length match (no prefix) should always win, and among prefixed matches, the fewest prefix segments should win:
>
> ```
> public static Pair<Route, Map<String, String>> from(HTTPMethod method, String path) {
> List<String> parts = SLASH.splitToList(path);
> Route best = null;
> for (Route candidate : Route.values()) {
> if (candidate.matches(method, parts)
> && (best == null || candidate.prefixLength(parts) < best.prefixLength(parts))) {
> best = candidate;
> }
> }
> return best == null ? null : Pair.of(best, best.variables(parts));
> }
> ```
**Yes, this edge case was a miss from me** but 1 question in this approach, if I do `/v1/config` then it will search for all the Route enum values i.e. all 25 routes LIST_NAMESPACES, CREATE_NAMESPACE etc. but as the number of routes is just 25 so there shouldn't be any performance concern but this cost will be for all REST HTTP calls.
**there might be a better way,**
how about we pre-sort (descending order) the routes based on `requiredLength` at startup. `prefixLength = request_path_size - requiredLength` **_The greater the requiredLength the lower the prefixLenght_**. So the first one will be best match. WDYT?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5783244007
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749) **Quality Gate passed**
Issues
 [21 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_coverage&view=list)
 [0.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024615428
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -173,10 +175,11 @@ enum Route {
Class<? extends RESTRequest> requestClass) {
this.method = method;
this.resourcePath = pattern;
+ this.withPrefix = pattern.contains(PREFIX);
// parse the pattern into requirements and variables
List<String> parts =
Review Comment:
maybe
````
private static final Splitter SLASH = Splitter.on('/').omitEmptyStrings();
private static final String PREFIX_VAR = "prefix";
private static final String PREFIX_PLACEHOLDER = "{" + PREFIX_VAR + "}";
private final String pathTemplate; // or keep `pattern`, matching the ctor param
private final boolean acceptsPrefix; // or supportsPrefix
List<String> segments = SLASH.splitToList(pattern);
this.acceptsPrefix = segments.contains(PREFIX_PLACEHOLDER);
List<String> parts = segments.stream()
.filter(s -> !PREFIX_PLACEHOLDER.equals(s))
.toList();
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r3941014443
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -162,6 +162,7 @@ enum Route {
private final Map<Integer, String> variables;
private final Class<? extends RESTRequest> requestClass;
private final String resourcePath;
+ private final boolean hasPrefix;
Review Comment:
`withPrefix` ?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5552720988
> This PR introduces a mathematical offset that dynamically calculates the length of any prefix, allowing the server to gracefully accept and route multi-segment prefixes.
doesn't iceberg lib provide utils we can reuse to do the proper routing?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5701401830
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749) **Quality Gate passed**
Issues
 [21 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024628689
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -189,26 +192,60 @@ enum Route {
}
this.requestClass = requestClass;
-
this.requiredLength = parts.size();
this.requirements = requirementsBuilder.build();
this.variables = variablesBuilder.build();
}
+ /**
+ * Shift index to skip the prefix.
+ */
+ private int mappedIndex(int baseIndex, int offset) {
Review Comment:
````
/** Index of the first prefix segment, right after "v1". */
private static final int PREFIX_START = 1;`
/** Maps a template index to a request-path index, skipping prefix segments. */
private static int mappedIndex(int templateIndex, int prefixLength) {
return templateIndex < PREFIX_START ? templateIndex : templateIndex + prefixLength;
}
/** Number of extra segments in the request, i.e. the prefix length. */
private int prefixLength(List<String> requestPath) {
return requestPath.size() - requiredLength;
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r3941016352
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -162,6 +162,7 @@ enum Route {
private final Map<Integer, String> variables;
private final Class<? extends RESTRequest> requestClass;
private final String resourcePath;
+ private final boolean hasPrefix;
Review Comment:
ACK
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5553483912
> > This PR introduces a mathematical offset that dynamically calculates the length of any prefix, allowing the server to gracefully accept and route multi-segment prefixes.
>
> doesn't iceberg lib provide utils we can reuse to do the proper routing?
LLM respose 😬:
Iceberg core provides the CatalogHandlers classes for the actual operations, but it does not provide framework-agnostic HTTP routing utilities. Iceberg deliberately leaves HTTP path routing and prefix extraction up to the implementing web framework (e.g., Spring Boot, Javalin, Spark Java). Since Hive's REST catalog implementation uses a raw HttpServlet (via IcebergMetastoreServlet), we have to manually parse the path and extract the optional prefix offset.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r3941019818
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -434,92 +470,39 @@ private static void commitTransaction(Catalog catalog, CommitTransactionRequest
// only commit if validations passed previously
transactions.forEach(Transaction::commitTransaction);
}
-
- @SuppressWarnings({"MethodLength", "unchecked"})
+
+ @SuppressWarnings({"unchecked"})
private <T extends RESTResponse> T handleRequest(
Route route, Map<String, String> vars, Object body) {
- switch (route) {
- case CONFIG:
- return (T) config();
-
- case LIST_NAMESPACES:
- return (T) listNamespaces(vars);
-
- case CREATE_NAMESPACE:
- return (T) createNamespace(body);
-
- case NAMESPACE_EXISTS:
- return (T) namespaceExists(vars);
-
- case LOAD_NAMESPACE:
- return (T) loadNamespace(vars);
-
- case DROP_NAMESPACE:
- return (T) dropNamespace(vars);
-
- case UPDATE_NAMESPACE:
- return (T) updateNamespace(vars, body);
-
- case LIST_TABLES:
- return (T) listTables(vars);
-
- case CREATE_TABLE:
- return (T) createTable(vars, body);
-
- case DROP_TABLE:
- return (T) dropTable(vars);
-
- case TABLE_EXISTS:
- return (T) tableExists(vars);
-
- case LOAD_TABLE:
- return (T) loadTable(vars);
-
- case REGISTER_TABLE:
- return (T) registerTable(vars, body);
-
- case UPDATE_TABLE:
- return (T) updateTable(vars, body);
-
- case RENAME_TABLE:
- return (T) renameTable(body);
-
- case REPORT_METRICS:
- return (T) reportMetrics(vars, body);
-
- case COMMIT_TRANSACTION:
- return (T) commitTransaction(body);
-
- case LIST_VIEWS:
- return (T) listViews(vars);
-
- case CREATE_VIEW:
- return (T) createView(vars, body);
-
- case VIEW_EXISTS:
- return (T) viewExists(vars);
-
- case LOAD_VIEW:
- return (T) loadView(vars);
-
- case UPDATE_VIEW:
- return (T) updateView(vars, body);
-
- case RENAME_VIEW:
- return (T) renameView(body);
-
- case DROP_VIEW:
- return (T) dropView(vars);
-
- case REGISTER_VIEW:
- return (T) registerView(vars, body);
-
- default:
- }
- return null;
+ return (T) switch (route) {
+ case CONFIG -> config();
+ case LIST_NAMESPACES -> listNamespaces(vars);
+ case CREATE_NAMESPACE -> createNamespace(body);
+ case NAMESPACE_EXISTS -> namespaceExists(vars);
+ case LOAD_NAMESPACE -> loadNamespace(vars);
+ case DROP_NAMESPACE -> dropNamespace(vars);
+ case UPDATE_NAMESPACE -> updateNamespace(vars, body);
+ case LIST_TABLES -> listTables(vars);
+ case CREATE_TABLE -> createTable(vars, body);
+ case DROP_TABLE -> dropTable(vars);
+ case TABLE_EXISTS -> tableExists(vars);
+ case LOAD_TABLE -> loadTable(vars);
+ case REGISTER_TABLE -> registerTable(vars, body);
+ case UPDATE_TABLE -> updateTable(vars, body);
+ case RENAME_TABLE -> renameTable(body);
+ case REPORT_METRICS -> reportMetrics(vars, body);
+ case COMMIT_TRANSACTION -> commitTransaction(body);
+ case LIST_VIEWS -> listViews(vars);
+ case CREATE_VIEW -> createView(vars, body);
+ case VIEW_EXISTS -> viewExists(vars);
+ case LOAD_VIEW -> loadView(vars);
+ case UPDATE_VIEW -> updateView(vars, body);
+ case RENAME_VIEW -> renameView(body);
+ case DROP_VIEW -> dropView(vars);
+ case REGISTER_VIEW -> registerView(vars, body);
Review Comment:
no default?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r3941040286
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -434,92 +470,39 @@ private static void commitTransaction(Catalog catalog, CommitTransactionRequest
// only commit if validations passed previously
transactions.forEach(Transaction::commitTransaction);
}
-
- @SuppressWarnings({"MethodLength", "unchecked"})
+
+ @SuppressWarnings({"unchecked"})
private <T extends RESTResponse> T handleRequest(
Route route, Map<String, String> vars, Object body) {
- switch (route) {
- case CONFIG:
- return (T) config();
-
- case LIST_NAMESPACES:
- return (T) listNamespaces(vars);
-
- case CREATE_NAMESPACE:
- return (T) createNamespace(body);
-
- case NAMESPACE_EXISTS:
- return (T) namespaceExists(vars);
-
- case LOAD_NAMESPACE:
- return (T) loadNamespace(vars);
-
- case DROP_NAMESPACE:
- return (T) dropNamespace(vars);
-
- case UPDATE_NAMESPACE:
- return (T) updateNamespace(vars, body);
-
- case LIST_TABLES:
- return (T) listTables(vars);
-
- case CREATE_TABLE:
- return (T) createTable(vars, body);
-
- case DROP_TABLE:
- return (T) dropTable(vars);
-
- case TABLE_EXISTS:
- return (T) tableExists(vars);
-
- case LOAD_TABLE:
- return (T) loadTable(vars);
-
- case REGISTER_TABLE:
- return (T) registerTable(vars, body);
-
- case UPDATE_TABLE:
- return (T) updateTable(vars, body);
-
- case RENAME_TABLE:
- return (T) renameTable(body);
-
- case REPORT_METRICS:
- return (T) reportMetrics(vars, body);
-
- case COMMIT_TRANSACTION:
- return (T) commitTransaction(body);
-
- case LIST_VIEWS:
- return (T) listViews(vars);
-
- case CREATE_VIEW:
- return (T) createView(vars, body);
-
- case VIEW_EXISTS:
- return (T) viewExists(vars);
-
- case LOAD_VIEW:
- return (T) loadView(vars);
-
- case UPDATE_VIEW:
- return (T) updateView(vars, body);
-
- case RENAME_VIEW:
- return (T) renameView(body);
-
- case DROP_VIEW:
- return (T) dropView(vars);
-
- case REGISTER_VIEW:
- return (T) registerView(vars, body);
-
- default:
- }
- return null;
+ return (T) switch (route) {
+ case CONFIG -> config();
+ case LIST_NAMESPACES -> listNamespaces(vars);
+ case CREATE_NAMESPACE -> createNamespace(body);
+ case NAMESPACE_EXISTS -> namespaceExists(vars);
+ case LOAD_NAMESPACE -> loadNamespace(vars);
+ case DROP_NAMESPACE -> dropNamespace(vars);
+ case UPDATE_NAMESPACE -> updateNamespace(vars, body);
+ case LIST_TABLES -> listTables(vars);
+ case CREATE_TABLE -> createTable(vars, body);
+ case DROP_TABLE -> dropTable(vars);
+ case TABLE_EXISTS -> tableExists(vars);
+ case LOAD_TABLE -> loadTable(vars);
+ case REGISTER_TABLE -> registerTable(vars, body);
+ case UPDATE_TABLE -> updateTable(vars, body);
+ case RENAME_TABLE -> renameTable(body);
+ case REPORT_METRICS -> reportMetrics(vars, body);
+ case COMMIT_TRANSACTION -> commitTransaction(body);
+ case LIST_VIEWS -> listViews(vars);
+ case CREATE_VIEW -> createView(vars, body);
+ case VIEW_EXISTS -> viewExists(vars);
+ case LOAD_VIEW -> loadView(vars);
+ case UPDATE_VIEW -> updateView(vars, body);
+ case RENAME_VIEW -> renameView(body);
+ case DROP_VIEW -> dropView(vars);
+ case REGISTER_VIEW -> registerView(vars, body);
Review Comment:
IntelliJ said :-)
<img width="499" height="211" alt="Screenshot 2026-09-05 at 8 39 33 PM" src="https://github.com/user-attachments/assets/fae60c7a-f34d-477b-8648-45cc3e1e6aa0" />
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5503543768
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749) **Quality Gate passed**
Issues
 [18 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024615428
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -173,10 +175,11 @@ enum Route {
Class<? extends RESTRequest> requestClass) {
this.method = method;
this.resourcePath = pattern;
+ this.withPrefix = pattern.contains(PREFIX);
// parse the pattern into requirements and variables
List<String> parts =
Review Comment:
maybe
````
private static final Splitter SLASH = Splitter.on('/').omitEmptyStrings();
private static final String PREFIX_VAR = "prefix";
private static final String PREFIX_PLACEHOLDER = "{" + PREFIX_VAR + "}";
private final String pathTemplate;
private final boolean acceptsPrefix;
Route(HTTPMethod method, String pattern,
Class<? extends RESTRequest> requestClass) {
this.method = method;
this.pathTemplate = pattern;
List<String> segments = SLASH.splitToList(pattern);
this.acceptsPrefix = segments.contains(PREFIX_PLACEHOLDER);
// parse the pattern into requirements and variables
List<String> parts = segments.stream()
.filter(s -> !PREFIX_PLACEHOLDER.equals(s))
.toList();
// unchanged
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4029839192
##########
standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCatalogAdapterRoutes.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.rest;
+
+import java.util.Map;
+import org.apache.iceberg.util.Pair;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.apache.iceberg.rest.HTTPRequest.HTTPMethod;
+
+public class TestHMSCatalogAdapterRoutes {
+
+ @Test
+ public void testPrefixRouting() {
Review Comment:
Could you please split the single test into one method per case, add negative and collision cases.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5500354046
**BEFORE:**
```
spark-sql ()> show databases;
26/09/01 19:21:04 ERROR SparkSQLDriver: Failed in [show databases]
org.apache.iceberg.exceptions.BadRequestException: Malformed request: No route for request: GET v1/my_test_prefix/namespaces
at org.apache.iceberg.rest.ErrorHandlers$NamespaceErrorHandler.accept(ErrorHandlers.java:276)
at org.apache.iceberg.rest.ErrorHandlers$NamespaceErrorHandler.accept(ErrorHandlers.java:266)
at org.apache.iceberg.rest.HTTPClient.throwFailure(HTTPClient.java:242)
at org.apache.iceberg.rest.HTTPClient.execute(HTTPClient.java:347)
at org.apache.iceberg.rest.HTTPClient.execute(HTTPClient.java:299)
at org.apache.iceberg.rest.BaseHTTPClient.get(BaseHTTPClient.java:77)
at org.apache.iceberg.rest.RESTSessionCatalog.listNamespaces(RESTSessionCatalog.java:776)
at org.apache.iceberg.catalog.BaseSessionCatalog$AsCatalog.listNamespaces(BaseSessionCatalog.java:139)
at org.apache.iceberg.rest.RESTCatalog.listNamespaces(RESTCatalog.java:256)
at org.apache.iceberg.catalog.SupportsNamespaces.listNamespaces(SupportsNamespaces.java:74)
at org.apache.iceberg.spark.SparkCatalog.listNamespaces(SparkCatalog.java:383)
at org.apache.spark.sql.execution.command.ShowNamespacesCommand.run(ShowNamespacesCommand.scala:44)
at org.apache.spark.sql.execution.command.ExecutedCommandExec.sideEffectResult$lzycompute(commands.scala:79)
```
**AFTER:**
<img width="1447" height="595" alt="Screenshot 2026-09-02 at 2 15 34 AM" src="https://github.com/user-attachments/assets/e68910c4-4bea-4602-a1a1-ec8dd58c017f" />
HTTP request fired from spark -> HMS IRC: `GET /iceberg/v1/my_test_prefix/namespaces?pageToken= HTTP/1.1`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5501337621
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749) **Quality Gate passed**
Issues
 [18 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #6749:
URL: https://github.com/apache/hive/pull/6749
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5706375699
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749) **Quality Gate passed**
Issues
 [52 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6749&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6749&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_coverage&view=list)
 [0.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6749&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6749)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5536598997
@deniskuzZ can you help with review?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5553446077
@deniskuzZ To address your question about routing utilities: Iceberg core provides the `CatalogHandlers` classes for the actual operations, but it does not provide framework-agnostic HTTP routing utilities. Iceberg deliberately leaves HTTP path routing and prefix extraction up to the implementing web framework (e.g., Spring Boot, Javalin, Spark Java). Since Hive's REST catalog implementation uses a raw `HttpServlet` (via `IcebergMetastoreServlet`), we have to manually parse the path and extract the optional prefix offset. I've also added the `default` case to the switch expression in the latest commit.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r3941015492
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -173,6 +174,7 @@ enum Route {
Class<? extends RESTRequest> requestClass) {
this.method = method;
this.resourcePath = pattern;
+ this.hasPrefix = pattern.contains("{prefix}");
Review Comment:
please move to constants
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r3941019818
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -434,92 +470,39 @@ private static void commitTransaction(Catalog catalog, CommitTransactionRequest
// only commit if validations passed previously
transactions.forEach(Transaction::commitTransaction);
}
-
- @SuppressWarnings({"MethodLength", "unchecked"})
+
+ @SuppressWarnings({"unchecked"})
private <T extends RESTResponse> T handleRequest(
Route route, Map<String, String> vars, Object body) {
- switch (route) {
- case CONFIG:
- return (T) config();
-
- case LIST_NAMESPACES:
- return (T) listNamespaces(vars);
-
- case CREATE_NAMESPACE:
- return (T) createNamespace(body);
-
- case NAMESPACE_EXISTS:
- return (T) namespaceExists(vars);
-
- case LOAD_NAMESPACE:
- return (T) loadNamespace(vars);
-
- case DROP_NAMESPACE:
- return (T) dropNamespace(vars);
-
- case UPDATE_NAMESPACE:
- return (T) updateNamespace(vars, body);
-
- case LIST_TABLES:
- return (T) listTables(vars);
-
- case CREATE_TABLE:
- return (T) createTable(vars, body);
-
- case DROP_TABLE:
- return (T) dropTable(vars);
-
- case TABLE_EXISTS:
- return (T) tableExists(vars);
-
- case LOAD_TABLE:
- return (T) loadTable(vars);
-
- case REGISTER_TABLE:
- return (T) registerTable(vars, body);
-
- case UPDATE_TABLE:
- return (T) updateTable(vars, body);
-
- case RENAME_TABLE:
- return (T) renameTable(body);
-
- case REPORT_METRICS:
- return (T) reportMetrics(vars, body);
-
- case COMMIT_TRANSACTION:
- return (T) commitTransaction(body);
-
- case LIST_VIEWS:
- return (T) listViews(vars);
-
- case CREATE_VIEW:
- return (T) createView(vars, body);
-
- case VIEW_EXISTS:
- return (T) viewExists(vars);
-
- case LOAD_VIEW:
- return (T) loadView(vars);
-
- case UPDATE_VIEW:
- return (T) updateView(vars, body);
-
- case RENAME_VIEW:
- return (T) renameView(body);
-
- case DROP_VIEW:
- return (T) dropView(vars);
-
- case REGISTER_VIEW:
- return (T) registerView(vars, body);
-
- default:
- }
- return null;
+ return (T) switch (route) {
+ case CONFIG -> config();
+ case LIST_NAMESPACES -> listNamespaces(vars);
+ case CREATE_NAMESPACE -> createNamespace(body);
+ case NAMESPACE_EXISTS -> namespaceExists(vars);
+ case LOAD_NAMESPACE -> loadNamespace(vars);
+ case DROP_NAMESPACE -> dropNamespace(vars);
+ case UPDATE_NAMESPACE -> updateNamespace(vars, body);
+ case LIST_TABLES -> listTables(vars);
+ case CREATE_TABLE -> createTable(vars, body);
+ case DROP_TABLE -> dropTable(vars);
+ case TABLE_EXISTS -> tableExists(vars);
+ case LOAD_TABLE -> loadTable(vars);
+ case REGISTER_TABLE -> registerTable(vars, body);
+ case UPDATE_TABLE -> updateTable(vars, body);
+ case RENAME_TABLE -> renameTable(body);
+ case REPORT_METRICS -> reportMetrics(vars, body);
+ case COMMIT_TRANSACTION -> commitTransaction(body);
+ case LIST_VIEWS -> listViews(vars);
+ case CREATE_VIEW -> createView(vars, body);
+ case VIEW_EXISTS -> viewExists(vars);
+ case LOAD_VIEW -> loadView(vars);
+ case UPDATE_VIEW -> updateView(vars, body);
+ case RENAME_VIEW -> renameView(body);
+ case DROP_VIEW -> dropView(vars);
+ case REGISTER_VIEW -> registerView(vars, body);
Review Comment:
no default? throw some exception
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024615428
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -173,10 +175,11 @@ enum Route {
Class<? extends RESTRequest> requestClass) {
this.method = method;
this.resourcePath = pattern;
+ this.withPrefix = pattern.contains(PREFIX);
// parse the pattern into requirements and variables
List<String> parts =
Review Comment:
maybe
````
private static final Splitter TEMPLATE_SPLITTER = Splitter.on('/').omitEmptyStrings();
private static final String PREFIX_VAR = "prefix";
private static final String PREFIX_PLACEHOLDER = "{" + PREFIX_VAR + "}";
private final String pathTemplate;
private final boolean acceptsPrefix;
Route(HTTPMethod method, String pattern,
Class<? extends RESTRequest> requestClass) {
..
this.pathTemplate = pattern;
List<String> segments = TEMPLATE_SPLITTER.splitToList(pattern);
this.acceptsPrefix = segments.contains(PREFIX_PLACEHOLDER);
// parse the pattern into requirements and variables
List<String> parts = segments.stream()
.filter(s -> !PREFIX_PLACEHOLDER.equals(s))
.toList();
....
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024742132
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -189,26 +192,60 @@ enum Route {
}
this.requestClass = requestClass;
-
this.requiredLength = parts.size();
this.requirements = requirementsBuilder.build();
this.variables = variablesBuilder.build();
}
+ /**
+ * Shift index to skip the prefix.
+ */
+ private int mappedIndex(int baseIndex, int offset) {
+ return (offset > 0 && baseIndex >= 1) ? baseIndex + offset : baseIndex;
+ }
+
private boolean matches(HTTPMethod requestMethod, List<String> requestPath) {
- return method == requestMethod
- && requiredLength == requestPath.size()
- && requirements.entrySet().stream()
- .allMatch(
- requirement ->
- requirement
- .getValue()
- .equalsIgnoreCase(requestPath.get(requirement.getKey())));
+ if (method != requestMethod) {
+ return false;
+ }
+
+ int size = requestPath.size();
+ // Calculate the size of the optional prefix by checking how much the path expanded.
+ // For a multi-segment prefix like 'catalogs/my_catalog', offset will be 2.
+ int offset = size - requiredLength;
+
+ // If the path is too short, or too long but the route doesn't support a prefix, reject.
+ if (offset < 0 || (offset > 0 && !withPrefix)) {
+ return false;
+ }
+
+ for (Map.Entry<Integer, String> requirement : requirements.entrySet()) {
+ if (!requirement.getValue().equalsIgnoreCase(requestPath.get(mappedIndex(requirement.getKey(), offset)))) {
+ return false;
+ }
+ }
+ return true;
}
private Map<String, String> variables(List<String> requestPath) {
ImmutableMap.Builder<String, String> vars = ImmutableMap.builder();
- variables.forEach((key, value) -> vars.put(value, requestPath.get(key)));
+ int offset = requestPath.size() - requiredLength;
Review Comment:
````
int prefixLength = prefixLength(requestPath);
ImmutableMap.Builder<String, String> vars = ImmutableMap.builder();
for (Map.Entry<Integer, String> var : variables.entrySet()) {
vars.put(var.getValue(), requestPath.get(mappedIndex(var.getKey(), prefixLength)));
}
if (prefixLength > 0) {
// Clients insert the configured prefix verbatim, so "catalogs/sales" arrives
// as two segments; rejoin them. An encoded %2F inside a prefix is not preserved.
String prefix = String.join("/",
requestPath.subList(PREFIX_START, PREFIX_START + prefixLength));
// The HMS backend serves a single HiveCatalog and does not scope by prefix yet.
LOG.debug("Ignoring request prefix '{}' for route {}", prefix, this);
vars.put(PREFIX_VAR, prefix);
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6749:
URL: https://github.com/apache/hive/pull/6749#discussion_r4024727939
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -189,26 +192,60 @@ enum Route {
}
this.requestClass = requestClass;
-
this.requiredLength = parts.size();
this.requirements = requirementsBuilder.build();
this.variables = variablesBuilder.build();
}
+ /**
+ * Shift index to skip the prefix.
+ */
+ private int mappedIndex(int baseIndex, int offset) {
+ return (offset > 0 && baseIndex >= 1) ? baseIndex + offset : baseIndex;
+ }
+
private boolean matches(HTTPMethod requestMethod, List<String> requestPath) {
- return method == requestMethod
- && requiredLength == requestPath.size()
- && requirements.entrySet().stream()
- .allMatch(
- requirement ->
- requirement
- .getValue()
- .equalsIgnoreCase(requestPath.get(requirement.getKey())));
+ if (method != requestMethod) {
+ return false;
+ }
+
+ int size = requestPath.size();
Review Comment:
````
// A multi-segment prefix like "catalogs/my_catalog" gives prefixLength == 2
int prefixLength = prefixLength(requestPath);
if (prefixLength < 0 || (prefixLength > 0 && !acceptsPrefix)) {
return false;
}
for (Map.Entry<Integer, String> requirement : requirements.entrySet()) {
String actual = requestPath.get(mappedIndex(requirement.getKey(), prefixLength));
if (!requirement.getValue().equalsIgnoreCase(actual)) {
return false;
}
}
````
helper
````
/** Number of extra segments in the request, i.e. the prefix length. */
private int prefixLength(List<String> requestPath) {
return requestPath.size() - requiredLength;
}
````
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints containing an optional {prefix} [hive]
Posted by "Aggarwal-Raghav (via GitHub)" <gi...@apache.org>.
Aggarwal-Raghav commented on PR #6749:
URL: https://github.com/apache/hive/pull/6749#issuecomment-5789676920
@deniskuzZ , updated the PR with your approach. Please compare the diff once. Hoping everything is addressed.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org