Repository navigation
Fix PAX build failure with GCC 15 - #2081
Conversation
leborchuk
left a comment
leborchuk
left a comment
There was a problem hiding this comment.
LGTM, checked - it really fails with
/home/xifos/git/cloudberry-leborchuk/contrib/pax_storage/src/cpp/storage/columns/pax_encoding_column.h: In instantiation of ‘class pax::PaxEncodingColumn<long int>’: /home/xifos/git/cloudberry-leborchuk/contrib/pax_storage/src/cpp/storage/columns/pax_encoding_column.h:79:23: required from here /home/xifos/git/cloudberry-leborchuk/contrib/pax_storage/src/cpp/storage/columns/pax_column.h:448:29: error: ‘std::pair<char, long unsigned int> pax::PaxCommColumn<T>::GetBuffer(size_t) [with T = long int; size_t = long unsigned int]’ was hidden [-Werror=overloaded-virtual] 448 | std::pair<char , size_t> GetBuffer(size_t position) override; | ^~~~~~~~~ In file included from /home/xifos/git/cloudberry-leborchuk/contrib/pax_storage/src/cpp/storage/columns/pax_encoding_column.cc:28: /home/xifos/git/cloudberry-leborchuk/contrib/pax_storage/src/cpp/storage/columns/pax_encoding_column.h:49:29: note: by ‘std::pair<char, long unsigned int> pax::PaxEncodingColumn<T>::GetBuffer() [with T = long int]’ 49 | std::pair<char *, size_t> GetBuffer() override;
The issue is in using multiple functions GetBuffer in class (multiple classes):
virtual std::pair<char *, size_t> GetBuffer() = 0;
virtual std::pair<char *, size_t> GetBuffer(size_t position) = 0;
PaxCommColumn overrides both. But PaxEncodingColumn overrides only the no-arg one.
And message is about it - we overloaded only one function.
It is just warning, no real bug here. It could be easily fixed - just use https://en.cppreference.com/cpp/language/using_declaration
code should looks like
using PaxCommColumn<T>::GetBuffer; // keep base GetBuffer(size_t) visible
std::pair<char *, size_t> GetBuffer() override;
I suggest create issue to fix all warnings in PAX.
Disable option is good for me.
1542baf to
3594dbc
Compare
|
Updates: The new commit fixes the cause instead of silencing the warning. (An earlier version of 1.
2.
There is no change in behavior, and |
|
Hi @leborchuk PTAL. Thanks! |
GCC 15 enables -Woverloaded-virtual under -Wall. PAX builds with -Werror, so the build failed, and a build with the warning only downgraded still printed dozens of warnings. The cause is a derived class that declares one overload of a function and so hides the other overloads of the base class. Fix the cause instead of silencing the warning. PaxColumn declares both GetBuffer() and GetBuffer(size_t position), but many column classes override only one of them. Add "using Base::GetBuffer;" to those classes: PaxEncodingColumn, PaxNonFixedEncodingColumn, PaxVecEncodingColumn, PaxVecNonFixedEncodingColumn, PaxVecBitPackedColumn, PaxVecBpCharColumn and PaxShortNumericColumn. PaxColumns::SetExternalToastDataBuffer(data, column_sizes) is not an override of PaxColumn::SetExternalToastDataBuffer(data). It stores the buffer and gives each column its part of it, while the base version only stores it. A using-declaration would expose the base version on PaxColumns, which would skip the per-column part. Rename it to DistributeExternalToastDataBuffer instead. There is no change in behavior. The fix is checked with -Woverloaded-virtual=2 and -Werror on Ubuntu 26.04 (GCC 15.2): the build had 257 diagnostics at 9 places before, and has none now. The 697 PAX unit tests pass in a debug build. Assisted-by: Claude Code Backpatch-through: REL_2_STABLE
The previous commit renamed PaxColumns::SetExternalToastDataBuffer(data, column_sizes) to DistributeExternalToastDataBuffer to stop it hiding PaxColumn::SetExternalToastDataBuffer(data). The rename is not needed, and a rename does not keep anyone from calling the base version on a PaxColumns either. Keep the original name and add "using PaxColumn::SetExternalToastDataBuffer;" to PaxColumns, the same as the GetBuffer cases. The definition and the call site in orc_format_reader.cc are back to what they were. There is no change in behavior. Checked with -Woverloaded-virtual=2 and -Werror on Ubuntu 26.04 (GCC 15.2): no diagnostics, and the 697 PAX unit tests pass in a debug build. Assisted-by: Claude Code Backpatch-through: REL_2_STABLE
2de1590 to
56e9fa4
Compare
4 participants
GCC 15 enables -Woverloaded-virtual=1 under -Wall, and PAX builds with -Werror, so the build fails in pax_column.h and
pax_vec_encoding_column.h where a derived class hides a virtual function of its base class.
Add -Wno-error=overloaded-virtual to the GCC flags, as the APPLE (clang) branch already does. The warning is still reported, but no longer fails the build.
Found when building Cloudberry on Ubuntu 26.04, which ships GCC 15.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions