From cbfe93a5e705815fb56f507e13c0b805399f0f5b Mon Sep 17 00:00:00 2001 From: retikulum Date: Mon, 24 Oct 2022 22:21:41 +0300 Subject: [PATCH 1/5] Implement serialization for ScalarValue::FixedSizeBinary --- datafusion/proto/proto/datafusion.proto | 7 +++++++ datafusion/proto/src/from_proto.rs | 4 ++++ datafusion/proto/src/lib.rs | 2 ++ datafusion/proto/src/to_proto.rs | 11 +++++++---- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/datafusion/proto/proto/datafusion.proto b/datafusion/proto/proto/datafusion.proto index d61f52ee7bb27..a03a90c3769df 100644 --- a/datafusion/proto/proto/datafusion.proto +++ b/datafusion/proto/proto/datafusion.proto @@ -760,6 +760,11 @@ message StructValue { repeated Field fields = 3; } +message ScalarFixedSizeBinary{ + bytes values = 1; + int32 length = 2; +} + message ScalarValue{ oneof value { // Null value of any type (type is encoded) @@ -794,6 +799,7 @@ message ScalarValue{ int64 time64_value = 30; IntervalMonthDayNanoValue interval_month_day_nano = 31; StructValue struct_value = 32; + ScalarFixedSizeBinary fixed_size_binary_value = 33; } } @@ -834,6 +840,7 @@ enum PrimitiveScalarType{ BINARY = 25; LARGE_BINARY = 26; + FIXED_SIZE_BINARY = 29; TIME64 = 27; } diff --git a/datafusion/proto/src/from_proto.rs b/datafusion/proto/src/from_proto.rs index 79b477b3e1ef0..c36605e24943d 100644 --- a/datafusion/proto/src/from_proto.rs +++ b/datafusion/proto/src/from_proto.rs @@ -248,6 +248,7 @@ impl From for DataType { protobuf::PrimitiveScalarType::IntervalMonthdaynano => { DataType::Interval(IntervalUnit::MonthDayNano) } + protobuf::PrimitiveScalarType::FixedSizeBinary => DataType::FixedSizeBinary(0), } } } @@ -620,6 +621,7 @@ impl TryFrom<&protobuf::PrimitiveScalarType> for ScalarValue { PrimitiveScalarType::IntervalYearmonth => Self::IntervalYearMonth(None), PrimitiveScalarType::IntervalDaytime => Self::IntervalDayTime(None), PrimitiveScalarType::IntervalMonthdaynano => Self::IntervalMonthDayNano(None), + PrimitiveScalarType::FixedSizeBinary => Self::FixedSizeBinary(0, None), }) } } @@ -754,6 +756,7 @@ impl TryFrom<&protobuf::ScalarValue> for ScalarValue { Self::Struct(values, Box::new(fields)) } + Value::FixedSizeBinaryValue(v) => Self::FixedSizeBinary(v.length, Some(v.clone().values)), }) } } @@ -1492,6 +1495,7 @@ fn typechecked_scalar_value_conversion( } PrimitiveScalarType::Binary => ScalarValue::Binary(None), PrimitiveScalarType::LargeBinary => ScalarValue::LargeBinary(None), + PrimitiveScalarType::FixedSizeBinary => ScalarValue::FixedSizeBinary(0, None) , }; scalar_value } else { diff --git a/datafusion/proto/src/lib.rs b/datafusion/proto/src/lib.rs index 7feae79652153..98bdb18ad5c69 100644 --- a/datafusion/proto/src/lib.rs +++ b/datafusion/proto/src/lib.rs @@ -556,6 +556,8 @@ mod roundtrip_tests { Field::new("a", DataType::Boolean, false), ]), ), + ScalarValue::FixedSizeBinary(b"bar".to_vec().len() as i32, Some(b"bar".to_vec())), + ScalarValue::Binary(None), ]; for test_case in should_pass.into_iter() { diff --git a/datafusion/proto/src/to_proto.rs b/datafusion/proto/src/to_proto.rs index f8dab779b4052..3f3c316b0e99c 100644 --- a/datafusion/proto/src/to_proto.rs +++ b/datafusion/proto/src/to_proto.rs @@ -1063,10 +1063,13 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { Value::LargeBinaryValue(s.to_owned()) }) } - scalar::ScalarValue::FixedSizeBinary(_, _) => { - return Err(Error::General( - "FixedSizeBinary is not yet implemented".to_owned(), - )) + scalar::ScalarValue::FixedSizeBinary(length, val) => { + create_proto_scalar(val, PrimitiveScalarType::FixedSizeBinary, |s| { + Value::FixedSizeBinaryValue(protobuf::ScalarFixedSizeBinary{ + values: s.to_owned(), + length: *length, + }) + }) } datafusion::scalar::ScalarValue::Time64(v) => { From 2f1576dac2819cbd6c53a31b31a9888d4a6ae322 Mon Sep 17 00:00:00 2001 From: retikulum Date: Mon, 24 Oct 2022 22:26:10 +0300 Subject: [PATCH 2/5] correct test --- datafusion/proto/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/proto/src/lib.rs b/datafusion/proto/src/lib.rs index 98bdb18ad5c69..daf2df70dd57a 100644 --- a/datafusion/proto/src/lib.rs +++ b/datafusion/proto/src/lib.rs @@ -557,7 +557,7 @@ mod roundtrip_tests { ]), ), ScalarValue::FixedSizeBinary(b"bar".to_vec().len() as i32, Some(b"bar".to_vec())), - ScalarValue::Binary(None), + ScalarValue::FixedSizeBinary(0, None), ]; for test_case in should_pass.into_iter() { From 5c290c727246567746b05b17d353306fa61e2333 Mon Sep 17 00:00:00 2001 From: retikulum Date: Mon, 24 Oct 2022 23:00:07 +0300 Subject: [PATCH 3/5] fix formatting --- datafusion/proto/src/from_proto.rs | 12 +++++++++--- datafusion/proto/src/lib.rs | 5 ++++- datafusion/proto/src/to_proto.rs | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/datafusion/proto/src/from_proto.rs b/datafusion/proto/src/from_proto.rs index caecfa2024572..32ce77dce4e66 100644 --- a/datafusion/proto/src/from_proto.rs +++ b/datafusion/proto/src/from_proto.rs @@ -248,7 +248,9 @@ impl From for DataType { protobuf::PrimitiveScalarType::IntervalMonthdaynano => { DataType::Interval(IntervalUnit::MonthDayNano) } - protobuf::PrimitiveScalarType::FixedSizeBinary => DataType::FixedSizeBinary(0), + protobuf::PrimitiveScalarType::FixedSizeBinary => { + DataType::FixedSizeBinary(0) + } } } } @@ -756,7 +758,9 @@ impl TryFrom<&protobuf::ScalarValue> for ScalarValue { Self::Struct(values, Box::new(fields)) } - Value::FixedSizeBinaryValue(v) => Self::FixedSizeBinary(v.length, Some(v.clone().values)), + Value::FixedSizeBinaryValue(v) => { + Self::FixedSizeBinary(v.length, Some(v.clone().values)) + } }) } } @@ -1495,7 +1499,9 @@ fn typechecked_scalar_value_conversion( } PrimitiveScalarType::Binary => ScalarValue::Binary(None), PrimitiveScalarType::LargeBinary => ScalarValue::LargeBinary(None), - PrimitiveScalarType::FixedSizeBinary => ScalarValue::FixedSizeBinary(0, None) , + PrimitiveScalarType::FixedSizeBinary => { + ScalarValue::FixedSizeBinary(0, None) + } }; scalar_value } else { diff --git a/datafusion/proto/src/lib.rs b/datafusion/proto/src/lib.rs index e163af1296d00..aef8cb67c307b 100644 --- a/datafusion/proto/src/lib.rs +++ b/datafusion/proto/src/lib.rs @@ -556,7 +556,10 @@ mod roundtrip_tests { Field::new("a", DataType::Boolean, false), ]), ), - ScalarValue::FixedSizeBinary(b"bar".to_vec().len() as i32, Some(b"bar".to_vec())), + ScalarValue::FixedSizeBinary( + b"bar".to_vec().len() as i32, + Some(b"bar".to_vec()), + ), ScalarValue::FixedSizeBinary(0, None), ]; diff --git a/datafusion/proto/src/to_proto.rs b/datafusion/proto/src/to_proto.rs index dd32e1ebc3712..aab294960177a 100644 --- a/datafusion/proto/src/to_proto.rs +++ b/datafusion/proto/src/to_proto.rs @@ -1066,7 +1066,7 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { } scalar::ScalarValue::FixedSizeBinary(length, val) => { create_proto_scalar(val, PrimitiveScalarType::FixedSizeBinary, |s| { - Value::FixedSizeBinaryValue(protobuf::ScalarFixedSizeBinary{ + Value::FixedSizeBinaryValue(protobuf::ScalarFixedSizeBinary { values: s.to_owned(), length: *length, }) From 7d2818821e98f3a2b3bc033fe063834031ca2ccb Mon Sep 17 00:00:00 2001 From: Burak Date: Tue, 25 Oct 2022 23:15:00 +0300 Subject: [PATCH 4/5] add none test case for non zero length Co-authored-by: Andrew Lamb --- datafusion/proto/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/datafusion/proto/src/lib.rs b/datafusion/proto/src/lib.rs index aef8cb67c307b..d5c3103475e01 100644 --- a/datafusion/proto/src/lib.rs +++ b/datafusion/proto/src/lib.rs @@ -561,6 +561,7 @@ mod roundtrip_tests { Some(b"bar".to_vec()), ), ScalarValue::FixedSizeBinary(0, None), + ScalarValue::FixedSizeBinary(5, None), ]; for test_case in should_pass.into_iter() { From a6173b50b066752acc9e2e20c574ee0b4aaa6782 Mon Sep 17 00:00:00 2001 From: retikulum Date: Tue, 25 Oct 2022 23:44:55 +0300 Subject: [PATCH 5/5] resolve conflict --- datafusion/proto/proto/datafusion.proto | 2 +- datafusion/proto/src/lib.rs | 1 + datafusion/proto/src/to_proto.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/datafusion/proto/proto/datafusion.proto b/datafusion/proto/proto/datafusion.proto index 8c7a96dcde558..4c56b2f081edc 100644 --- a/datafusion/proto/proto/datafusion.proto +++ b/datafusion/proto/proto/datafusion.proto @@ -813,7 +813,7 @@ message ScalarValue{ int64 time64_value = 30; IntervalMonthDayNanoValue interval_month_day_nano = 31; StructValue struct_value = 32; - ScalarFixedSizeBinary fixed_size_binary_value = 33; + ScalarFixedSizeBinary fixed_size_binary_value = 34; } } diff --git a/datafusion/proto/src/lib.rs b/datafusion/proto/src/lib.rs index 5cf5d542a975e..1552542a10710 100644 --- a/datafusion/proto/src/lib.rs +++ b/datafusion/proto/src/lib.rs @@ -687,6 +687,7 @@ mod roundtrip_tests { Some(b"bar".to_vec()), ), ScalarValue::FixedSizeBinary(0, None), + ScalarValue::FixedSizeBinary(5, None), ]; for test_case in should_pass.into_iter() { diff --git a/datafusion/proto/src/to_proto.rs b/datafusion/proto/src/to_proto.rs index aa3d80a1be8a8..bdfe5becae135 100644 --- a/datafusion/proto/src/to_proto.rs +++ b/datafusion/proto/src/to_proto.rs @@ -1034,7 +1034,7 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { }) } scalar::ScalarValue::FixedSizeBinary(length, val) => { - create_proto_scalar(val, PrimitiveScalarType::FixedSizeBinary, |s| { + create_proto_scalar(val, &data_type, |s| { Value::FixedSizeBinaryValue(protobuf::ScalarFixedSizeBinary { values: s.to_owned(), length: *length,