stevenzwu commented on code in PR #16425:
URL: https://github.com/apache/iceberg/pull/16425#discussion_r4342208473


##########
format/spec.md:
##########
@@ -553,7 +554,7 @@ Note that:
 
 ### Partitioning
 
-Data files are stored in manifests with a tuple of partition values that are 
used in scans to filter out files that cannot contain records that match the 
scan’s filter predicate. Partition values for a data file must be the same for 
all records stored in the data file. Manifests store data files from any 
partition. Manifests in v3 and earlier store files for a single spec. Starting 
in v4, manifests may store files from any spec. A manifest is considered 
partitioned by a spec if every entry in the manifest is partitioned by that 
spec.
+Data files are stored in manifests with a tuple of partition values that are 
used in scans to filter out files that cannot contain records that match the 
scan’s filter predicate. Partition values for a data file must be the same for 
all records stored in the data file. For a data file with column files, this 
requirement applies only to rows that are not deleted, using values read with 
its column files. Manifests store data files from any partition. Manifests in 
v3 and earlier store files for a single spec. Starting in v4, manifests may 
store files from any spec. A manifest is considered partitioned by a spec if 
every entry in the manifest is partitioned by that spec.

Review Comment:
   +1. I am not sure what this added sentence is for.
   
   > Partition values for a data file must be the same for all records stored 
in the data file.
   
   if we want to clarify anything, the above sentence is probably where we may 
want to update a little bit. 
   
   ```
   Partition values for a data file (including attached column files) must be 
the same for all records stored in the data file. 
   ```
   
   But we can also just add a general clarification that in v4 a data file 
means a base file + optional column files in v4. Otherwise, we may need to 
qualify/clarify data file in many places.



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