rdblue commented on code in PR #18138:
URL: https://github.com/apache/iceberg/pull/18138#discussion_r4030146082
##########
core/src/main/java/org/apache/iceberg/ManifestInfo.java:
##########
@@ -56,12 +55,6 @@ interface ManifestInfo {
Types.NestedField DV =
Types.NestedField.optional(
522, "dv", Types.BinaryType.get(), "Deletion vector for manifest
entries");
- Types.NestedField DV_CARDINALITY =
Review Comment:
This is a good thing to think through. Thanks for pointing it out.
I don't think that we want to keep cardinality because it is easy to recover
from each bitmap when you're working with a manifest, by calling
`manifest.manifestDeletionVector().cardinality()` rather than
`manifest.cardinality()` directly (assuming that we're working with
`ManfiestFile`). That just reads the first 6 bytes, if a DV is present.
Otherwise the DV is lazy.
I think the trade-off is that you have to actually read the DV bytes instead
of reading the cardinality and row count when scanning the root manifest. But
if you did identify a leaf manifest that needed to be compacted, you'd just go
and re-scan the root manifest to recover the DV so you can rewrite the file. So
I think the case where you'd scan the root twice isn't very realistic.
--
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]
