d87zhang commented on code in PR #585:
URL: https://github.com/apache/polaris/pull/585#discussion_r1894519877
##########
service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java:
##########
@@ -554,14 +554,27 @@ private String resolveNamespaceLocation(Namespace
namespace, Map<String, String>
.map(
entity -> {
if (entity.getType().equals(PolarisEntityType.CATALOG)) {
- return CatalogEntity.of(entity).getDefaultBaseLocation();
+ CatalogEntity catEntity = CatalogEntity.of(entity);
+ String catalogDefaultBaseLocation =
catEntity.getDefaultBaseLocation();
+ if (catalogDefaultBaseLocation == null) {
+ LOGGER.warn(
+ "Tried to resolve location with catalog with null
default base location. Catalog = {}",
+ catEntity);
+ }
+ return catalogDefaultBaseLocation;
Review Comment:
I'm not too familiar and don't know if this should ever not happen but
sounds good to use `PolarisDiagnostics.checkNotNull`. I'm adding some code
```
callContext
.getPolarisCallContext().getDiagServices().checkNotNull
```
to get the diagnostic services to call checkNotNull though so please check
if it's safe to do this or if we should be worried about NPEs.. It seems like
elsewhere in the code we're assuming there aren't nulls when calling such
methods.
##########
service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java:
##########
@@ -554,14 +554,27 @@ private String resolveNamespaceLocation(Namespace
namespace, Map<String, String>
.map(
entity -> {
if (entity.getType().equals(PolarisEntityType.CATALOG)) {
- return CatalogEntity.of(entity).getDefaultBaseLocation();
+ CatalogEntity catEntity = CatalogEntity.of(entity);
+ String catalogDefaultBaseLocation =
catEntity.getDefaultBaseLocation();
+ if (catalogDefaultBaseLocation == null) {
+ LOGGER.warn(
+ "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 {
- return entity.getName();
+ String entityName = entity.getName();
+ if (entityName == null) {
+ LOGGER.warn(
+ "Tried to resolve location with entity without base
location or name. entity = {}",
+ entity);
+ }
+ return entityName;
Review Comment:
See https://github.com/apache/polaris/pull/585#discussion_r1894519877
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
