Core: Extend V4 DeletionVector with key_metadata field - #17438
Core: Extend V4 DeletionVector with key_metadata field#17438gaborkaszab wants to merge 1 commit into
Conversation
| "Number of set bits (deleted rows) in the vector"); | ||
| Types.NestedField KEY_METADATA = | ||
| Types.NestedField.optional( | ||
| 149, |
There was a problem hiding this comment.
ColumnFiles PR takes field IDs 158-169. I see 149 is free
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object other) { |
There was a problem hiding this comment.
I haven't added key_metadata here. I think we should rather drop equals() and hashCode. We agreed such metadata structs shouldn't override these. Will remove them in a follow-up PR.
anoopj
left a comment
anoopj
left a comment
There was a problem hiding this comment.
Can we also update the spec PR @amogh-jahagirdar is working on and the AMT proposal doc?
| long cardinality(); | ||
|
|
||
| /** Returns encryption key metadata, or null if the file is not encrypted. */ | ||
| ByteBuffer keyMetadata(); |
There was a problem hiding this comment.
I think we need to update the adapter TrackedDVDeleteFile to return the key metadata? (right now it just hard returns null).
There was a problem hiding this comment.
I initially figured I just take care of the schema and structure changes and deal with the rest on top later. Since the change in the adapter is minimal, I covered it too as you suggested. Thanks, @anoopj !
95fb136 to
908a23a
Compare
gaborkaszab
left a comment
gaborkaszab
left a comment
There was a problem hiding this comment.
Thanks for taking a look, @anoopj !
@amogh-jahagirdar would you mind checking this to be in sync with the spec changes you have in mind?
| long cardinality(); | ||
|
|
||
| /** Returns encryption key metadata, or null if the file is not encrypted. */ | ||
| ByteBuffer keyMetadata(); |
There was a problem hiding this comment.
I initially figured I just take care of the schema and structure changes and deal with the rest on top later. Since the change in the adapter is minimal, I covered it too as you suggested. Thanks, @anoopj !
2 participants
No description provided.