Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FixedSizeBinaryBuilder Constructors #2516

Merged
merged 2 commits into from Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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