Skip to content

Commit feccbfb

Browse files
committed
fix clang
1 parent da64ca3 commit feccbfb

24 files changed

Lines changed: 74 additions & 188 deletions

include/paimon/format/format_writer.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,7 @@
2222
#include "paimon/type_fwd.h"
2323

2424
struct ArrowArray;
25-
26-
namespace arrow {
27-
class Schema;
28-
} // namespace arrow
25+
struct ArrowSchema;
2926

3027
namespace paimon {
3128
/// File format writer, each writer corresponds to a data file.
@@ -74,9 +71,9 @@ class PAIMON_EXPORT FormatWriter {
7471

7572
/// Updates the schema used for serialization in the file footer.
7673
/// Must be called before Finish(). This allows per-field metadata
77-
/// (e.g., extend MAP metadata) to be written into the file.
74+
/// (e.g., shared-shredding MAP metadata) to be written into the file.
7875
/// The default implementation returns NotImplemented.
79-
virtual Status UpdateSchema(const std::shared_ptr<arrow::Schema>& /*schema*/) {
76+
virtual Status UpdateSchema(::ArrowSchema* /*schema*/) {
8077
return Status::NotImplemented("UpdateSchema is not supported by this format writer.");
8178
}
8279
};

src/paimon/common/data/shredding/map_shared_shredding_batch_converter.cpp

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,12 @@
3232
namespace paimon {
3333

3434
MapSharedShreddingBatchConverter::MapSharedShreddingBatchConverter(
35-
std::shared_ptr<arrow::Schema> logical_schema, std::shared_ptr<arrow::Schema> physical_schema,
35+
const std::shared_ptr<arrow::Schema>& logical_schema,
36+
const std::shared_ptr<arrow::Schema>& physical_schema,
3637
const std::map<int32_t, int32_t>& column_to_num_columns,
3738
const std::shared_ptr<MemoryPool>& pool)
38-
: logical_schema_(std::move(logical_schema)),
39-
physical_schema_(std::move(physical_schema)),
39+
: logical_schema_(logical_schema),
40+
physical_schema_(physical_schema),
4041
pool_(GetArrowPool(pool)) {
4142
for (const auto& [col_index, num_columns] : column_to_num_columns) {
4243
contexts_.emplace_back(col_index, num_columns);
@@ -106,18 +107,17 @@ Result<std::shared_ptr<arrow::Array>> MapSharedShreddingBatchConverter::ConvertO
106107
// Create StructBuilder from physical struct type — it owns all child builders.
107108
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::unique_ptr<arrow::ArrayBuilder> struct_builder_base,
108109
arrow::MakeBuilder(physical_struct_type, pool_.get()));
109-
arrow::StructBuilder* struct_builder =
110-
dynamic_cast<arrow::StructBuilder*>(struct_builder_base.get());
110+
auto* struct_builder = dynamic_cast<arrow::StructBuilder*>(struct_builder_base.get());
111111
PAIMON_CHECK_NOT_NULL(struct_builder,
112112
"MapSharedShreddingBatchConverter: failed to create StructBuilder");
113113
PAIMON_RETURN_NOT_OK_FROM_ARROW(struct_builder->Reserve(num_rows));
114114

115115
// Extract child builders: [field_mapping, col_0..K-1, overflow]
116-
arrow::ListBuilder* field_mapping_builder =
116+
auto* field_mapping_builder =
117117
dynamic_cast<arrow::ListBuilder*>(struct_builder->field_builder(0));
118118
PAIMON_CHECK_NOT_NULL(field_mapping_builder,
119119
"MapSharedShreddingBatchConverter: field_mapping is not a ListBuilder");
120-
arrow::Int32Builder* field_mapping_value_builder =
120+
auto* field_mapping_value_builder =
121121
dynamic_cast<arrow::Int32Builder*>(field_mapping_builder->value_builder());
122122
PAIMON_CHECK_NOT_NULL(
123123
field_mapping_value_builder,
@@ -135,11 +135,11 @@ Result<std::shared_ptr<arrow::Array>> MapSharedShreddingBatchConverter::ConvertO
135135
}
136136

137137
int32_t overflow_field_idx = 1 + num_cols;
138-
arrow::MapBuilder* overflow_builder =
138+
auto* overflow_builder =
139139
dynamic_cast<arrow::MapBuilder*>(struct_builder->field_builder(overflow_field_idx));
140140
PAIMON_CHECK_NOT_NULL(overflow_builder,
141141
"MapSharedShreddingBatchConverter: overflow is not a MapBuilder");
142-
arrow::Int32Builder* overflow_key_builder =
142+
auto* overflow_key_builder =
143143
dynamic_cast<arrow::Int32Builder*>(overflow_builder->key_builder());
144144
PAIMON_CHECK_NOT_NULL(overflow_key_builder,
145145
"MapSharedShreddingBatchConverter: overflow key is not Int32Builder");
@@ -257,13 +257,6 @@ Status MapSharedShreddingBatchConverter::AppendOverflow(
257257
return Status::OK();
258258
}
259259

260-
void MapSharedShreddingBatchConverter::Reset() {
261-
for (auto& context : contexts_) {
262-
context.dict.Reset();
263-
context.allocator.Reset();
264-
}
265-
}
266-
267260
MapSharedShreddingFieldMeta MapSharedShreddingBatchConverter::BuildFieldMeta(
268261
int32_t logical_col_index) const {
269262
for (const auto& context : contexts_) {

src/paimon/common/data/shredding/map_shared_shredding_batch_converter.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ class MapSharedShreddingBatchConverter {
5959
/// @param physical_schema The physical schema (MAP columns replaced with STRUCT).
6060
/// @param column_to_num_columns Map from logical column index to K.
6161
/// @param pool Paimon memory pool for Arrow allocations.
62-
MapSharedShreddingBatchConverter(std::shared_ptr<arrow::Schema> logical_schema,
63-
std::shared_ptr<arrow::Schema> physical_schema,
62+
MapSharedShreddingBatchConverter(const std::shared_ptr<arrow::Schema>& logical_schema,
63+
const std::shared_ptr<arrow::Schema>& physical_schema,
6464
const std::map<int32_t, int32_t>& column_to_num_columns,
6565
const std::shared_ptr<MemoryPool>& pool);
6666

@@ -69,10 +69,6 @@ class MapSharedShreddingBatchConverter {
6969
/// @return Owned physical ArrowArray (C ABI) with physical schema.
7070
Result<std::unique_ptr<ArrowArray>> Convert(ArrowArray* logical_batch);
7171

72-
/// Resets dict and allocator state for all shredding columns.
73-
/// Called when rolling to a new file.
74-
void Reset();
75-
7672
/// Builds MapSharedShreddingFieldMeta for one shredding column (by logical index).
7773
/// Called at file close to serialize metadata.
7874
MapSharedShreddingFieldMeta BuildFieldMeta(int32_t logical_col_index) const;

src/paimon/common/data/shredding/map_shared_shredding_batch_converter_test.cpp

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -105,75 +105,6 @@ TEST_F(MapSharedShreddingBatchConverterTest, BasicConversion) {
105105
ASSERT_EQ(expected_meta, converter.BuildFieldMeta(1));
106106
}
107107

108-
TEST_F(MapSharedShreddingBatchConverterTest, MultiConvertResetAndFileMeta) {
109-
auto logical_schema = arrow::schema({
110-
arrow::field("id", arrow::int32()),
111-
arrow::field("tags", arrow::map(arrow::utf8(), arrow::int64())),
112-
});
113-
std::map<int32_t, int32_t> column_to_num_columns = {{1, 3}};
114-
ASSERT_OK_AND_ASSIGN(auto physical_schema, MapSharedShreddingUtils::LogicalToPhysicalSchema(
115-
logical_schema, column_to_num_columns));
116-
MapSharedShreddingBatchConverter converter(logical_schema, physical_schema,
117-
column_to_num_columns, pool_);
118-
119-
auto logical_type = arrow::struct_(logical_schema->fields());
120-
auto physical_type = arrow::struct_(physical_schema->fields());
121-
122-
// --- File 1: two batches ---
123-
// Batch1: {x:1, y:2}
124-
RunConvert(&converter, logical_type, physical_type, R"([
125-
[1, [["x", 1], ["y", 2]]]
126-
])");
127-
// Batch2: {y:3, z:4, w:5, v:6} — 4 fields, K=3 -> 1 overflow
128-
auto actual2 = RunConvert(&converter, logical_type, physical_type, R"([
129-
[2, [["y", 3], ["z", 4], ["w", 5], ["v", 6]]]
130-
])");
131-
132-
// Verify batch2 output: y=fid1->col0, z=fid2->col1, w=fid3->col2, v=fid4->overflow
133-
auto expected2 = ArrayFromJSON(physical_type, R"([
134-
[2, [[1, 2, 3], 3, 4, 5, [[4, 6]]]]
135-
])")
136-
.ValueOrDie();
137-
AssertArrayEquals(expected2, actual2);
138-
139-
// File1 meta: x=0,y=1,z=2,w=3,v=4; K=3, max_row_width=4; v overflowed
140-
MapSharedShreddingFieldMeta expected_meta1;
141-
expected_meta1.name_to_id = {{"x", 0}, {"y", 1}, {"z", 2}, {"w", 3}, {"v", 4}};
142-
expected_meta1.field_to_columns = {{0, {0}}, {1, {0, 1}}, {2, {1}}, {3, {2}}};
143-
expected_meta1.overflow_field_set = {4};
144-
expected_meta1.num_columns = 3;
145-
expected_meta1.max_row_width = 4;
146-
ASSERT_EQ(expected_meta1, converter.BuildFieldMeta(1));
147-
148-
// --- Reset for File 2 ---
149-
converter.Reset();
150-
151-
// After reset, meta should be empty except num_columns (K is preserved)
152-
MapSharedShreddingFieldMeta empty_meta;
153-
empty_meta.num_columns = 3;
154-
ASSERT_EQ(empty_meta, converter.BuildFieldMeta(1));
155-
156-
// File 2: single batch with new keys — field_ids restart from 0
157-
auto actual3 = RunConvert(&converter, logical_type, physical_type, R"([
158-
[10, [["p", 100], ["q", 200]]]
159-
])");
160-
auto expected3 = ArrayFromJSON(physical_type, R"([
161-
[10, [[0, 1, -1], 100, 200, null, []]]
162-
])")
163-
.ValueOrDie();
164-
AssertArrayEquals(expected3, actual3);
165-
166-
MapSharedShreddingFieldMeta expected_meta2;
167-
expected_meta2.name_to_id = {{"p", 0}, {"q", 1}};
168-
expected_meta2.field_to_columns = {{0, {0}}, {1, {1}}};
169-
expected_meta2.num_columns = 3;
170-
expected_meta2.max_row_width = 2;
171-
ASSERT_EQ(expected_meta2, converter.BuildFieldMeta(1));
172-
173-
// GetShreddingColumnIndices unchanged across resets
174-
ASSERT_EQ(std::vector<int32_t>({1}), converter.GetShreddingColumnIndices());
175-
}
176-
177108
TEST_F(MapSharedShreddingBatchConverterTest, NestedValueStruct) {
178109
// MAP<STRING, STRUCT<x:INT32, y:DOUBLE>>, K=2
179110
auto value_type = arrow::struct_({

src/paimon/common/data/shredding/map_shared_shredding_column_allocator.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,6 @@ RowAllocation MapSharedShreddingColumnAllocator::AllocateRow(
4646
return result;
4747
}
4848

49-
void MapSharedShreddingColumnAllocator::Reset() {
50-
field_to_columns_.clear();
51-
overflow_field_set_.clear();
52-
max_row_width_ = 0;
53-
}
54-
5549
const std::map<int32_t, std::set<int32_t>>& MapSharedShreddingColumnAllocator::GetFieldToColumns()
5650
const {
5751
return field_to_columns_;

src/paimon/common/data/shredding/map_shared_shredding_column_allocator.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ struct RowAllocation {
3838
/// Allocates MAP field ids to K physical columns on a per-row basis,
3939
/// and accumulates file-level metadata (field_to_columns, overflow_field_set, max_row_width).
4040
///
41-
/// This is a fake (trivial) implementation: each row simply assigns columns 0..min(N,K)-1
41+
/// This is a trivial implementation: each row simply assigns columns 0..min(N,K)-1
4242
/// in order, with no LRU eviction. The real strategy will be implemented separately.
43+
/// TODO(jinli.zjw): support LRU
4344
class MapSharedShreddingColumnAllocator {
4445
public:
4546
/// @param num_columns Number of physical columns K for this shared-shredding MAP column.
@@ -50,9 +51,6 @@ class MapSharedShreddingColumnAllocator {
5051
/// @return Allocation result with column assignments and overflow list.
5152
RowAllocation AllocateRow(const std::vector<int32_t>& field_ids);
5253

53-
/// Resets all state. Called when rolling to a new file.
54-
void Reset();
55-
5654
/// Returns accumulated field_id -> set of column indices (for MapSharedShreddingFileMeta).
5755
const std::map<int32_t, std::set<int32_t>>& GetFieldToColumns() const;
5856

src/paimon/common/data/shredding/map_shared_shredding_column_allocator_test.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -108,26 +108,6 @@ TEST(MapSharedShreddingColumnAllocatorTest, OverflowFieldSetAccumulated) {
108108
ASSERT_EQ(std::set<int32_t>({3, 6, 7}), overflow_set);
109109
}
110110

111-
TEST(MapSharedShreddingColumnAllocatorTest, ResetClearsAll) {
112-
MapSharedShreddingColumnAllocator allocator(2);
113-
114-
allocator.AllocateRow({1, 2, 3});
115-
ASSERT_EQ(3, allocator.GetMaxRowWidth());
116-
ASSERT_FALSE(allocator.GetFieldToColumns().empty());
117-
ASSERT_FALSE(allocator.GetOverflowFieldSet().empty());
118-
119-
allocator.Reset();
120-
ASSERT_EQ(0, allocator.GetMaxRowWidth());
121-
ASSERT_TRUE(allocator.GetFieldToColumns().empty());
122-
ASSERT_TRUE(allocator.GetOverflowFieldSet().empty());
123-
124-
// Works correctly after reset
125-
auto result = allocator.AllocateRow({10});
126-
ASSERT_EQ(2u, result.col_to_field.size());
127-
ASSERT_EQ(10, result.col_to_field[0]);
128-
ASSERT_EQ(-1, result.col_to_field[1]);
129-
}
130-
131111
TEST(MapSharedShreddingColumnAllocatorTest, GetNumColumns) {
132112
MapSharedShreddingColumnAllocator allocator(5);
133113
ASSERT_EQ(5, allocator.GetNumColumns());

src/paimon/common/data/shredding/map_shared_shredding_context.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@
1717
#include "paimon/common/data/shredding/map_shared_shredding_context.h"
1818

1919
#include <algorithm>
20-
#include <utility>
2120

2221
namespace paimon {
2322

24-
MapSharedShreddingContext::MapSharedShreddingContext(std::map<int32_t, int32_t> column_to_k_max)
25-
: column_to_k_max_(std::move(column_to_k_max)) {}
23+
MapSharedShreddingContext::MapSharedShreddingContext(
24+
const std::map<int32_t, int32_t>& column_to_k_max)
25+
: column_to_k_max_(column_to_k_max) {}
2626

2727
std::map<int32_t, int32_t> MapSharedShreddingContext::ComputeNextK() const {
2828
std::map<int32_t, int32_t> result;

src/paimon/common/data/shredding/map_shared_shredding_context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace paimon {
3333
class MapSharedShreddingContext {
3434
public:
3535
/// @param column_to_k_max Map from logical column index to its K_max (from options).
36-
explicit MapSharedShreddingContext(std::map<int32_t, int32_t> column_to_k_max);
36+
explicit MapSharedShreddingContext(const std::map<int32_t, int32_t>& column_to_k_max);
3737

3838
/// Returns the K to use for each extend column in the next file.
3939
/// First file returns K_max for all columns; subsequent files adapt

src/paimon/common/data/shredding/map_shared_shredding_field_dict.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ class MapSharedShreddingFieldDict {
5252
return next_id_;
5353
}
5454

55-
/// Resets all state. Called when rolling to a new file.
56-
void Reset() {
57-
name_to_id_.clear();
58-
next_id_ = 0;
59-
}
60-
6155
private:
6256
std::map<std::string, int32_t> name_to_id_;
6357
int32_t next_id_ = 0;

0 commit comments

Comments
 (0)