You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "dramaticlly (via GitHub)" <gi...@apache.org> on 2023/02/09 00:06:18 UTC

[GitHub] [iceberg] dramaticlly commented on a diff in pull request #6779: use table partition schema in add_files for getPartitions to avoid data corruption

dramaticlly commented on code in PR #6779:
URL: https://github.com/apache/iceberg/pull/6779#discussion_r1100813099


##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkSchemaUtil.java:
##########
@@ -392,4 +392,14 @@ public static Map<Integer, String> indexQuotedNameById(Schema schema) {
     Function<String, String> quotingFunc = name -> String.format("`%s`", name.replace("`", "``"));
     return TypeUtil.indexQuotedNameById(schema.asStruct(), quotingFunc);
   }
+
+  /**
+   * convert partition spec to Spark type
+   *
+   * @param spec
+   * @return
+   */
+  public static StructType convert(PartitionSpec spec) {

Review Comment:
   nit: 
   
   ```java
   
     /**
      * Convert a {@link PartitionSpec} to a {@link DataType Spark type}.
      *
      * @param spec a iceberg PartitionSpec
      * @return the equivalent Spark type
      */
   ```



##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -836,9 +836,29 @@ public static String quotedFullIdentifier(String catalogName, Identifier identif
    * @param format format of the file
    * @param partitionFilter partitionFilter of the file
    * @return all table's partitions
+   * @deprecated use {@link Spark3Util#getPartitions(SparkSession, Path, String, Map, Option)}
    */
+  @Deprecated
   public static List<SparkPartition> getPartitions(
       SparkSession spark, Path rootPath, String format, Map<String, String> partitionFilter) {
+    return getPartitions(spark, rootPath, format, partitionFilter, Option.empty());
+  }
+
+  /**
+   * Use Spark to list all partitions in the table.

Review Comment:
   Do you want to add some comment in javadoc to differentiate from the deprecated comment above?
   
   like optional iceberg partition spec parameter to influence how spark list partitions



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org