Skip to content

Commit f1ea6e1

Browse files
authored
feat: StructAccessor.get returns Result<Option<Datum>> instead of Result<Datum> (#390)
This is so that the accessor's result can represent null field values. Fixes: #379
1 parent 1aa05e0 commit f1ea6e1

File tree

3 files changed

+51
-12
lines changed

3 files changed

+51
-12
lines changed

crates/iceberg/src/expr/accessor.rs

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use crate::spec::{Datum, Literal, PrimitiveType, Struct};
19-
use crate::{Error, ErrorKind};
2018
use serde_derive::{Deserialize, Serialize};
2119
use std::sync::Arc;
2220

21+
use crate::spec::{Datum, Literal, PrimitiveType, Struct};
22+
use crate::{Error, ErrorKind, Result};
23+
2324
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
2425
pub struct StructAccessor {
2526
position: usize,
@@ -54,11 +55,13 @@ impl StructAccessor {
5455
&self.r#type
5556
}
5657

57-
pub(crate) fn get<'a>(&'a self, container: &'a Struct) -> crate::Result<Datum> {
58+
pub(crate) fn get<'a>(&'a self, container: &'a Struct) -> Result<Option<Datum>> {
5859
match &self.inner {
5960
None => {
60-
if let Literal::Primitive(literal) = &container[self.position] {
61-
Ok(Datum::new(self.r#type().clone(), literal.clone()))
61+
if container.is_null_at_index(self.position) {
62+
Ok(None)
63+
} else if let Literal::Primitive(literal) = &container[self.position] {
64+
Ok(Some(Datum::new(self.r#type().clone(), literal.clone())))
6265
} else {
6366
Err(Error::new(
6467
ErrorKind::Unexpected,
@@ -95,7 +98,19 @@ mod tests {
9598
let test_struct =
9699
Struct::from_iter(vec![Some(Literal::bool(false)), Some(Literal::bool(true))]);
97100

98-
assert_eq!(accessor.get(&test_struct).unwrap(), Datum::bool(true));
101+
assert_eq!(accessor.get(&test_struct).unwrap(), Some(Datum::bool(true)));
102+
}
103+
104+
#[test]
105+
fn test_single_level_accessor_null() {
106+
let accessor = StructAccessor::new(1, PrimitiveType::Boolean);
107+
108+
assert_eq!(accessor.r#type(), &PrimitiveType::Boolean);
109+
assert_eq!(accessor.position(), 1);
110+
111+
let test_struct = Struct::from_iter(vec![Some(Literal::bool(false)), None]);
112+
113+
assert_eq!(accessor.get(&test_struct).unwrap(), None);
99114
}
100115

101116
#[test]
@@ -115,6 +130,25 @@ mod tests {
115130
Some(Literal::Struct(nested_test_struct)),
116131
]);
117132

118-
assert_eq!(accessor.get(&test_struct).unwrap(), Datum::bool(true));
133+
assert_eq!(accessor.get(&test_struct).unwrap(), Some(Datum::bool(true)));
134+
}
135+
136+
#[test]
137+
fn test_nested_accessor_null() {
138+
let nested_accessor = StructAccessor::new(0, PrimitiveType::Boolean);
139+
let accessor = StructAccessor::wrap(2, Box::new(nested_accessor));
140+
141+
assert_eq!(accessor.r#type(), &PrimitiveType::Boolean);
142+
//assert_eq!(accessor.position(), 1);
143+
144+
let nested_test_struct = Struct::from_iter(vec![None, Some(Literal::bool(true))]);
145+
146+
let test_struct = Struct::from_iter(vec![
147+
Some(Literal::bool(false)),
148+
Some(Literal::bool(false)),
149+
Some(Literal::Struct(nested_test_struct)),
150+
]);
151+
152+
assert_eq!(accessor.get(&test_struct).unwrap(), None);
119153
}
120154
}

crates/iceberg/src/spec/schema.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,39 +1683,39 @@ table {
16831683
.unwrap()
16841684
.get(&test_struct)
16851685
.unwrap(),
1686-
Datum::string("foo value")
1686+
Some(Datum::string("foo value"))
16871687
);
16881688
assert_eq!(
16891689
schema
16901690
.accessor_by_field_id(2)
16911691
.unwrap()
16921692
.get(&test_struct)
16931693
.unwrap(),
1694-
Datum::int(1002)
1694+
Some(Datum::int(1002))
16951695
);
16961696
assert_eq!(
16971697
schema
16981698
.accessor_by_field_id(3)
16991699
.unwrap()
17001700
.get(&test_struct)
17011701
.unwrap(),
1702-
Datum::bool(true)
1702+
Some(Datum::bool(true))
17031703
);
17041704
assert_eq!(
17051705
schema
17061706
.accessor_by_field_id(16)
17071707
.unwrap()
17081708
.get(&test_struct)
17091709
.unwrap(),
1710-
Datum::string("Testy McTest")
1710+
Some(Datum::string("Testy McTest"))
17111711
);
17121712
assert_eq!(
17131713
schema
17141714
.accessor_by_field_id(17)
17151715
.unwrap()
17161716
.get(&test_struct)
17171717
.unwrap(),
1718-
Datum::int(33)
1718+
Some(Datum::int(33))
17191719
);
17201720
}
17211721

crates/iceberg/src/spec/values.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,6 +1440,11 @@ impl Struct {
14401440
},
14411441
)
14421442
}
1443+
1444+
/// returns true if the field at position `index` is null
1445+
pub fn is_null_at_index(&self, index: usize) -> bool {
1446+
self.null_bitmap[index]
1447+
}
14431448
}
14441449

14451450
impl Index<usize> for Struct {

0 commit comments

Comments
 (0)