Skip to content

Commit 90a43f6

Browse files
committed
fix review 2
1 parent 705ce20 commit 90a43f6

8 files changed

Lines changed: 18 additions & 23 deletions

src/paimon/format/avro/avro_file_format_factory.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,10 @@ const char AvroFileFormatFactory::IDENTIFIER[] = "avro";
2727

2828
Result<std::unique_ptr<FileFormat>> AvroFileFormatFactory::Create(
2929
const std::map<std::string, std::string>& options) const {
30-
RegisterLogicalTypes();
3130
return std::make_unique<AvroFileFormat>(options);
3231
}
3332

34-
void AvroFileFormatFactory::RegisterLogicalTypes() {
33+
static __attribute__((constructor)) void AvroFileFormatFactoryRegisterLogicalTypes() {
3534
::avro::CustomLogicalTypeRegistry::instance().registerType(
3635
"map", [](const std::string&) { return std::make_shared<MapLogicalType>(); });
3736
}

src/paimon/format/avro/avro_file_format_factory.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ class AvroFileFormatFactory : public FileFormatFactory {
4141

4242
Result<std::unique_ptr<FileFormat>> Create(
4343
const std::map<std::string, std::string>& options) const override;
44-
45-
private:
46-
static void RegisterLogicalTypes();
4744
};
4845

4946
} // namespace paimon::avro

src/paimon/format/avro/avro_format_writer.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,34 +39,33 @@
3939
namespace arrow {
4040
class Array;
4141
} // namespace arrow
42-
namespace avro {
43-
class OutputStream;
44-
} // namespace avro
4542
struct ArrowArray;
4643

4744
namespace paimon::avro {
4845

4946
AvroFormatWriter::AvroFormatWriter(
5047
const std::shared_ptr<::avro::DataFileWriter<::avro::GenericDatum>>& file_writer,
5148
const ::avro::ValidSchema& avro_schema, const std::shared_ptr<arrow::DataType>& data_type,
52-
std::unique_ptr<AvroAdaptor> adaptor)
49+
std::unique_ptr<AvroAdaptor> adaptor, AvroOutputStreamImpl* avro_output_stream)
5350
: writer_(file_writer),
5451
avro_schema_(avro_schema),
5552
data_type_(data_type),
56-
adaptor_(std::move(adaptor)) {}
53+
adaptor_(std::move(adaptor)),
54+
avro_output_stream_(avro_output_stream) {}
5755

