MINOR: [C++][Parquet] Fix some comments and style in Statistics #33996
Conversation
|
@pitrou Mind take a look? This patch is small and didn't change any impl logics... |
pitrou
changed the title
|
@wgtmac @emkornfield Would you like to comment on this? The changes look fine to me on the principle. |
Looking good on my side. |
|
@pitrou @wjones127 Mind take a look? |
|
Benchmark runs are scheduled for baseline = 39bad54 and contender = 3e7b7e0. 3e7b7e0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
['Python', 'R'] benchmarks have high level of regressions. |
…he#33996) ### Rationale for this change Statistics uses some virtual functions in constructor, so here I change it to explicit call rather than implicit virtual fn call. And I also fixed some comments typo ### What changes are included in this PR? Some code style and comment fix in parquet statistics ### Are these changes tested? No, and no tests are need. ### Are there any user-facing changes? no Authored-by: mwish <maplewish117@gmail.com> Signed-off-by: Will Jones <willjones127@gmail.com>
|
Apologies, seems OK to me, not clear exactly why we prefer full references to the static methods? |
Personally it's confusing. Calling "virtual" method in dtor and ctor is not virtual. But user is easy to take it as virtual. |
…he#33996) ### Rationale for this change Statistics uses some virtual functions in constructor, so here I change it to explicit call rather than implicit virtual fn call. And I also fixed some comments typo ### What changes are included in this PR? Some code style and comment fix in parquet statistics ### Are these changes tested? No, and no tests are need. ### Are there any user-facing changes? no Authored-by: mwish <maplewish117@gmail.com> Signed-off-by: Will Jones <willjones127@gmail.com>
…he#33996) ### Rationale for this change Statistics uses some virtual functions in constructor, so here I change it to explicit call rather than implicit virtual fn call. And I also fixed some comments typo ### What changes are included in this PR? Some code style and comment fix in parquet statistics ### Are these changes tested? No, and no tests are need. ### Are there any user-facing changes? no Authored-by: mwish <maplewish117@gmail.com> Signed-off-by: Will Jones <willjones127@gmail.com>
Rationale for this change
Statistics uses some virtual functions in constructor, so here I change it to explicit call rather than implicit virtual fn call. And I also fixed some comments typo
What changes are included in this PR?
Some code style and comment fix in parquet statistics
Are these changes tested?
No, and no tests are need.
Are there any user-facing changes?
no