Skip to content

Commit 251e9f5

Browse files
zjw1111zjw1111
authored
test: improve ut coverage for avro (#138)
* chore: fix typo * chore: optimize status output format * feat: add ut for avro
1 parent e2b24bf commit 251e9f5

20 files changed

Lines changed: 764 additions & 57 deletions

‎docs/source/user_guide/data_types.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ and `Arrow DataTypes <https://arrow.apache.org/docs/format/Columnar.html#data-ty
158158

159159
This type fills the gap between time zone free and time zone mandatory
160160
timestamp types by allowing the interpretation of UTC timestamps according
161-
to the configured session time zone. A conversion from and to int describes
161+
to the configured session time zone. A conversion from and to int describes
162162
the number of seconds since epoch. A conversion from and to long describes the number of milliseconds since epoch.
163163

164164
* - ``ARRAY<t>``

‎src/paimon/common/utils/string_utils.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,11 @@ Result<int32_t> StringUtils::StringToDate(const std::string& str) {
138138
std::istringstream ss(str);
139139
ss >> std::get_time(&timeinfo, "%Y-%m-%d");
140140
if (ss.fail()) {
141-
return Status::Invalid(fmt::format("failed to convert string {} to date", str));
141+
return Status::Invalid(fmt::format("failed to convert string '{}' to date", str));
142142
}
143143
std::time_t time = timegm(&timeinfo);
144144
if (time == -1) {
145-
return Status::Invalid(fmt::format("failed to convert string {} to date", str));
145+
return Status::Invalid(fmt::format("failed to convert string '{}' to date", str));
146146
}
147147
static const int64_t SECONDS_PER_DAY = 86400l; // = 24 * 60 * 60
148148
return time / SECONDS_PER_DAY;

‎src/paimon/core/casting/cast_executor_test.cpp‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ TEST_F(CastExecutorTest, TestStringToBooleanCastExecutorCastLiteral) {
837837
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
838838
src_data, arrow::boolean());
839839
ASSERT_TRUE(
840-
msg.find("StringToBooleanCastExecutor cast failed: STRING cannot cast to BOOLEAN") !=
840+
msg.find("StringToBooleanCastExecutor cast failed: STRING '' cannot cast to BOOLEAN") !=
841841
std::string::npos);
842842
}
843843
{
@@ -847,7 +847,7 @@ TEST_F(CastExecutorTest, TestStringToBooleanCastExecutorCastLiteral) {
847847
src_data, arrow::boolean());
848848
ASSERT_TRUE(
849849
msg.find(
850-
"StringToBooleanCastExecutor cast failed: STRING ttrue cannot cast to BOOLEAN") !=
850+
"StringToBooleanCastExecutor cast failed: STRING 'ttrue' cannot cast to BOOLEAN") !=
851851
std::string::npos);
852852
}
853853
}
@@ -864,7 +864,7 @@ TEST_F(CastExecutorTest, TestStringToBooleanCastExecutorCastArray) {
864864
auto msg =
865865
CheckArrayInvalidResult(cast_executor, arrow::utf8(), arrow::boolean(), R"([""])");
866866
ASSERT_TRUE(
867-
msg.find("StringToBooleanCastExecutor cast failed: STRING cannot cast to BOOLEAN") !=
867+
msg.find("StringToBooleanCastExecutor cast failed: STRING '' cannot cast to BOOLEAN") !=
868868
std::string::npos);
869869
}
870870
{
@@ -873,7 +873,7 @@ TEST_F(CastExecutorTest, TestStringToBooleanCastExecutorCastArray) {
873873
R"(["true", "ttrue"])");
874874
ASSERT_TRUE(
875875
msg.find(
876-
"StringToBooleanCastExecutor cast failed: STRING ttrue cannot cast to BOOLEAN") !=
876+
"StringToBooleanCastExecutor cast failed: STRING 'ttrue' cannot cast to BOOLEAN") !=
877877
std::string::npos);
878878
}
879879
}
@@ -940,57 +940,57 @@ TEST_F(CastExecutorTest, TestStringToNumericPrimitiveCastExecutorCastLiteral) {
940940
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
941941
src_data, arrow::int8());
942942
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
943-
"cast 128 from STRING to TINYINT") != std::string::npos);
943+
"cast '128' from STRING to TINYINT") != std::string::npos);
944944
}
945945
{
946946
std::string src_data = "-129";
947947
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
948948
src_data, arrow::int8());
949949
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
950-
"cast -129 from STRING to TINYINT") != std::string::npos);
950+
"cast '-129' from STRING to TINYINT") != std::string::npos);
951951
}
952952
{
953953
std::string src_data = "32768";
954954
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
955955
src_data, arrow::int16());
956956
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
957-
"cast 32768 from STRING to SMALLINT") != std::string::npos);
957+
"cast '32768' from STRING to SMALLINT") != std::string::npos);
958958
}
959959
{
960960
std::string src_data = "-32769";
961961
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
962962
src_data, arrow::int16());
963963
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
964-
"cast -32769 from STRING to SMALLINT") != std::string::npos);
964+
"cast '-32769' from STRING to SMALLINT") != std::string::npos);
965965
}
966966
{
967967
std::string src_data = "2147483648";
968968
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
969969
src_data, arrow::int32());
970970
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
971-
"cast 2147483648 from STRING to INT") != std::string::npos);
971+
"cast '2147483648' from STRING to INT") != std::string::npos);
972972
}
973973
{
974974
std::string src_data = "-2147483649";
975975
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
976976
src_data, arrow::int32());
977977
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
978-
"cast -2147483649 from STRING to INT") != std::string::npos);
978+
"cast '-2147483649' from STRING to INT") != std::string::npos);
979979
}
980980
{
981981
std::string src_data = "9223372036854775808";
982982
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
983983
src_data, arrow::int64());
984984
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
985-
"cast 9223372036854775808 from STRING to BIGINT") !=
985+
"cast '9223372036854775808' from STRING to BIGINT") !=
986986
std::string::npos);
987987
}
988988
{
989989
std::string src_data = "-9223372036854775809";
990990
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
991991
src_data, arrow::int64());
992992
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
993-
"cast -9223372036854775809 from STRING to BIGINT") !=
993+
"cast '-9223372036854775809' from STRING to BIGINT") !=
994994
std::string::npos);
995995
}
996996
{
@@ -1011,14 +1011,14 @@ TEST_F(CastExecutorTest, TestStringToNumericPrimitiveCastExecutorCastLiteral) {
10111011
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
10121012
src_data, arrow::int16());
10131013
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
1014-
"cast from STRING to SMALLINT") != std::string::npos);
1014+
"cast '' from STRING to SMALLINT") != std::string::npos);
10151015
}
10161016
{
10171017
std::string src_data = "abc";
10181018
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
10191019
src_data, arrow::int32());
10201020
ASSERT_TRUE(msg.find("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
1021-
"cast abc from STRING to INT") != std::string::npos);
1021+
"cast 'abc' from STRING to INT") != std::string::npos);
10221022
}
10231023
}
10241024

@@ -1421,36 +1421,38 @@ TEST_F(CastExecutorTest, TestStringToDateCastExecutorCastLiteral) {
14211421
std::string src_data = "9223372036854775807";
14221422
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
14231423
src_data, arrow::date32());
1424-
ASSERT_TRUE(msg.find("failed to convert string 9223372036854775807 to date") !=
1424+
ASSERT_TRUE(msg.find("failed to convert string '9223372036854775807' to date") !=
14251425
std::string::npos);
14261426
}
14271427
{
14281428
// invalid date str
14291429
std::string src_data = "11970-01-02";
14301430
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
14311431
src_data, arrow::date32());
1432-
ASSERT_TRUE(msg.find("failed to convert string 11970-01-02 to date") != std::string::npos);
1432+
ASSERT_TRUE(msg.find("failed to convert string '11970-01-02' to date") !=
1433+
std::string::npos);
14331434
}
14341435
{
14351436
// invalid date str
14361437
std::string src_data = "-1970-01-02";
14371438
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
14381439
src_data, arrow::date32());
1439-
ASSERT_TRUE(msg.find("failed to convert string -1970-01-02 to date") != std::string::npos);
1440+
ASSERT_TRUE(msg.find("failed to convert string '-1970-01-02' to date") !=
1441+
std::string::npos);
14401442
}
14411443
{
14421444
// invalid date str
14431445
std::string src_data = "";
14441446
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
14451447
src_data, arrow::date32());
1446-
ASSERT_TRUE(msg.find("failed to convert string to date") != std::string::npos);
1448+
ASSERT_TRUE(msg.find("failed to convert string '' to date") != std::string::npos);
14471449
}
14481450
{
14491451
// invalid date str
14501452
std::string src_data = "0x1";
14511453
auto msg = CheckLiteralInvalidResult<std::string>(cast_executor, FieldType::STRING,
14521454
src_data, arrow::date32());
1453-
ASSERT_TRUE(msg.find("failed to convert string 0x1 to date") != std::string::npos);
1455+
ASSERT_TRUE(msg.find("failed to convert string '0x1' to date") != std::string::npos);
14541456
}
14551457
}
14561458

‎src/paimon/core/casting/string_to_boolean_cast_executor.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ Result<Literal> StringToBooleanCastExecutor::Cast(
5252
std::optional<bool> bool_value = StringUtils::StringToValue<bool>(value);
5353
if (bool_value == std::nullopt) {
5454
return Status::Invalid(fmt::format(
55-
"StringToBooleanCastExecutor cast failed: STRING {} cannot cast to BOOLEAN", value));
55+
"StringToBooleanCastExecutor cast failed: STRING '{}' cannot cast to BOOLEAN", value));
5656
}
5757
return Literal(bool_value.value());
5858
}
@@ -71,7 +71,7 @@ Result<std::shared_ptr<arrow::Array>> StringToBooleanCastExecutor::Cast(
7171
StringUtils::StringToValue<bool>(string_array->GetString(i));
7272
if (bool_value == std::nullopt) {
7373
return Status::Invalid(fmt::format(
74-
"StringToBooleanCastExecutor cast failed: STRING {} cannot cast to BOOLEAN",
74+
"StringToBooleanCastExecutor cast failed: STRING '{}' cannot cast to BOOLEAN",
7575
string_array->GetString(i)));
7676
}
7777
PAIMON_RETURN_NOT_OK_FROM_ARROW(bool_builder->Append(bool_value.value()));

‎src/paimon/core/casting/string_to_numeric_primitive_cast_executor.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ Result<Literal> StringToNumericPrimitiveCastExecutor::CastLiteral(const Literal&
7171
if (!success) {
7272
return Status::Invalid(
7373
fmt::format("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
74-
"cast {} from STRING to {}",
74+
"cast '{}' from STRING to {}",
7575
value, FieldTypeUtils::FieldTypeToString(target_type)));
7676
}
7777
return Literal(out);
@@ -80,7 +80,7 @@ Result<Literal> StringToNumericPrimitiveCastExecutor::CastLiteral(const Literal&
8080
if (!casted_value) {
8181
return Status::Invalid(
8282
fmt::format("cast literal in StringToNumericPrimitiveCastExecutor failed: cannot "
83-
"cast {} from STRING to {}",
83+
"cast '{}' from STRING to {}",
8484
value, FieldTypeUtils::FieldTypeToString(target_type)));
8585
}
8686
return Literal(casted_value.value());

‎src/paimon/core/io/complete_row_tracking_fields_reader.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ Status CompleteRowTrackingFieldsBatchReader::SetReadSchema(
5353
int32_t sequence_id_idx = arrow_schema->GetFieldIndex(SpecialFields::SequenceNumber().Name());
5454
if (sequence_id_idx != -1 &&
5555
file_schema->GetFieldIndex(SpecialFields::SequenceNumber().Name()) == -1) {
56-
// read special fields but file not exist, remove special fields to format reader
56+
// read special fields but file not exist, remove special fields to format reader
5757
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(arrow_schema, arrow_schema->RemoveField(sequence_id_idx));
5858
}
5959
ArrowSchema c_schema;

‎src/paimon/core/io/row_to_arrow_array_converter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
#include "paimon/memory/memory_pool.h"
3232
#include "paimon/reader/batch_reader.h"
3333
namespace paimon {
34-
// convert row T to output R (R maybe BatchReader::ReadBatch or KeyValueBatch)
34+
// convert row T to output R (R maybe BatchReader::ReadBatch or KeyValueBatch)
3535
template <typename T, typename R>
3636
class RowToArrowArrayConverter {
3737
public:

‎src/paimon/format/avro/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ if(PAIMON_ENABLE_AVRO)
5050
if(PAIMON_BUILD_TESTS)
5151
add_paimon_test(avro_format_test
5252
SOURCES
53+
avro_direct_encoder_decoder_test.cpp
5354
avro_file_batch_reader_test.cpp
5455
avro_file_format_test.cpp
5556
avro_format_writer_test.cpp

‎src/paimon/format/avro/avro_direct_decoder.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ Status DecodeFieldToBuilder(const ::avro::NodePtr& avro_node,
418418

419419
const auto& branch_node = avro_node->leafAt(branch_index);
420420
if (branch_node->type() == ::avro::AVRO_NULL) {
421+
decoder->decodeNull();
421422
PAIMON_RETURN_NOT_OK_FROM_ARROW(array_builder->AppendNull());
422423
return Status::OK();
423424
} else {

‎src/paimon/format/avro/avro_direct_encoder.cpp‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ Result<UnionBranches> ValidateUnion(const ::avro::NodePtr& union_node) {
5757
return UnionBranches{.null_index = 0, .value_index = 1, .value_node = branch_1};
5858
}
5959
if (branch_1->type() == ::avro::AVRO_NULL && branch_0->type() != ::avro::AVRO_NULL) {
60-
return UnionBranches{.null_index = 1, .value_index = 0, .value_node = branch_0};
60+
return Status::Invalid(
61+
"Unexpected: In paimon, we expect the null branch to be the first branch in a union.");
6162
}
6263
return Status::Invalid("Union must have exactly one null branch");
6364
}
@@ -92,10 +93,6 @@ Status AvroDirectEncoder::EncodeArrowToAvro(const ::avro::NodePtr& avro_node,
9293
}
9394

9495
switch (avro_node->type()) {
95-
case ::avro::AVRO_NULL:
96-
encoder->encodeNull();
97-
return Status::OK();
98-
9996
case ::avro::AVRO_BOOL: {
10097
const auto& bool_array =
10198
arrow::internal::checked_cast<const arrow::BooleanArray&>(array);
@@ -230,9 +227,7 @@ Status AvroDirectEncoder::EncodeArrowToAvro(const ::avro::NodePtr& avro_node,
230227
const auto& binary_array =
231228
arrow::internal::checked_cast<const arrow::BinaryArray&>(array);
232229
std::string_view value = binary_array.GetView(row_index);
233-
// TODO(jinli.zjw): need to copy to ctx?
234-
ctx->assign(value.begin(), value.end());
235-
encoder->encodeBytes(ctx->data(), ctx->size());
230+
encoder->encodeBytes(reinterpret_cast<const uint8_t*>(value.data()), value.size());
236231
return Status::OK();
237232
}
238233

@@ -294,7 +289,7 @@ Status AvroDirectEncoder::EncodeArrowToAvro(const ::avro::NodePtr& avro_node,
294289
element_node->leaves() != 2)) {
295290
return Status::Invalid(
296291
fmt::format("Expected AVRO_RECORD for map key-value pair, got {}",
297-
::avro::toString(element_node->type())));
292+
AvroUtils::ToString(avro_node)));
298293
}
299294

300295
const auto& map_array =
@@ -366,9 +361,11 @@ Status AvroDirectEncoder::EncodeArrowToAvro(const ::avro::NodePtr& avro_node,
366361
return Status::OK();
367362
}
368363

364+
case ::avro::AVRO_NULL:
369365
case ::avro::AVRO_UNION:
370366
// Already handled above
371-
return Status::Invalid("Unexpected union handling");
367+
return Status::Invalid(fmt::format("Unexpected Avro type handling: {}",
368+
::avro::toString(avro_node->type())));
372369
default:
373370
return Status::Invalid(
374371
fmt::format("Unsupported Avro type: {}", ::avro::toString(avro_node->type())));

0 commit comments

Comments
 (0)