You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "stevenzwu (via GitHub)" <gi...@apache.org> on 2026/04/29 18:24:37 UTC

[PR] Core, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

stevenzwu opened a new pull request, #16160:
URL: https://github.com/apache/iceberg/pull/16160

   ## Summary
   
   Adds Java reference implementations for the `CatalogObjectIdentifier` and `CatalogObjectType` schemas added to the REST OpenAPI spec in #16144.
   
   - `CatalogObjectIdentifier` (api module): hand-written POJO mirroring `Namespace` — static `of(String...)` factory, null/null-byte validation, `levels()` / `level(i)` / `length()` accessors, dotted `toString`. Accompanied by `CatalogObjectIdentifierParser` (core) for JSON serde and a serializer/deserializer pair registered in `RESTSerializers`.
   - `CatalogObjectType` (core): string enum with values `TABLE`, `VIEW`, `NAMESPACE` — wire strings `table`, `view`, `namespace`. Mirrors `PlanStatus` with a `fromName` factory.
   
   Spec PR: #16144
   
   ## Test plan
   
   - [x] `./gradlew :iceberg-api:test --tests TestCatalogObjectIdentifier`
   - [x] `./gradlew :iceberg-core:test --tests TestCatalogObjectIdentifierParser --tests TestCatalogObjectType`
   - [x] `./gradlew :iceberg-api:spotlessApply :iceberg-core:spotlessApply`
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3185338676


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object (table, view, or namespace) as an ordered list of hierarchical
+ * levels.
+ *
+ * <p>The object kind is determined by context (e.g. the endpoint or a companion {@link
+ * CatalogObjectType} discriminator), not by the identifier structure alone.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create CatalogObjectIdentifier from null array");
+    Preconditions.checkArgument(
+        levels.length > 0, "Cannot create CatalogObjectIdentifier with no levels");
+
+    for (String level : levels) {
+      Preconditions.checkNotNull(
+          level, "Cannot create a CatalogObjectIdentifier with a null level");
+      Preconditions.checkArgument(
+          !CONTAINS_NULL_CHARACTER.test(level),
+          "Cannot create a CatalogObjectIdentifier with the null-byte character");

Review Comment:
   let me fix the style though as we usually don't put class name in the error msg



##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object (table, view, or namespace) as an ordered list of hierarchical
+ * levels.
+ *
+ * <p>The object kind is determined by context (e.g. the endpoint or a companion {@link
+ * CatalogObjectType} discriminator), not by the identifier structure alone.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create CatalogObjectIdentifier from null array");
+    Preconditions.checkArgument(
+        levels.length > 0, "Cannot create CatalogObjectIdentifier with no levels");
+
+    for (String level : levels) {
+      Preconditions.checkNotNull(
+          level, "Cannot create a CatalogObjectIdentifier with a null level");
+      Preconditions.checkArgument(
+          !CONTAINS_NULL_CHARACTER.test(level),
+          "Cannot create a CatalogObjectIdentifier with the null-byte character");

Review Comment:
   This mirrors the same null-byte check on `Namespace` levels (added in #3938). Null bytes are a foot-gun in identifier-level strings — they can corrupt any encoding that uses a control byte as a delimiter, mis-terminate strings in C-based downstream consumers, and slip past path/log normalization. Rejecting them at construction keeps the invariant explicit at the API boundary, the same way `Namespace` does for the same reason.



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3166265525


##########
core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java:
##########
@@ -273,6 +276,27 @@ public void serialize(
     }
   }
 
+  public static class CatalogObjectIdentifierDeserializer
+      extends JsonDeserializer<CatalogObjectIdentifier> {
+    @Override
+    public CatalogObjectIdentifier deserialize(JsonParser p, DeserializationContext context)
+        throws IOException {
+      String[] levels = JsonUtil.getStringArray(p.getCodec().readTree(p));

Review Comment:
   I know it's essentially a one-liner and we do the same for Namespace, but I wonder if it's worth still creating a separate Parser class. That way we can independently test the from/to JSON 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: 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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu merged PR #16160:
URL: https://github.com/apache/iceberg/pull/16160


-- 
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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3358770260


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create catalog object identifier from null array");
+
+    for (String level : levels) {
+      Preconditions.checkNotNull(
+          level, "Cannot create a catalog object identifier with a null level");
+      Preconditions.checkArgument(
+          !CONTAINS_NULL_CHARACTER.test(level),
+          "Cannot create a catalog object identifier with the null-byte character");
+    }

Review Comment:
   I would generally think these checks should belong in the constructor.  The constructor should be the final defense against bad values and in this case it's just a pass through.  



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3166273224


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectType.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.catalog;
+
+import java.util.Locale;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/** The type of a catalog object referenced by a {@code CatalogObjectIdentifier}. */
+public enum CatalogObjectType {
+  TABLE("table"),
+  VIEW("view"),
+  NAMESPACE("namespace");
+
+  private final String type;
+
+  CatalogObjectType(String type) {
+    this.type = type;
+  }
+
+  public String type() {
+    return type;
+  }
+
+  public static CatalogObjectType fromName(String type) {
+    Preconditions.checkArgument(type != null, "Catalog object type is null");

Review Comment:
   nit: I think we use a pattern similar to `Invalid catalog object type: null` in other places



-- 
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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3358740347


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =

Review Comment:
   We don't do this check anywhere else, for any identifier.  What prompted adding this here?



-- 
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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3321077762


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create catalog object identifier from null array");
+    Preconditions.checkArgument(
+        levels.length > 0, "Cannot create catalog object identifier with no levels");

Review Comment:
   also chatted with Ryan offline. It seems the spec and Java impl don't need to add this check of non-empty array. An empty array can reference the catalog root. Although we don't have a strong use case yet, it is not necessary to prevent it. Catalog server may implement their own validations.
   
   With that, I will drop this validation and close the OpenAPI PR #16587  that adds `minItems: 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: 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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3177597032


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object (table, view, or namespace) as an ordered list of hierarchical
+ * levels.
+ *
+ * <p>The object kind is determined by context (e.g. the endpoint or a companion {@link
+ * CatalogObjectType} discriminator), not by the identifier structure alone.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create CatalogObjectIdentifier from null array");
+    Preconditions.checkArgument(
+        levels.length > 0, "Cannot create CatalogObjectIdentifier with no levels");
+
+    for (String level : levels) {
+      Preconditions.checkNotNull(
+          level, "Cannot create a CatalogObjectIdentifier with a null level");
+      Preconditions.checkArgument(
+          !CONTAINS_NULL_CHARACTER.test(level),
+          "Cannot create a CatalogObjectIdentifier with the null-byte character");

Review Comment:
   can you please elaborate why this check is required ? 



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3169633612


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object (table, view, or namespace) as an ordered list of hierarchical
+ * levels.
+ *
+ * <p>The object kind is determined by context (e.g. the endpoint or a companion {@link
+ * CatalogObjectType} discriminator), not by the identifier structure alone.
+ */
+public class CatalogObjectIdentifier {

Review Comment:
   `CatalogObjectIdentifier` is more accurate about identifying an object **within** a catalog.
   
   E.g. a SQL may reference a table as `my_catalog.my_db.my_tbl`. catalog name should be excluded in the `CatalogObjectIdentifier`, which would have `my_db.my_tbl`.
   
   There were discussions that some catalog servers (like Polaris) have `listCatalogs` endpoint. I don't want to confuse this with the catalog name/identifier.



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3166259391


##########
api/src/test/java/org/apache/iceberg/catalog/TestCatalogObjectIdentifier.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.catalog;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+
+public class TestCatalogObjectIdentifier {
+
+  @Test
+  public void testWithNullAndEmpty() {

Review Comment:
   minor: we can omit `test` prefixes in newly added tests as this is a legacy JUnit thing that isn't adding any value



-- 
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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3358891990


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =

Review Comment:
   This mirrors the null-byte check on `Namespace` levels (added in [#3938](https://github.com/apache/iceberg/pull/3938)) — the class Javadoc explicitly notes that this type mirrors `Namespace` structurally. The motivation from #3938 still applies: a `\u0000` inside a level corrupts encodings that use that byte as a delimiter (Hive metastore, some catalog backends), so failing fast at construction is the safe default. `TableIdentifier` does not have the check, but `Namespace` is the closer precedent for this type.



##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create catalog object identifier from null array");
+
+    for (String level : levels) {
+      Preconditions.checkNotNull(
+          level, "Cannot create a catalog object identifier with a null level");
+      Preconditions.checkArgument(
+          !CONTAINS_NULL_CHARACTER.test(level),
+          "Cannot create a catalog object identifier with the null-byte character");
+    }

Review Comment:
   Will move the per-level validation into the private constructor. The constructor as the universal invariant point is the more defensive convention — the rule survives any future construction path (additional factories, test-only construction, reflective deserializers). Was following `Namespace`'s factory-only style earlier, but happy to flip and could align `Namespace` in a follow-up if worth the consistency.



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3170499062


##########
core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java:
##########
@@ -273,6 +276,27 @@ public void serialize(
     }
   }
 
+  public static class CatalogObjectIdentifierDeserializer
+      extends JsonDeserializer<CatalogObjectIdentifier> {
+    @Override
+    public CatalogObjectIdentifier deserialize(JsonParser p, DeserializationContext context)
+        throws IOException {
+      String[] levels = JsonUtil.getStringArray(p.getCodec().readTree(p));

Review Comment:
   I am fine either way. I changed the code as you suggested



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3169633612


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object (table, view, or namespace) as an ordered list of hierarchical
+ * levels.
+ *
+ * <p>The object kind is determined by context (e.g. the endpoint or a companion {@link
+ * CatalogObjectType} discriminator), not by the identifier structure alone.
+ */
+public class CatalogObjectIdentifier {

Review Comment:
   `CatalogObjectIdentifier` is more accurate about identifying an object **within** a catalog.
   
   E.g. a SQL may reference a table as `my_catalog.my_db.my_tbl`. catalog name should be excluded in the `CatalogObjectIdentifier`, which would have `my_db.my_tbl`.
   
   There were discussions that some catalog servers (like Polaris) have `listCatalogs` endpoint. 



-- 
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, API: Add CatalogObjectIdentifier and CatalogObjectType [iceberg]

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3166250870


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object (table, view, or namespace) as an ordered list of hierarchical
+ * levels.
+ *
+ * <p>The object kind is determined by context (e.g. the endpoint or a companion {@link
+ * CatalogObjectType} discriminator), not by the identifier structure alone.
+ */
+public class CatalogObjectIdentifier {

Review Comment:
   nti: what about just calling it `Identifier` as `CatalogObjectIdentifier` seems quite long



-- 
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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3314500354


##########
core/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifierParser.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.catalog;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.util.JsonUtil;
+
+/**
+ * Parses {@link CatalogObjectIdentifier} from its JSON representation used by the REST catalog.
+ *
+ * <p>{@code CatalogObjectIdentifier.of("accounting", "tax", "paid")} serializes to {@code
+ * ["accounting", "tax", "paid"]}.
+ */
+public class CatalogObjectIdentifierParser {
+
+  private CatalogObjectIdentifierParser() {}
+
+  public static String toJson(CatalogObjectIdentifier identifier) {
+    return toJson(identifier, false);
+  }
+
+  public static String toJson(CatalogObjectIdentifier identifier, boolean pretty) {
+    return JsonUtil.generate(gen -> toJson(identifier, gen), pretty);
+  }
+
+  public static void toJson(CatalogObjectIdentifier identifier, JsonGenerator generator)
+      throws IOException {
+    Preconditions.checkArgument(null != identifier, "Invalid catalog object identifier: null");
+    generator.writeArray(identifier.levels(), 0, identifier.length());
+  }
+
+  public static CatalogObjectIdentifier fromJson(String json) {
+    Preconditions.checkArgument(
+        json != null, "Cannot parse catalog object identifier from invalid JSON: null");
+    Preconditions.checkArgument(
+        !json.isEmpty(), "Cannot parse catalog object identifier from invalid JSON: ''");
+    return JsonUtil.parse(json, CatalogObjectIdentifierParser::fromJson);
+  }
+
+  public static CatalogObjectIdentifier fromJson(JsonNode node) {
+    Preconditions.checkArgument(
+        node != null && !node.isNull() && node.isArray(),
+        "Cannot parse missing or non-array catalog object identifier: %s",
+        node);
+    String[] levels = new String[node.size()];
+    for (int i = 0; i < node.size(); i++) {
+      JsonNode element = node.get(i);
+      Preconditions.checkArgument(
+          element.isTextual(),

Review Comment:
   Follow-up: [#16586](https://github.com/apache/iceberg/pull/16586) adds the equivalent `isTextual` validation inside `JsonUtil.getStringArray` (already used by `RESTSerializers.NamespaceDeserializer`). This manual loop will be reverted to `JsonUtil.getStringArray(node)` once that PR merges.



##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create catalog object identifier from null array");
+    Preconditions.checkArgument(
+        levels.length > 0, "Cannot create catalog object identifier with no levels");

Review Comment:
   Follow-up: [#16587](https://github.com/apache/iceberg/pull/16587) adds `minItems: 1` to the OpenAPI schema to match this check. An identifier with zero levels references no object, so it has no meaningful interpretation — unlike `Namespace`, where the empty array is the catalog root.



-- 
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] API, Core: Add CatalogObjectIdentifier [iceberg]

Posted by "c-thiel (via GitHub)" <gi...@apache.org>.
c-thiel commented on code in PR #16160:
URL: https://github.com/apache/iceberg/pull/16160#discussion_r3318557650


##########
api/src/main/java/org/apache/iceberg/catalog/CatalogObjectIdentifier.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.catalog;
+
+import java.util.Arrays;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+
+/**
+ * A reference to a catalog object as an ordered list of hierarchical levels (for example, a table,
+ * view, or namespace). The kind of object is determined by context — the endpoint or a companion
+ * type discriminator — not by the identifier structure alone.
+ *
+ * <p>Mirrors {@link Namespace} structurally; the distinct name signals "any object within a
+ * catalog" and avoids confusion with a future top-level catalog name.
+ */
+public class CatalogObjectIdentifier {
+  private static final Joiner DOT = Joiner.on('.');
+  private static final Predicate<String> CONTAINS_NULL_CHARACTER =
+      Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate();
+
+  public static CatalogObjectIdentifier of(String... levels) {
+    Preconditions.checkArgument(
+        null != levels, "Cannot create catalog object identifier from null array");
+    Preconditions.checkArgument(
+        levels.length > 0, "Cannot create catalog object identifier with no levels");

Review Comment:
   The Event endpoint might become a user of an empty array: The catalog-objects-by-name Filter accepts `CatalogObjectIdentifier`. If an implementation wants to add events that are directly targeting the root catalog object (i.e. some setting changes), then beeing able to express this via an empty array in `CatalogObjectIdentifier` might actually be helpful. To me the interpretation is quite clear: An empty array is the Catalog root.
   I don't have strong feelings about this though.



-- 
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