alamb commented on a change in pull request #1891:
URL: https://github.com/apache/arrow-datafusion/pull/1891#discussion_r816820195
##########
File path: datafusion/src/row/mod.rs
##########
@@ -349,6 +354,38 @@ mod tests {
assert_eq!(batch, output_batch);
Ok(())
}
+
+ #[test]
+ #[allow(non_snake_case)]
+ fn [<test_single_ $TYPE _nf>]() -> Result<()> {
Review comment:
```suggestion
fn [<test_single_ $TYPE _null_free>]() -> Result<()> {
```
##########
File path: datafusion/src/row/mod.rs
##########
@@ -349,6 +354,38 @@ mod tests {
assert_eq!(batch, output_batch);
Ok(())
}
+
+ #[test]
+ #[allow(non_snake_case)]
+ fn [<test_single_ $TYPE _nf>]() -> Result<()> {
+ let schema = Arc::new(Schema::new(vec![Field::new("a",
$TYPE, false)]));
+ let v = $VEC.into_iter().filter(|o|
o.is_some()).collect::<Vec<_>>();
+ let a = $ARRAY::from(v);
+ let batch = RecordBatch::try_new(schema.clone(),
vec![Arc::new(a)])?;
+ let mut vector = vec![0; 1024];
+ let row_offsets =
+ { write_batch_unchecked(&mut vector, 0, &batch, 0,
schema.clone()) };
+ let output_batch = { read_as_batch(&vector, schema,
row_offsets)? };
+ assert_eq!(batch, output_batch);
+ Ok(())
+ }
+
+ #[test]
+ #[allow(non_snake_case)]
+ #[cfg(feature = "jit")]
+ fn [<test_single_ $TYPE _jit_nf>]() -> Result<()> {
Review comment:
```suggestion
fn [<test_single_ $TYPE _jit_null_free>]() -> Result<()> {
```
##########
File path: datafusion/src/row/mod.rs
##########
@@ -439,7 +476,7 @@ mod tests {
#[test]
fn test_single_binary() -> Result<()> {
- let schema = Arc::new(Schema::new(vec![Field::new("a", Binary,
false)]));
+ let schema = Arc::new(Schema::new(vec![Field::new("a", Binary,
true)]));
Review comment:
👍
##########
File path: datafusion/src/row/mod.rs
##########
@@ -478,6 +515,45 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_single_binary_nf() -> Result<()> {
Review comment:
```suggestion
fn test_single_binary_null_free() -> Result<()> {
```
##########
File path: datafusion/src/row/mod.rs
##########
@@ -349,6 +354,38 @@ mod tests {
assert_eq!(batch, output_batch);
Ok(())
}
+
+ #[test]
+ #[allow(non_snake_case)]
+ fn [<test_single_ $TYPE _nf>]() -> Result<()> {
Review comment:
I wondered what the null free thing was about
##########
File path: datafusion/src/row/writer.rs
##########
@@ -17,19 +17,24 @@
//! Reusable row writer backed by Vec<u8> to stitch attributes together
+#[cfg(feature = "jit")]
use crate::error::Result;
#[cfg(feature = "jit")]
use crate::reg_fn;
#[cfg(feature = "jit")]
use crate::row::fn_name;
-use crate::row::{estimate_row_width, fixed_size, get_offsets, supported};
+use crate::row::{
+ estimate_row_width, fixed_size, get_offsets, schema_null_free, supported,
+};
use arrow::array::*;
use arrow::datatypes::{DataType, Schema};
use arrow::record_batch::RecordBatch;
use arrow::util::bit_util::{ceil, round_upto_power_of_2, set_bit_raw,
unset_bit_raw};
+#[cfg(feature = "jit")]
Review comment:
I think over time it would be good to start trying to encapsulate the
JIT'd code more (as in reduce the number of `#[cfg(feature = "jit")]` calls --
perhaps by defining a common interface for creating jit and non jit versions.
As I am interested in getting more involved in this project, I would be happy
to try and do so (or do it as part of a larger body of work)
##########
File path: datafusion/src/row/reader.rs
##########
@@ -174,14 +184,22 @@ impl<'a> RowReader<'a> {
#[inline(always)]
fn null_bits(&self) -> &[u8] {
- let start = self.base_offset;
- &self.data[start..start + self.null_width]
+ if self.null_free {
+ &[]
+ } else {
+ let start = self.base_offset;
+ &self.data[start..start + self.null_width]
Review comment:
if `null_width` is always zero, I wonder if the check for
`self.null_free` is needed?
##########
File path: datafusion/src/row/mod.rs
##########
@@ -478,6 +515,45 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_single_binary_nf() -> Result<()> {
+ let schema = Arc::new(Schema::new(vec![Field::new("a", Binary,
false)]));
+ let values: Vec<&[u8]> = vec![b"one", b"two", b"", b"three"];
+ let a = BinaryArray::from_vec(values);
+ let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(a)])?;
+ let mut vector = vec![0; 8192];
+ let row_offsets =
+ { write_batch_unchecked(&mut vector, 0, &batch, 0, schema.clone())
};
+ let output_batch = { read_as_batch(&vector, schema, row_offsets)? };
+ assert_eq!(batch, output_batch);
+ Ok(())
+ }
+
+ #[test]
+ #[cfg(feature = "jit")]
+ fn test_single_binary_jit_nf() -> Result<()> {
Review comment:
```suggestion
fn test_single_binary_jit_null_free() -> Result<()> {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
