PR Review Suggestions and Fixes

awslabs · Mar 14, 2024 · 92001c0 · 92001c0
1 parent 42a242e
commit 92001c0
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 19 deletions.
diff --git a/...amodb/src/main/java/com/amazonaws/athena/connectors/dynamodb/DynamoDBMetadataHandler.java b/...amodb/src/main/java/com/amazonaws/athena/connectors/dynamodb/DynamoDBMetadataHandler.java
@@ -98,7 +98,6 @@
 import static com.amazonaws.athena.connectors.dynamodb.constants.DynamoDBConstants.SEGMENT_ID_PROPERTY;
 import static com.amazonaws.athena.connectors.dynamodb.constants.DynamoDBConstants.TABLE_METADATA;
 import static com.amazonaws.athena.connectors.dynamodb.throttling.DynamoDBExceptionFilter.EXCEPTION_FILTER;
-import static com.amazonaws.athena.connectors.dynamodb.util.DDBTypeUtils.toAttributeValue;
 
 /**
  * Handles metadata requests for the Athena DynamoDB Connector.
@@ -437,7 +436,7 @@ public GetSplitsResponse doGetSplits(BlockAllocator allocator, GetSplitsRequest
                 Map<String, String> splitMetadata = new HashMap<>(partitionMetadata);
 
                 Object hashKeyValue = DDBTypeUtils.convertArrowTypeIfNecessary(hashKeyName, hashKeyValueReader.readObject());
-                splitMetadata.put(hashKeyName, DDBTypeUtils.attributeToJson(toAttributeValue(hashKeyValue), hashKeyName));
+                splitMetadata.put(hashKeyName, DDBTypeUtils.attributeToJson(DDBTypeUtils.toAttributeValue(hashKeyValue), hashKeyName));
 
                 splits.add(new Split(spillLocation, makeEncryptionKey(), splitMetadata));
 

diff --git a/...namodb/src/main/java/com/amazonaws/athena/connectors/dynamodb/util/.DDBTypeUtils.java.swp b/...namodb/src/main/java/com/amazonaws/athena/connectors/dynamodb/util/.DDBTypeUtils.java.swp
diff --git a/...na-dynamodb/src/main/java/com/amazonaws/athena/connectors/dynamodb/util/DDBTypeUtils.java b/...na-dynamodb/src/main/java/com/amazonaws/athena/connectors/dynamodb/util/DDBTypeUtils.java
@@ -118,20 +118,17 @@ else if (enhancedAttributeValue.isBoolean()) {
         else if (enhancedAttributeValue.isNumber()) {
             return new Field(key, FieldType.nullable(new ArrowType.Decimal(38, 9)), null);
         }
-        else if (enhancedAttributeValue.isSetOfBytes() || enhancedAttributeValue.isSetOfNumbers() || enhancedAttributeValue.isSetOfStrings()) {
-            Field child = null;
-
-            if (enhancedAttributeValue.isSetOfBytes()) {
-                child = new Field(key, FieldType.nullable(Types.MinorType.VARCHAR.getType()), null);
-            }
-            else { 
-               child = new Field(key, FieldType.nullable(Types.MinorType.VARBINARY.getType()), null);
-            }
-
-            return child == null
-                    ? null
-                    : new Field(key, FieldType.nullable(Types.MinorType.LIST.getType()),
-                                Collections.singletonList(child));
+        else if (enhancedAttributeValue.isSetOfBytes()) {
+            Field child = new Field(key, FieldType.nullable(Types.MinorType.VARBINARY.getType()), null);
+            return new Field(key, FieldType.nullable(Types.MinorType.LIST.getType()),  Collections.singletonList(child));
+        }
+        else if (enhancedAttributeValue.isSetOfNumbers()) {
+            Field child = new Field(key, FieldType.nullable(Types.MinorType.DECIMAL.getType()), null);
+            return new Field(key, FieldType.nullable(Types.MinorType.LIST.getType()),  Collections.singletonList(child));
+        }
+        else if (enhancedAttributeValue.isSetOfStrings()) {
+            Field child = new Field(key, FieldType.nullable(Types.MinorType.VARCHAR.getType()), null);
+            return new Field(key, FieldType.nullable(Types.MinorType.LIST.getType()), Collections.singletonList(child));
         }
         else if (enhancedAttributeValue.isListOfAttributeValues()) { 
             Field child = null;
@@ -190,9 +187,8 @@ else if (enhancedAttributeValue.isMap()) {
             return new Field(key, FieldType.nullable(Types.MinorType.STRUCT.getType()), children);
         }
 
-        // #todo; fix this
-        String className = (value == null || value.getClass() == null) ? "null" : value.getClass().getName();
-        throw new RuntimeException("Unknown type[" + className + "] for field[" + key + "]");
+        String attributeTypeName = (value == null || value.getClass() == null) ? "null" : enhancedAttributeValue.type().name();
+        throw new RuntimeException("Unknown Attribute Value Type[" + attributeTypeName + "] for field[" + key + "]");
     }
 
     /**