Skip to content

Commit ab171b2

Browse files
HuaHuaYHuaHuaY
authored
GH-51245: [C++][Parquet][FOLLOWUP] Add support for LLVM 23.1 (#51468)
### Rationale for this change Add support for LLVM 23.1. My mistake. In the previous PR, I used the `ninja-debug-gandiva` CMake preset but didn't notice that some Parquet tests weren't being compiled; consequently, certain compilation errors within the Parquet component went undetected. This time I used the `ninja-debug` preset. I'm not sure if there are compilation errors in other presets—I don't use them often, and CI doesn't check them either—so we'll just fix them if we run into them. ### What changes are included in this PR? Remove static from some template functions in header files because LLVM 23 added `-Wunused-template` to `-Wall`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51245 Authored-by: Zehua Zou <zehuazou2000@gmail.com> Signed-off-by: Zehua Zou <zehuazou2000@gmail.com>
1 parent 13f20f2 commit ab171b2

3 files changed

Lines changed: 32 additions & 71 deletions

File tree

‎cpp/src/parquet/arrow/test_util.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ ::arrow::enable_if_fixed_size_binary<ArrowType, Status> NonNullArray(
141141
}
142142

143143
template <int32_t byte_width>
144-
static void random_decimals(int64_t n, uint32_t seed, int32_t precision, uint8_t* out) {
144+
void random_decimals(int64_t n, uint32_t seed, int32_t precision, uint8_t* out) {
145145
auto gen = ::arrow::random::RandomArrayGenerator(seed);
146146
std::shared_ptr<Array> decimals;
147147
if constexpr (byte_width == 4) {

‎cpp/src/parquet/column_reader_test.cc‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -48,38 +48,6 @@ using testing::ElementsAre;
4848

4949
namespace test {
5050

51-
template <typename T>
52-
static inline bool vector_equal_with_def_levels(const std::vector<T>& left,
53-
const std::vector<int16_t>& def_levels,
54-
int16_t max_def_levels,
55-
int16_t max_rep_levels,
56-
const std::vector<T>& right) {
57-
size_t i_left = 0;
58-
size_t i_right = 0;
59-
for (size_t i = 0; i < def_levels.size(); i++) {
60-
if (def_levels[i] == max_def_levels) {
61-
// Compare
62-
if (left[i_left] != right[i_right]) {
63-
std::cerr << "index " << i << " left was " << left[i_left] << " right was "
64-
<< right[i] << std::endl;
65-
return false;
66-
}
67-
i_left++;
68-
i_right++;
69-
} else if (def_levels[i] == (max_def_levels - 1)) {
70-
// Null entry on the lowest nested level
71-
i_right++;
72-
} else if (def_levels[i] < (max_def_levels - 1)) {
73-
// Null entry on a higher nesting level, only supported for non-repeating data
74-
if (max_rep_levels == 0) {
75-
i_right++;
76-
}
77-
}
78-
}
79-
80-
return true;
81-
}
82-
8351
class TestPrimitiveReader : public ::testing::Test {
8452
public:
8553
void InitReader(const ColumnDescriptor* d) {

‎cpp/src/parquet/test_util.h‎

Lines changed: 31 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ std::string get_bad_data_dir();
7676
std::string get_data_file(const std::string& filename, bool is_good = true);
7777

7878
template <typename T>
79-
static inline void assert_vector_equal(const std::vector<T>& left,
80-
const std::vector<T>& right) {
79+
inline void assert_vector_equal(const std::vector<T>& left, const std::vector<T>& right) {
8180
ASSERT_EQ(left.size(), right.size());
8281

8382
for (size_t i = 0; i < left.size(); ++i) {
@@ -86,7 +85,7 @@ static inline void assert_vector_equal(const std::vector<T>& left,
8685
}
8786

8887
template <typename T>
89-
static inline bool vector_equal(const std::vector<T>& left, const std::vector<T>& right) {
88+
inline bool vector_equal(const std::vector<T>& left, const std::vector<T>& right) {
9089
if (left.size() != right.size()) {
9190
return false;
9291
}
@@ -103,7 +102,7 @@ static inline bool vector_equal(const std::vector<T>& left, const std::vector<T>
103102
}
104103

105104
template <typename T>
106-
static std::vector<T> slice(const std::vector<T>& values, int start, int end) {
105+
std::vector<T> slice(const std::vector<T>& values, int start, int end) {
107106
if (end < start) {
108107
return std::vector<T>(0);
109108
}
@@ -176,21 +175,20 @@ std::shared_ptr<Buffer> EncodeValues(Encoding::type encoding, bool use_dictionar
176175
}
177176

178177
template <typename T>
179-
static void InitValues(int num_values, uint32_t seed, std::vector<T>& values,
180-
std::vector<uint8_t>& buffer) {
178+
void InitValues(int num_values, uint32_t seed, std::vector<T>& values,
179+
std::vector<uint8_t>& buffer) {
181180
random_numbers(num_values, seed, std::numeric_limits<T>::min(),
182181
std::numeric_limits<T>::max(), values.data());
183182
}
184183

185184
template <typename T>
186-
static void InitValues(int num_values, std::vector<T>& values,
187-
std::vector<uint8_t>& buffer) {
185+
void InitValues(int num_values, std::vector<T>& values, std::vector<uint8_t>& buffer) {
188186
InitValues(num_values, 0, values, buffer);
189187
}
190188

191189
template <typename T>
192-
static void InitDictValues(int num_values, int num_dicts, std::vector<T>& values,
193-
std::vector<uint8_t>& buffer) {
190+
void InitDictValues(int num_values, int num_dicts, std::vector<T>& values,
191+
std::vector<uint8_t>& buffer) {
194192
int repeat_factor = num_values / num_dicts;
195193
InitValues<T>(num_dicts, values, buffer);
196194
// add some repeated values
@@ -347,7 +345,7 @@ inline void DataPageBuilder<BooleanType>::AppendValues(const ColumnDescriptor* d
347345
}
348346

349347
template <typename Type>
350-
static std::shared_ptr<DataPageV1> MakeDataPage(
348+
std::shared_ptr<DataPageV1> MakeDataPage(
351349
const ColumnDescriptor* d, const std::vector<typename Type::c_type>& values,
352350
int num_vals, Encoding::type encoding, const uint8_t* indices, int indices_size,
353351
const std::vector<int16_t>& def_levels, int16_t max_def_level,
@@ -441,7 +439,7 @@ inline std::shared_ptr<Buffer> DictionaryPageBuilder<BooleanType>::AppendValues(
441439
}
442440

443441
template <typename Type>
444-
inline static std::shared_ptr<DictionaryPage> MakeDictPage(
442+
inline std::shared_ptr<DictionaryPage> MakeDictPage(
445443
const ColumnDescriptor* d, const std::vector<typename Type::c_type>& values,
446444
const std::vector<int>& values_per_page, Encoding::type encoding,
447445
std::vector<std::shared_ptr<Buffer>>& rle_indices) {
@@ -463,15 +461,13 @@ inline static std::shared_ptr<DictionaryPage> MakeDictPage(
463461

464462
// Given def/rep levels and values create multiple dict pages
465463
template <typename Type>
466-
inline static void PaginateDict(const ColumnDescriptor* d,
467-
const std::vector<typename Type::c_type>& values,
468-
const std::vector<int16_t>& def_levels,
469-
int16_t max_def_level,
470-
const std::vector<int16_t>& rep_levels,
471-
int16_t max_rep_level, int num_levels_per_page,
472-
const std::vector<int>& values_per_page,
473-
std::vector<std::shared_ptr<Page>>& pages,
474-
Encoding::type encoding = Encoding::RLE_DICTIONARY) {
464+
inline void PaginateDict(const ColumnDescriptor* d,
465+
const std::vector<typename Type::c_type>& values,
466+
const std::vector<int16_t>& def_levels, int16_t max_def_level,
467+
const std::vector<int16_t>& rep_levels, int16_t max_rep_level,
468+
int num_levels_per_page, const std::vector<int>& values_per_page,
469+
std::vector<std::shared_ptr<Page>>& pages,
470+
Encoding::type encoding = Encoding::RLE_DICTIONARY) {
475471
int num_pages = static_cast<int>(values_per_page.size());
476472
std::vector<std::shared_ptr<Buffer>> rle_indices;
477473
std::shared_ptr<DictionaryPage> dict_page =
@@ -501,15 +497,14 @@ inline static void PaginateDict(const ColumnDescriptor* d,
501497

502498
// Given def/rep levels and values create multiple plain pages
503499
template <typename Type>
504-
static inline void PaginatePlain(const ColumnDescriptor* d,
505-
const std::vector<typename Type::c_type>& values,
506-
const std::vector<int16_t>& def_levels,
507-
int16_t max_def_level,
508-
const std::vector<int16_t>& rep_levels,
509-
int16_t max_rep_level, int num_levels_per_page,
510-
const std::vector<int>& values_per_page,
511-
std::vector<std::shared_ptr<Page>>& pages,
512-
Encoding::type encoding = Encoding::PLAIN) {
500+
inline void PaginatePlain(const ColumnDescriptor* d,
501+
const std::vector<typename Type::c_type>& values,
502+
const std::vector<int16_t>& def_levels, int16_t max_def_level,
503+
const std::vector<int16_t>& rep_levels, int16_t max_rep_level,
504+
int num_levels_per_page,
505+
const std::vector<int>& values_per_page,
506+
std::vector<std::shared_ptr<Page>>& pages,
507+
Encoding::type encoding = Encoding::PLAIN) {
513508
int num_pages = static_cast<int>(values_per_page.size());
514509
int def_level_start = 0;
515510
int def_level_end = 0;
@@ -537,14 +532,12 @@ static inline void PaginatePlain(const ColumnDescriptor* d,
537532

538533
// Generates pages from randomly generated data
539534
template <typename Type>
540-
static inline int MakePages(const ColumnDescriptor* d, int num_pages, int levels_per_page,
541-
std::vector<int16_t>& def_levels,
542-
std::vector<int16_t>& rep_levels,
543-
std::vector<typename Type::c_type>& values,
544-
std::vector<uint8_t>& buffer,
545-
std::vector<std::shared_ptr<Page>>& pages,
546-
Encoding::type encoding = Encoding::PLAIN,
547-
uint32_t seed = 0) {
535+
inline int MakePages(const ColumnDescriptor* d, int num_pages, int levels_per_page,
536+
std::vector<int16_t>& def_levels, std::vector<int16_t>& rep_levels,
537+
std::vector<typename Type::c_type>& values,
538+
std::vector<uint8_t>& buffer,
539+
std::vector<std::shared_ptr<Page>>& pages,
540+
Encoding::type encoding = Encoding::PLAIN, uint32_t seed = 0) {
548541
int num_levels = levels_per_page * num_pages;
549542
int num_values = 0;
550543
int16_t zero = 0;

0 commit comments

Comments
 (0)