You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "adamreeve (via GitHub)" <gi...@apache.org> on 2024/03/26 23:33:56 UTC
[PR] MINOR: [C++] Remove misleading comment on FileKeyUnwrapper constructor [arrow]
adamreeve opened a new pull request, #40808:
URL: https://github.com/apache/arrow/pull/40808
### Rationale for this change
I added this comment in #34181, but from the discussion in https://github.com/apache/arrow/pull/40732#discussion_r1535001401, I realised this comment was incorrect. The extra overload appears to just be a convenience as a `FileKeyMaterialStore` is already constructed in `KeyToolkit::RotateMasterKeys`, but the store isn't actually used by the `FileKeyUnwrapper` in that method, as only `FileKeyUnwrapper::GetDataEncryptionKey` is called, which bypasses the store.
`RotateMasterKeys` does however rely on the `temp_key_material_store` passed to the `FileKeyWrapper` being used, which is possibly where this confusion came from.
### What changes are included in this PR?
Removes an incorrect statement from a C++ header comment.
### Are these changes tested?
NA
### Are there any user-facing changes?
No
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: [C++] Remove misleading comment on FileKeyUnwrapper constructor [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40808:
URL: https://github.com/apache/arrow/pull/40808#issuecomment-2024252290
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit dc2c5c66f5234a92169da76613399135786dbffb.
There were 3 benchmark results indicating a performance regression:
- Commit Run on `ursa-i9-9960x` at [2024-03-27 16:59:08Z](https://conbench.ursa.dev/compare/runs/e7e9aeb1c55d4b3390b8bc841a16a202...c310b55c33484dc4957660bfd3282864/)
- [`tpch` (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-03, scale_factor=1](https://conbench.ursa.dev/compare/benchmarks/0660440bbf1177598000eafb83334e43...0660463553e2795680000d769b7c83b6)
- [`tpch` (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-03, scale_factor=1](https://conbench.ursa.dev/compare/benchmarks/0660440c5c8672b18000676a8241ab24...0660463637517d528000f3b889d5c45a)
- and 1 more (see the report linked below)
The [full Conbench report](https://github.com/apache/arrow/runs/23180663883) has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: [C++] Remove misleading comment on FileKeyUnwrapper constructor [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #40808:
URL: https://github.com/apache/arrow/pull/40808
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org