singhpk234 commented on a change in pull request #4248:
URL: https://github.com/apache/iceberg/pull/4248#discussion_r817329038



##########
File path: 
aws/src/main/java/org/apache/iceberg/aws/glue/IcebergToGlueConverter.java
##########
@@ -175,12 +185,25 @@ static void validateTableIdentifier(TableIdentifier 
tableIdentifier) {
    * and should never be used by any query processing engine to infer 
information like schema, partition spec, etc.
    * The source of truth is stored in the actual Iceberg metadata file defined 
by the metadata_location table property.
    * @param tableInputBuilder Glue TableInput builder
+   * @param tableProps Table properties
    * @param metadata Iceberg table metadata
    */
-  static void setTableInputInformation(TableInput.Builder tableInputBuilder, 
TableMetadata metadata) {
+  static void setTableInputInformation(
+      TableInput.Builder tableInputBuilder,
+      Map<String, String> tableProps,
+      TableMetadata metadata) {
     try {
+      StorageDescriptor.Builder storageDescriptor = 
StorageDescriptor.builder();
+      if (!ADDITIONAL_LOCATIONS.isNoop()) {
+        ADDITIONAL_LOCATIONS.invoke(storageDescriptor.additionalLocations(),
+            ImmutableList.of(

Review comment:
       +1
   
   checked ImmutableList.of() fails with null values... 
   - updated test case as well where in the additionalLocations the properties 
we are fetching values from is not present. 
   
   replaced this with location's with not null filtering, as it's not worth 
storing null as well.
   
   ```java
     private static final List<String> PROPERTIES_FOR_GLUE_ADDITIONAL_LOCATIONS 
= ImmutableList.of(
         TableProperties.WRITE_DATA_LOCATION,
         TableProperties.WRITE_METADATA_LOCATION,
         TableProperties.OBJECT_STORE_PATH,
         TableProperties.WRITE_FOLDER_STORAGE_LOCATION
     );
   
     private static Set<String> getAdditionalLocations(TableMetadata 
tableMetadata) {
       return PROPERTIES_FOR_GLUE_ADDITIONAL_LOCATIONS.stream()
           .map(tableMetadata.properties()::get)
           .filter(Objects::nonNull)
           .collect(Collectors.toSet());
     }
   ```




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to