gaborkaszab commented on PR #18503: URL: https://github.com/apache/iceberg/pull/18503#issuecomment-6589373722
> I think we can really just narrow this down to adding the `encryption-key` field (and i'd like to bundle in the root manifest size changes as well) > > I don't think we actually need to write any rules about how these are used except for saying the key_id is now optional? I don't think we need to specify that they are mutual exclusive or anything since their usage will be encryption specific. I think within the standard encryption scheme we are defining they are mutually exclusive (either V3 or V4 versions) but for a general writer I think they can do either or both. Not that it would matter much since a reader which understood setting both would also have their own allowances for both being set. Thanks @RussellSpitzer for taking a look! Before I clean this up following your recommendations, some observations: 1) "encryption spec covering the details": My view on the encryption spec is that it should be generic enough to avoid mentioning the table spec too many times. E.g. whether we store `key-id` and/or `encryption-key` in `Snapshot` is not the concern of the encryption spec, nor if these are mutually exclusive. It should only cover how to decrypt such a file that has an `encryption-key` IMO. 2) "Leaving flexibility to still use `key-id` for V4 snapshots": I'm not sure why we'd like to do that, because the `key-id` based approach relied on an assumption (keys can be reused across files) that was proven to be wrong, so the indirection of keeping them in a list is no longer needed. I think we are free to tell the writers that they have to write the `encryption-key` into `Snapshot` and not into `TableMetadata.encryption-keys`. WDYT? If you disagree, I can drop the excess details from the PR. -- 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]
