You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "ZephyrYWZhou (via GitHub)" <gi...@apache.org> on 2026/10/05 23:51:47 UTC

[PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

ZephyrYWZhou opened a new pull request, #5707:
URL: https://github.com/apache/polaris/pull/5707

   ## What
   
   Builds on the merged DynamoDB backend skeleton (#5622) — this is the next increment: the single-table **data model** for the `dynamodb-atomic` backend. Pure serialization layer (how Polaris objects map to DynamoDB items), with no client calls, persistence logic, or runtime wiring yet.
   
   - `DynamoDbConstants` — key encodings and attribute names for the single-table layout: entity (`PK realmId#id` / `SK ENTITY`), grant (co-located under the securable), name-sentinel (`realmId#catalogId#parentId#typeCode#name`), plus the two sparse GSI key formats (by-name, by-grantee).
   - `EntityItem`, `GrantItem`, `NameSentinelItem` — to/from DynamoDB item mappers for each item type.
   
   ## Tests
   
   13 unit tests (pure JVM, no DynamoDB required): key-encoding assertions and a full round-trip for each item type.
   
   ## Scope / not in this PR
   
   Landing the key layout on its own so it can be reviewed before anything is built on it. Follow-ups: schema/table creation, the `BasePersistence` logic (writes, CAS, lookups, listing), and the factory/CDI/config wiring that makes the backend selectable.
   
   End-to-end picture (illustrative draft): #5694
   


-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4219189627


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Does this stay unique if `realmId` or the name contains `#`? Iceberg names can't have `#`, but realm ids and catalog names aren't run through `EntityNameValidator`. WDYT ? @ZephyrYWZhou 
   
   cc: @dimas-b 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4223194499


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   I'd just ban `#` in the realm id. The name can still have `#` because it sits at the end of the key.
   
   Putting the length in front of the realm id also works, but then every key that starts with the realm has to do it, and the keys get harder to read. I wouldn't do that unless we really need `#` in realm ids.
   
   @dimas-b does banning `#` in the realm id still look right to you? 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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4202674868


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();
+    item.put(PK, s(entityPk(realmId, e.getId())));
+    item.put(SK, s(SK_ENTITY));
+
+    item.put(A_ID, n(e.getId()));
+    item.put(A_CATALOG_ID, n(e.getCatalogId()));
+    item.put(A_PARENT_ID, n(e.getParentId()));
+    item.put(A_TYPE_CODE, n(e.getTypeCode()));
+    item.put(A_SUB_TYPE_CODE, n(e.getSubTypeCode()));
+    item.put(A_NAME, s(e.getName()));
+    item.put(A_ENTITY_VERSION, n(e.getEntityVersion()));
+    item.put(A_GRANT_RECORDS_VERSION, n(e.getGrantRecordsVersion()));
+    item.put(A_CREATE_TIMESTAMP, n(e.getCreateTimestamp()));
+    item.put(A_DROP_TIMESTAMP, n(e.getDropTimestamp()));
+    item.put(A_PURGE_TIMESTAMP, n(e.getPurgeTimestamp()));
+    item.put(A_TO_PURGE_TIMESTAMP, n(e.getToPurgeTimestamp()));
+    item.put(A_LAST_UPDATE_TIMESTAMP, n(e.getLastUpdateTimestamp()));
+    item.put(A_PROPERTIES, map(e.getPropertiesAsMap()));
+    item.put(A_INTERNAL_PROPERTIES, map(e.getInternalPropertiesAsMap()));
+
+    // by-name GSI keys (sparse: only entity items carry them) -> serve "list children".
+    item.put(GSI1_PK, s(byNameGsiPk(realmId, e.getCatalogId(), e.getParentId(), e.getTypeCode())));
+    item.put(GSI1_SK, s(e.getName()));

Review Comment:
   Just renamed it to make it more readable.



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4202674868


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();
+    item.put(PK, s(entityPk(realmId, e.getId())));
+    item.put(SK, s(SK_ENTITY));
+
+    item.put(A_ID, n(e.getId()));
+    item.put(A_CATALOG_ID, n(e.getCatalogId()));
+    item.put(A_PARENT_ID, n(e.getParentId()));
+    item.put(A_TYPE_CODE, n(e.getTypeCode()));
+    item.put(A_SUB_TYPE_CODE, n(e.getSubTypeCode()));
+    item.put(A_NAME, s(e.getName()));
+    item.put(A_ENTITY_VERSION, n(e.getEntityVersion()));
+    item.put(A_GRANT_RECORDS_VERSION, n(e.getGrantRecordsVersion()));
+    item.put(A_CREATE_TIMESTAMP, n(e.getCreateTimestamp()));
+    item.put(A_DROP_TIMESTAMP, n(e.getDropTimestamp()));
+    item.put(A_PURGE_TIMESTAMP, n(e.getPurgeTimestamp()));
+    item.put(A_TO_PURGE_TIMESTAMP, n(e.getToPurgeTimestamp()));
+    item.put(A_LAST_UPDATE_TIMESTAMP, n(e.getLastUpdateTimestamp()));
+    item.put(A_PROPERTIES, map(e.getPropertiesAsMap()));
+    item.put(A_INTERNAL_PROPERTIES, map(e.getInternalPropertiesAsMap()));
+
+    // by-name GSI keys (sparse: only entity items carry them) -> serve "list children".
+    item.put(GSI1_PK, s(byNameGsiPk(realmId, e.getCatalogId(), e.getParentId(), e.getTypeCode())));
+    item.put(GSI1_SK, s(e.getName()));

Review Comment:
   Just have renamed it to avoid confustion. 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4315239493


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   length-prefix look reasonable 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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4225314595


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Agreed @dimas-b. I'd rather not ban a character from Polaris realm IDs just to accommodate one backend. Changing the encoding not only avoids putting that limitation on users but also unblocks this backend option implementation without requiring a separate `dev` discussion. (The development process here is incremental so next PR will strictly rely on last one)
   
   I personally prefer to go with length-prefixed encoding since that is the best I can think of for now but If anyone has a cleaner encoding approach, I'm very open to suggestions.
   



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4225314595


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Agreed @dimas-b. I'd rather not ban a character from Polaris realm IDs just to accommodate one backend. Changing the encoding not only avoids putting that limitation on users but also unblocks this backend option implementation without requiring a separate `dev` discussion. (The development process here is incremental so one PR will strictly rely on last one)
   
   I personally prefer to go with length-prefixed encoding since that is the best I can think of for now but If anyone has a cleaner encoding approach, I'm very open to suggestions.
   



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4317298564


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();

Review Comment:
   Actually this is a good callout - appreicate it I will change it. 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4233041057


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Using a non-printable char for the separator is also an option. I do not think they will ever be a concern in realm names.



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4202219566


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();
+    item.put(PK, s(entityPk(realmId, e.getId())));
+    item.put(SK, s(SK_ENTITY));
+
+    item.put(A_ID, n(e.getId()));
+    item.put(A_CATALOG_ID, n(e.getCatalogId()));
+    item.put(A_PARENT_ID, n(e.getParentId()));
+    item.put(A_TYPE_CODE, n(e.getTypeCode()));
+    item.put(A_SUB_TYPE_CODE, n(e.getSubTypeCode()));
+    item.put(A_NAME, s(e.getName()));
+    item.put(A_ENTITY_VERSION, n(e.getEntityVersion()));
+    item.put(A_GRANT_RECORDS_VERSION, n(e.getGrantRecordsVersion()));
+    item.put(A_CREATE_TIMESTAMP, n(e.getCreateTimestamp()));
+    item.put(A_DROP_TIMESTAMP, n(e.getDropTimestamp()));
+    item.put(A_PURGE_TIMESTAMP, n(e.getPurgeTimestamp()));
+    item.put(A_TO_PURGE_TIMESTAMP, n(e.getToPurgeTimestamp()));
+    item.put(A_LAST_UPDATE_TIMESTAMP, n(e.getLastUpdateTimestamp()));
+    item.put(A_PROPERTIES, map(e.getPropertiesAsMap()));
+    item.put(A_INTERNAL_PROPERTIES, map(e.getInternalPropertiesAsMap()));
+
+    // by-name GSI keys (sparse: only entity items carry them) -> serve "list children".
+    item.put(GSI1_PK, s(byNameGsiPk(realmId, e.getCatalogId(), e.getParentId(), e.getTypeCode())));
+    item.put(GSI1_SK, s(e.getName()));

Review Comment:
   Fair point @dimas-b, I picked `s` because it mirrors DynamoDB's own type tags but yeah it is not very intuitive when reading. 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4317300620


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbLayout.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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /**
+   * Name-sentinel partition key. {@code realmId} is length-prefixed so the key stays unambiguous
+   * when {@code realmId} or {@code name} contains the separator; both are free-form, whereas the
+   * numeric coordinates cannot contain it.
+   */
+  public static String sentinelPk(

Review Comment:
   Yep make sense



##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();

Review Comment:
   Actually this is a very good callout - appreicate it I will change it. 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4219712911


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Forbidding `SEP` in `realmId` should clear this problem, I think. Name can remain unrestricted.



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4225103254


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Ideally, we should not restrict API-level objects (realm ID) based on one specific backend impl.
   
   If we want to ban `#` from realm ID as a project-level decision, I'd be ok with that... but that needs a `dev` discussion.
   
   I think it is simpler to just change the encoding in this method to be isomorphic.



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #5707:
URL: https://github.com/apache/polaris/pull/5707#issuecomment-6274816548

   This PR is an intermediate step. The code it adds is not exposed to users yet and it has been in review for quite some time.
   
   I'm merging it in its current state. If other reviews have any follow-up concerns we have time to address them on `main` before the new backend is released.


-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4224076907


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Tagging @flyrain as well for input.



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4315239493


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   length prefix looks reasonable 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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on PR #5707:
URL: https://github.com/apache/polaris/pull/5707#issuecomment-6201888417

   Tagging @flyrain for review on this PR as well. 


-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4276604384


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Thanks @dimas-b. Following your point that we should keep the encoding isomorphic rather than restrict realm ids, I went with a length-prefix on the realm id, so realm ids stay unrestricted and the keys are still guaranteed unique.
   
   A couple of reasons I settled on this for now:
   - It keeps the chain moving. The backend is landing as a stack of incremental PRs that each build on the last, so I'd rather unblock it with a small, lightweight fix than hold it up on a longer encoding debate.
   - It's isomorphic with essentially zero overhead, just a length measurement and a prepend, no new dependency, and nothing to parse back.
   
   There are other reasonable options too (the JSON-string idea you raised, etc.), but since none of them differ functionally or on performance for this use case, I didn't want to block progress on the choice. 
   
   Happy to revisit the encoding later if we'd prefer something more self-describing, very open to further discussion. Does that sounds good to you? @dimas-b 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4201712982


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();
+    item.put(PK, s(entityPk(realmId, e.getId())));
+    item.put(SK, s(SK_ENTITY));
+
+    item.put(A_ID, n(e.getId()));
+    item.put(A_CATALOG_ID, n(e.getCatalogId()));
+    item.put(A_PARENT_ID, n(e.getParentId()));
+    item.put(A_TYPE_CODE, n(e.getTypeCode()));
+    item.put(A_SUB_TYPE_CODE, n(e.getSubTypeCode()));
+    item.put(A_NAME, s(e.getName()));
+    item.put(A_ENTITY_VERSION, n(e.getEntityVersion()));
+    item.put(A_GRANT_RECORDS_VERSION, n(e.getGrantRecordsVersion()));
+    item.put(A_CREATE_TIMESTAMP, n(e.getCreateTimestamp()));
+    item.put(A_DROP_TIMESTAMP, n(e.getDropTimestamp()));
+    item.put(A_PURGE_TIMESTAMP, n(e.getPurgeTimestamp()));
+    item.put(A_TO_PURGE_TIMESTAMP, n(e.getToPurgeTimestamp()));
+    item.put(A_LAST_UPDATE_TIMESTAMP, n(e.getLastUpdateTimestamp()));
+    item.put(A_PROPERTIES, map(e.getPropertiesAsMap()));
+    item.put(A_INTERNAL_PROPERTIES, map(e.getInternalPropertiesAsMap()));
+
+    // by-name GSI keys (sparse: only entity items carry them) -> serve "list children".
+    item.put(GSI1_PK, s(byNameGsiPk(realmId, e.getCatalogId(), e.getParentId(), e.getTypeCode())));
+    item.put(GSI1_SK, s(e.getName()));

Review Comment:
   nit: is the short name `s` helpful? It can safe horizontal space, but at the expense of cognitive load... One always has to think that it means 🤔 



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4219682713


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Yes, it is possible that two sets of inputs produce the same key. Good point!
   



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "ZephyrYWZhou (via GitHub)" <gi...@apache.org>.
ZephyrYWZhou commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4222933420


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Good catch @vigneshio! And thanks to @dimas-b @vigneshio  both for digging into this.
   
   Before we settle on forbidding # in `realmId`, I'd like to put another option on the table that keeps realmId unrestricted and still guarantees uniqueness.
   
   The idea is to length-prefix the realmId in the key, so a `realmId` of `a#b` encodes as `3#a#b#....` The reason we have the bug today is that realmId and name are both free-form, so the numeric fields between them can "slide", and two different (realmId, name) combinations can line up on the exact same key when one of them happens to contain a `#`. Writing `realmId`'s length up front pins down exactly where it ends, so that sliding can't happen and two distinct `realmIds` can never collide, even if one of them contains the separator.
   
   There's effectively no added complexity or latency here. We never parse these keys back, so it's just measuring a string's length and prepending it, not a real encode/decode scheme.
   
   The only real cost is that the keys get a little harder to read, which I'd personally consider a fair trade against telling users they can't use a particular character in their realm id. What do you both think?



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #5707:
URL: https://github.com/apache/polaris/pull/5707#issuecomment-6277404524

   It was merged while I'm reviewing. I got a few comments, given this is one of multiple PRs, we can continue to work on it in followups. Thanks @ZephyrYWZhou for the contribution. Thanks @dimas-b and @vigneshio for the 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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4233019468


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   How about JSON object as a string (very short property names)? I do not think lightning fast performance is a concern here. JSON will rename human-readable.
   
   Cf. `PageToken`.



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4219682713


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /** Name-sentinel partition key: {@code realmId#catalogId#parentId#typeCode#name}. */
+  public static String sentinelPk(
+      String realmId, long catalogId, long parentId, int typeCode, String name) {
+    return realmId + SEP + catalogId + SEP + parentId + SEP + typeCode + SEP + name;

Review Comment:
   Yes, it is possible that two distinct sets of inputs produce the same key. Good point!
   



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5707:
URL: https://github.com/apache/polaris/pull/5707#discussion_r4315211381


##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/models/EntityItem.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.polaris.persistence.atomic.dynamodb.models;
+
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CATALOG_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_CREATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_DROP_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ENTITY_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_GRANT_RECORDS_VERSION;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_INTERNAL_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_LAST_UPDATE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_NAME;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PARENT_ID;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PROPERTIES;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_SUB_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TO_PURGE_TIMESTAMP;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.A_TYPE_CODE;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.GSI1_SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.PK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.SK_ENTITY;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.byNameGsiPk;
+import static org.apache.polaris.persistence.atomic.dynamodb.DynamoDbConstants.entityPk;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
+import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
+
+/**
+ * Maps a {@link PolarisBaseEntity} to and from its DynamoDB "entity item" representation.
+ *
+ * <p>The item is keyed by {@code PK = realmId#id}, {@code SK = "ENTITY"}, and carries the by-name
+ * GSI keys so a parent's children can be listed.
+ */
+public final class EntityItem {
+
+  private EntityItem() {}
+
+  /** Serialize an entity to a DynamoDB item map for the given realm. */
+  public static Map<String, AttributeValue> toItem(String realmId, PolarisBaseEntity e) {
+    Map<String, AttributeValue> item = new HashMap<>();

Review Comment:
   Since this appear to be involved in interactions with the database, it would be nice to make the returned value immutable. It greatly simplifies reasoning about behaviour correctness.



##########
persistence/dynamodb-atomic/src/main/java/org/apache/polaris/persistence/atomic/dynamodb/DynamoDbConstants.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.polaris.persistence.atomic.dynamodb;
+
+/**
+ * Attribute names, item-type markers, and key encodings for the single-table DynamoDB layout.
+ *
+ * <p>One base table holds the Polaris item types, each partitioned by its own identity so load
+ * spreads across partitions; a sparse GSI serves the by-name lookup. The {@code realmId} is only a
+ * key prefix (tenant isolation), never a partition key on its own.
+ */
+public final class DynamoDbConstants {
+
+  private DynamoDbConstants() {}
+
+  // ---- Key attributes --------------------------------------------------------------------------
+  public static final String PK = "PK";
+  public static final String SK = "SK";
+
+  // by-name GSI (list children): gsi1pk = realmId#catalogId#parentId#typeCode, gsi1sk = name
+  public static final String GSI1 = "gsi1";
+  public static final String GSI1_PK = "gsi1pk";
+  public static final String GSI1_SK = "gsi1sk";
+
+  // ---- Sort-key / item-type markers ------------------------------------------------------------
+  public static final String SK_ENTITY = "ENTITY";
+  public static final String SK_NAME = "NAME";
+
+  // ---- Entity item attributes ------------------------------------------------------------------
+  public static final String A_CATALOG_ID = "catalogId";
+  public static final String A_PARENT_ID = "parentId";
+  public static final String A_TYPE_CODE = "typeCode";
+  public static final String A_SUB_TYPE_CODE = "subTypeCode";
+  public static final String A_NAME = "name";
+  public static final String A_ENTITY_VERSION = "entityVersion";
+  public static final String A_GRANT_RECORDS_VERSION = "grantRecordsVersion";
+  public static final String A_CREATE_TIMESTAMP = "createTimestamp";
+  public static final String A_DROP_TIMESTAMP = "dropTimestamp";
+  public static final String A_PURGE_TIMESTAMP = "purgeTimestamp";
+  public static final String A_TO_PURGE_TIMESTAMP = "toPurgeTimestamp";
+  public static final String A_LAST_UPDATE_TIMESTAMP = "lastUpdateTimestamp";
+  public static final String A_PROPERTIES = "properties";
+  public static final String A_INTERNAL_PROPERTIES = "internalProperties";
+
+  // ---- Name-sentinel attributes ----------------------------------------------------------------
+  public static final String A_ID = "id";
+
+  private static final String SEP = "#";
+
+  // ---- Key encodings ---------------------------------------------------------------------------
+
+  /** Entity partition key: {@code realmId#id} (one partition per entity). */
+  public static String entityPk(String realmId, long id) {
+    return realmId + SEP + id;
+  }
+
+  /**
+   * Name-sentinel partition key. {@code realmId} is length-prefixed so the key stays unambiguous
+   * when {@code realmId} or {@code name} contains the separator; both are free-form, whereas the
+   * numeric coordinates cannot contain it.
+   */
+  public static String sentinelPk(

Review Comment:
   nit: technically this is not a constant 🤔 WDYT about renaming the class to `DynamoDBUtils`?



-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Persistence: DynamoDB (atomic) single-table data model [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #5707:
URL: https://github.com/apache/polaris/pull/5707


-- 
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@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org