Skip to content

fix: ignore null objects #6748

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.

Sign up for GitHub

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

Open
cemremengu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: ignore null objects #6748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions quickwit/quickwit-doc-mapper/src/doc_mapper/doc_mapper_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,117 @@ mod tests {
Ok(())
}

#[test]
fn test_parse_document_with_null_object() {
use serde::Deserialize;

let doc_mapper_json = json!({
"field_mappings": [
{
"name": "payload",
"type": "object",
"field_mappings": [
{
"name": "head",
"type": "object",
"field_mappings": [
{"name": "label", "type": "text"},
{
"name": "repo",
"type": "object",
"field_mappings": [
{"name": "name", "type": "text"}
]
}
]
}
]
}
]
});
let doc_mapper = DocMapperBuilder::deserialize(doc_mapper_json)
.unwrap()
.try_build()
.unwrap();

// A `null` object is simply ignored, like a `null` leaf value.
let JsonValue::Object(json_obj) = json!({
"payload": {
"head": {
"label": "main",
"repo": JsonValue::Null
}
}
}) else {
panic!("expected a JSON object");
};
let (_, document) = doc_mapper.doc_from_json_obj(json_obj, 0).unwrap();
let schema = doc_mapper.schema();
let label_field = schema.get_field("payload.head.label").unwrap();
assert_eq!(
document
.get_first(label_field)
.and_then(|value| value.as_str()),
Some("main")
);

// The whole document can also be `null` all the way up.
let JsonValue::Object(json_obj) = json!({"payload": JsonValue::Null}) else {
panic!("expected a JSON object");
};
doc_mapper.doc_from_json_obj(json_obj, 0).unwrap();

// Non-null, non-object values are still rejected.
let JsonValue::Object(json_obj) = json!({"payload": {"head": {"repo": "quickwit"}}}) else {
panic!("expected a JSON object");
};
let error = doc_mapper.doc_from_json_obj(json_obj, 0).unwrap_err();
assert!(matches!(error, DocParsingError::ValueError(field_path, _)
if field_path == "payload.head.repo"));
}

#[test]
fn test_validate_document_with_null_object() {
use serde::Deserialize;

let doc_mapper_json = json!({
"mode": "strict",
"field_mappings": [
{
"name": "payload",
"type": "object",
"field_mappings": [
{
"name": "repo",
"type": "object",
"field_mappings": [
{"name": "name", "type": "text"}
]
}
]
}
]
});
let doc_mapper = DocMapperBuilder::deserialize(doc_mapper_json)
.unwrap()
.try_build()
.unwrap();

let json_val: serde_json_borrow::Value =
serde_json::from_str(r#"{"payload": {"repo": null}}"#).unwrap();
doc_mapper
.validate_json_obj(json_val.as_object().unwrap())
.unwrap();

let json_val: serde_json_borrow::Value =
serde_json::from_str(r#"{"payload": {"repo": "quickwit"}}"#).unwrap();
let error = doc_mapper
.validate_json_obj(json_val.as_object().unwrap())
.unwrap_err();
assert!(matches!(error, DocParsingError::ValueError(field_path, _)
if field_path == "payload.repo"));
}

#[test]
fn test_timestamp_field_in_object_is_valid() {
serde_json::from_str::<DocMapper>(
Expand Down
6 changes: 6 additions & 0 deletions quickwit/quickwit-doc-mapper/src/doc_mapper/mapping_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,9 @@ impl MappingTree {
mapping_leaf.validate_from_json(json_value, field_path)
}
MappingTree::Node(mapping_node) => {
if json_value.is_null() {
return Ok(());
}
if let Some(json_obj) = json_value.as_object() {
mapping_node.validate_from_json(json_obj, strict_mode, field_path)
} else {
Expand All @@ -970,6 +973,9 @@ impl MappingTree {
mapping_leaf.doc_from_json(json_value, document, path)
}
MappingTree::Node(mapping_node) => {
if json_value.is_null() {
return Ok(());
}
if let JsonValue::Object(json_obj) = json_value {
mapping_node.doc_from_json(json_obj, mode, document, path, dynamic_json_obj)
} else {
Expand Down
Loading