5856
Result<std::unique_ptr<AvroFormatWriter>> AvroFormatWriter::Create(
59-
std::unique_ptr<::avro::OutputStream> out, const std::shared_ptr<arrow::Schema>& schema,
57+
std::unique_ptr<AvroOutputStreamImpl> out, const std::shared_ptr<arrow::Schema>& schema,
6058
const ::avro::Codec codec) {
6159
try {
6260
PAIMON_ASSIGN_OR_RAISE(::avro::ValidSchema avro_schema,
6361
AvroSchemaConverter::ArrowSchemaToAvroSchema(schema));
62+
AvroOutputStreamImpl* avro_output_stream = out.get();
6463
auto writer = std::make_shared<::avro::DataFileWriter<::avro::GenericDatum>>(
6564
std::move(out), avro_schema, DEFAULT_SYNC_INTERVAL, codec);
6665
auto data_type = arrow::struct_(schema->fields());
6766
auto adaptor = std::make_unique<AvroAdaptor>(data_type);
68-
return std::unique_ptr<AvroFormatWriter>(
69-
new AvroFormatWriter(writer, avro_schema, data_type, std::move(adaptor)));
67+
return std::unique_ptr<AvroFormatWriter>(new AvroFormatWriter(
68+
writer, avro_schema, data_type, std::move(adaptor), avro_output_stream));
7069
} catch (const ::avro::Exception& e) {
7170
return Status::Invalid(fmt::format("avro format writer create failed. {}", e.what()));
7271
} catch (const std::exception& e) {
@@ -92,6 +91,7 @@ Status AvroFormatWriter::Flush() {
9291

9392
Status AvroFormatWriter::Finish() {
9493
try {
94+
avro_output_stream_->FlushBuffer(); // we need flush buffer before close writer
9595
writer_->close();
9696
} catch (const ::avro::Exception& e) {
9797
return Status::Invalid(fmt::format("avro writer close failed. {}", e.what()));

src/paimon/format/avro/avro_format_writer.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222

2323
#include "arrow/api.h"
2424
#include "avro/DataFile.hh"
25-
#include "avro/Stream.hh"
2625
#include "avro/ValidSchema.hh"
2726
#include "paimon/format/avro/avro_adaptor.h"
27+
#include "paimon/format/avro/avro_output_stream_impl.h"
2828
#include "paimon/format/format_writer.h"
2929
#include "paimon/metrics.h"
3030
#include "paimon/result.h"
@@ -36,7 +36,6 @@ class Schema;
3636
} // namespace arrow
3737
namespace avro {
3838
class GenericDatum;
39-
class OutputStream;
4039
} // namespace avro
4140
namespace paimon {
4241
class Metrics;
@@ -49,7 +48,7 @@ namespace paimon::avro {
4948
class AvroFormatWriter : public FormatWriter {
5049
public:
5150
static Result<std::unique_ptr<AvroFormatWriter>> Create(
52-
std::unique_ptr<::avro::OutputStream> out, const std::shared_ptr<arrow::Schema>& schema,
51+
std::unique_ptr<AvroOutputStreamImpl> out, const std::shared_ptr<arrow::Schema>& schema,
5352
const ::avro::Codec codec);
5453

5554
Status AddBatch(ArrowArray* batch) override;
@@ -70,13 +69,14 @@ class AvroFormatWriter : public FormatWriter {
7069
AvroFormatWriter(
7170
const std::shared_ptr<::avro::DataFileWriter<::avro::GenericDatum>>& file_writer,
7271
const ::avro::ValidSchema& avro_schema, const std::shared_ptr<arrow::DataType>& data_type,
73-
std::unique_ptr<AvroAdaptor> adaptor);
72+
std::unique_ptr<AvroAdaptor> adaptor, AvroOutputStreamImpl* avro_output_stream);
7473

7574
std::shared_ptr<::avro::DataFileWriter<::avro::GenericDatum>> writer_;
7675
::avro::ValidSchema avro_schema_;
7776
std::shared_ptr<arrow::DataType> data_type_;
7877
std::shared_ptr<Metrics> metrics_;
7978
std::unique_ptr<AvroAdaptor> adaptor_;
79+
AvroOutputStreamImpl* avro_output_stream_;
8080
};
8181

8282
} // namespace paimon::avro

src/paimon/format/avro/avro_output_stream_impl.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ AvroOutputStreamImpl::AvroOutputStreamImpl(const std::shared_ptr<paimon::OutputS
3939
byte_count_(0) {}
4040

4141
AvroOutputStreamImpl::~AvroOutputStreamImpl() {
42-
FlushBuffer();
4342
pool_->Free(buffer_, buffer_size_);
4443
}
4544

src/paimon/format/avro/avro_output_stream_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ class AvroOutputStreamImpl : public ::avro::OutputStream {
4444
return byte_count_;
4545
}
4646

47-
private:
4847
void FlushBuffer();
4948

49+
private:
5050
std::shared_ptr<MemoryPool> pool_;
5151
const size_t buffer_size_;
5252
uint8_t* const buffer_;

src/paimon/format/avro/avro_schema_converter.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,16 +178,16 @@ Result<std::shared_ptr<arrow::DataType>> AvroSchemaConverter::GetArrowType(
178178
if (fields.size() != 2) {
179179
return Status::TypeError("invalid avro logical map struct fields size");
180180
}
181-
auto key_field = fields[0];
182-
key_field = key_field->WithNullable(false);
181+
auto key_field = fields[0]->WithNullable(false);
183182
auto value_field = fields[1];
184183
if (key_field->name() != "key" || value_field->name() != "value") {
185184
return Status::TypeError("invalid avro logical map struct field names");
186185
}
187186
return std::make_shared<arrow::MapType>(std::move(key_field), std::move(value_field));
188187
}
189188
default:
190-
return Status::Invalid("not support logical type: ", AvroUtils::ToString(logical_type));
189+
return Status::Invalid("invalid avro logical type: ",
190+
AvroUtils::ToString(logical_type));
191191
}
192192

193193
size_t subtype_count = avro_node->leaves();

src/paimon/format/orc/orc_adapter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ Result<std::shared_ptr<arrow::ArrayBuilder>> MakeArrowBuilder(
856856
arrow::MemoryPool* pool) {
857857
if (column_vector_batch->numElements == 0) {
858858
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::ArrayBuilder> builder,
859-
arrow::MakeBuilder(type));
859+
arrow::MakeBuilder(type, pool));
860860
return builder;
861861
}
862862
arrow::Type::type kind = type->id();

0 commit comments

Comments
 (0)