wgtmac commented on PR #1158: URL: https://github.com/apache/orc/pull/1158#issuecomment-1179883140
> 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). Good suggestion @dongjoon-hyun. We should be able to set nullptr to ReaderMetrics to disable it at run time. BTW, we can totally disable it at compile time if we have something like this: https://github.com/apache/impala/blob/master/be/src/util/runtime-profile-counters.h#L53 -- 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]
