From ebfc008aecaaebf75185f25cf837adbe69a34b9d Mon Sep 17 00:00:00 2001
From: d87zhang <dzhang323@hotmail.com>
Date: Mon, 6 Jan 2025 15:43:03 -0800
Subject: [PATCH] Add log lines to help debug NPE in resolveLocationForPath
 (#585)

* Add log line to debug npe in resolveLocationForPath

* Fix per Yufei's comments

* use checkNotNull
---
 .../service/catalog/BasePolarisCatalog.java   | 57 +++++++++++++------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
index 83f8fe968..3f2cd7a4d 100644
--- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
+++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
@@ -24,6 +24,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.Arrays;
@@ -351,7 +352,7 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
             "Namespace does not exist: %s", tableIdentifier.namespace());
       }
       List<PolarisEntity> namespacePath = resolvedNamespace.getRawFullPath();
-      String namespaceLocation = resolveLocationForPath(namespacePath);
+      String namespaceLocation = resolveLocationForPath(callContext, namespacePath);
       return SLASH.join(namespaceLocation, tableIdentifier.name());
     }
   }
@@ -532,13 +533,14 @@ private String resolveNamespaceLocation(Namespace namespace, Map<String, String>
               ? getResolvedParentNamespace(namespace).getRawFullPath()
               : List.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity());
 
-      String parentLocation = resolveLocationForPath(parentPath);
+      String parentLocation = resolveLocationForPath(callContext, parentPath);
 
       return parentLocation + "/" + namespace.level(namespace.length() - 1);
     }
   }
 
-  private static @Nonnull String resolveLocationForPath(List<PolarisEntity> parentPath) {
+  private static @Nonnull String resolveLocationForPath(
+      @Nonnull CallContext callContext, List<PolarisEntity> parentPath) {
     // always take the first object. If it has the base-location, stop there
     AtomicBoolean foundBaseLocation = new AtomicBoolean(false);
     return parentPath.reversed().stream()
@@ -551,24 +553,45 @@ private String resolveNamespaceLocation(Namespace namespace, Map<String, String>
         .toList()
         .reversed()
         .stream()
-        .map(
-            entity -> {
-              if (entity.getType().equals(PolarisEntityType.CATALOG)) {
-                return CatalogEntity.of(entity).getDefaultBaseLocation();
-              } else {
-                String baseLocation =
-                    entity.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION);
-                if (baseLocation != null) {
-                  return baseLocation;
-                } else {
-                  return entity.getName();
-                }
-              }
-            })
+        .map(entity -> baseLocation(callContext, entity))
         .map(BasePolarisCatalog::stripLeadingTrailingSlash)
         .collect(Collectors.joining("/"));
   }
 
+  private static @Nullable String baseLocation(
+      @Nonnull CallContext callContext, PolarisEntity entity) {
+    if (entity.getType().equals(PolarisEntityType.CATALOG)) {
+      CatalogEntity catEntity = CatalogEntity.of(entity);
+      String catalogDefaultBaseLocation = catEntity.getDefaultBaseLocation();
+      callContext
+          .getPolarisCallContext()
+          .getDiagServices()
+          .checkNotNull(
+              catalogDefaultBaseLocation,
+              "Tried to resolve location with catalog with null default base location",
+              "catalog = {}",
+              catEntity);
+      return catalogDefaultBaseLocation;
+    } else {
+      String baseLocation =
+          entity.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION);
+      if (baseLocation != null) {
+        return baseLocation;
+      } else {
+        String entityName = entity.getName();
+        callContext
+            .getPolarisCallContext()
+            .getDiagServices()
+            .checkNotNull(
+                entityName,
+                "Tried to resolve location with entity without base location or name",
+                "entity = {}",
+                entity);
+        return entityName;
+      }
+    }
+  }
+
   private static String stripLeadingTrailingSlash(String location) {
     if (location.startsWith("/")) {
       return stripLeadingTrailingSlash(location.substring(1));