Skip to content

Commit

Permalink
Refactor FixedSizeBinaryBuilder Constructors (#2516)
Browse files Browse the repository at this point in the history
* refactor FixedSizeBinaryBuilder apis

* Fixing errors
  • Loading branch information
psvri committed Aug 19, 2022
1 parent ac8b0eb commit b3fd4bd
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 14 deletions.
4 changes: 2 additions & 2 deletions arrow/src/array/builder/decimal_builder.rs
Expand Up @@ -65,7 +65,7 @@ impl Decimal128Builder {
/// array
pub fn new(capacity: usize, precision: usize, scale: usize) -> Self {
Self {
builder: FixedSizeBinaryBuilder::new(capacity, Self::BYTE_LENGTH),
builder: FixedSizeBinaryBuilder::with_capacity(capacity, Self::BYTE_LENGTH),
precision,
scale,
value_validation: true,
Expand Down Expand Up @@ -159,7 +159,7 @@ impl Decimal256Builder {
/// array
pub fn new(capacity: usize, precision: usize, scale: usize) -> Self {
Self {
builder: FixedSizeBinaryBuilder::new(capacity, Self::BYTE_LENGTH),
builder: FixedSizeBinaryBuilder::with_capacity(capacity, Self::BYTE_LENGTH),
precision,
scale,
value_validation: true,
Expand Down
17 changes: 11 additions & 6 deletions arrow/src/array/builder/fixed_size_binary_builder.rs
Expand Up @@ -33,9 +33,14 @@ pub struct FixedSizeBinaryBuilder {
}

impl FixedSizeBinaryBuilder {
/// Creates a new [`FixedSizeBinaryBuilder`]
pub fn new(byte_width: i32) -> Self {
Self::with_capacity(1024, byte_width)
}

/// Creates a new [`FixedSizeBinaryBuilder`], `capacity` is the number of bytes in the values
/// buffer
pub fn new(capacity: usize, byte_width: i32) -> Self {
pub fn with_capacity(capacity: usize, byte_width: i32) -> Self {
assert!(
byte_width >= 0,
"value length ({}) of the array must >= 0",
Expand Down Expand Up @@ -132,7 +137,7 @@ mod tests {

#[test]
fn test_fixed_size_binary_builder() {
let mut builder = FixedSizeBinaryBuilder::new(15, 5);
let mut builder = FixedSizeBinaryBuilder::with_capacity(15, 5);

// [b"hello", null, "arrow"]
builder.append_value(b"hello").unwrap();
Expand All @@ -149,7 +154,7 @@ mod tests {

#[test]
fn test_fixed_size_binary_builder_with_zero_value_length() {
let mut builder = FixedSizeBinaryBuilder::new(0, 0);
let mut builder = FixedSizeBinaryBuilder::new(0);

builder.append_value(b"").unwrap();
builder.append_null();
Expand All @@ -171,12 +176,12 @@ mod tests {
expected = "Byte slice does not have the same length as FixedSizeBinaryBuilder value lengths"
)]
fn test_fixed_size_binary_builder_with_inconsistent_value_length() {
let mut builder = FixedSizeBinaryBuilder::new(15, 4);
let mut builder = FixedSizeBinaryBuilder::with_capacity(15, 4);
builder.append_value(b"hello").unwrap();
}
#[test]
fn test_fixed_size_binary_builder_empty() {
let mut builder = FixedSizeBinaryBuilder::new(15, 5);
let mut builder = FixedSizeBinaryBuilder::new(5);
assert!(builder.is_empty());

let fixed_size_binary_array = builder.finish();
Expand All @@ -190,6 +195,6 @@ mod tests {
#[test]
#[should_panic(expected = "value length (-1) of the array must >= 0")]
fn test_fixed_size_binary_builder_invalid_value_length() {
let _ = FixedSizeBinaryBuilder::new(15, -1);
let _ = FixedSizeBinaryBuilder::with_capacity(15, -1);
}
}
2 changes: 1 addition & 1 deletion arrow/src/array/builder/struct_builder.rs
Expand Up @@ -109,7 +109,7 @@ pub fn make_builder(datatype: &DataType, capacity: usize) -> Box<dyn ArrayBuilde
DataType::Float64 => Box::new(Float64Builder::new(capacity)),
DataType::Binary => Box::new(BinaryBuilder::new(capacity)),
DataType::FixedSizeBinary(len) => {
Box::new(FixedSizeBinaryBuilder::new(capacity, *len))
Box::new(FixedSizeBinaryBuilder::with_capacity(capacity, *len))
}
DataType::Decimal128(precision, scale) => {
Box::new(Decimal128Builder::new(capacity, *precision, *scale))
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/array/equal/mod.rs
Expand Up @@ -768,7 +768,7 @@ mod tests {
fn create_fixed_size_binary_array<U: AsRef<[u8]>, T: AsRef<[Option<U>]>>(
data: T,
) -> ArrayData {
let mut builder = FixedSizeBinaryBuilder::new(15, 5);
let mut builder = FixedSizeBinaryBuilder::with_capacity(15, 5);

for d in data.as_ref() {
if let Some(v) = d {
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/util/integration_util.rs
Expand Up @@ -643,7 +643,7 @@ pub fn array_from_json(
Ok(Arc::new(b.finish()))
}
DataType::FixedSizeBinary(len) => {
let mut b = FixedSizeBinaryBuilder::new(json_col.count, *len);
let mut b = FixedSizeBinaryBuilder::with_capacity(json_col.count, *len);
for (is_valid, value) in json_col
.validity
.as_ref()
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/util/pretty.rs
Expand Up @@ -317,7 +317,7 @@ mod tests {
let field_type = DataType::FixedSizeBinary(3);
let schema = Arc::new(Schema::new(vec![Field::new("d1", field_type, true)]));

let mut builder = FixedSizeBinaryBuilder::new(3, 3);
let mut builder = FixedSizeBinaryBuilder::with_capacity(3, 3);

builder.append_value(&[1, 2, 3]).unwrap();
builder.append_null();
Expand Down
2 changes: 1 addition & 1 deletion parquet/src/arrow/arrow_writer/mod.rs
Expand Up @@ -1477,7 +1477,7 @@ mod tests {

#[test]
fn fixed_size_binary_single_column() {
let mut builder = FixedSizeBinaryBuilder::new(16, 4);
let mut builder = FixedSizeBinaryBuilder::new(4);
builder.append_value(b"0123").unwrap();
builder.append_null();
builder.append_value(b"8910").unwrap();
Expand Down
3 changes: 2 additions & 1 deletion parquet/src/arrow/buffer/converter.rs
Expand Up @@ -55,7 +55,8 @@ impl Converter<Vec<Option<FixedLenByteArray>>, FixedSizeBinaryArray>
&self,
source: Vec<Option<FixedLenByteArray>>,
) -> Result<FixedSizeBinaryArray> {
let mut builder = FixedSizeBinaryBuilder::new(source.len(), self.byte_width);
let mut builder =
FixedSizeBinaryBuilder::with_capacity(source.len(), self.byte_width);
for v in source {
match v {
Some(array) => builder.append_value(array.data())?,
Expand Down

0 comments on commit b3fd4bd

Please sign in to comment.