You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by GitBox <gi...@apache.org> on 2022/07/12 02:56:40 UTC

[GitHub] [orc] coderex2522 commented on pull request #1158: ORC-961: [C++] expose related metrics of the reader

coderex2522 commented on PR #1158:
URL: https://github.com/apache/orc/pull/1158#issuecomment-1181256803

   > Thank you, @coderex2522 and @wgtmac .
   > 
   > I have three questions as the final pieces:
   > 
   > * Backward-compatibility: Apache ORC C++ API is used in the downstream. So, although this PR looks good, it would be great to make it sure that we don't break the backward compatibility once more. I believe the default value of new parameters could reduce the pain.
   > * The ability to disable this metrics. According to the code, we can disable this feature by hand over NULL pointer? Could you double-confirm it? Could you add a test coverage to confirm that this case is possible? It will be helpful to prevent accidentally segfault in the future.
   > * Last question is about the target version. For this PR, if you want, you can backport this to branch-1.8 for Apache ORC 1.8.0. Which one do you prefer v1.9.0 (Next year) or v1.8.0 (This year).
   
   * External projects will depend on the api interface in the include directory and need to maintain compatibility, while the header files in the src directory will not be dependent on external projects, so I prefer not to add the default value.
   * I will add the new test to coverage the disable metrics case.
   * The target version v1.9.0 is fine, I have no requirement to support in branch-1.8.


-- 
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: issues-unsubscribe@orc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org