szehon-ho commented on code in PR #6771:
URL: https://github.com/apache/iceberg/pull/6771#discussion_r1106265362


##########
docs/spark-queries.md:
##########
@@ -346,6 +346,9 @@ SELECT * FROM prod.db.table.partitions;
 Note:
 For unpartitioned tables, the partitions table will contain only the 
record_count and file_count columns.
 
+Note2:

Review Comment:
   Can you make the two notes a bullet list.  See other notes like Manifests / 
All Manifests



##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -170,6 +170,8 @@ static CloseableIterable<FileScanTask> 
planFiles(StaticTableScan scan) {
                       scan.filter(), transformedSpec, caseSensitive);
                 });
 
+    // Note, the delete files aren't applied to the data files so if there are 
partition values that
+    // have been updated then both the 'old' and the 'new' values are present 
in the output.

Review Comment:
   Im actually not sure it should be inline comment, those to me seems for impl 
details for specific lines and maybe not read if you are just skimming.  If we 
need to, how about the javadoc of the class or method, which is more about the 
overall behavior, as is being commented here?
   
   And also if we do keep it, we should change to document the general case, as 
mentioned in the comment on the docs page, like:
   
   `Note, the delete files aren't applied to the data files, so partitions with 
0 rows may still be present in the output.`
   
   
   



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

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


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

Reply via email to