Skip to content

Commit e47e2f1

Browse files
authored
fix panic when decode a group with no child (#5322)
* fix panic when decode group with no child * remove copied comment in test
1 parent b594d90 commit e47e2f1

File tree

1 file changed

+44
-14
lines changed

1 file changed

+44
-14
lines changed

parquet/src/schema/types.rs

+44-14
Original file line numberDiff line numberDiff line change
@@ -1085,20 +1085,38 @@ fn from_thrift_helper(elements: &[SchemaElement], index: usize) -> Result<(usize
10851085
));
10861086
}
10871087
let repetition = Repetition::try_from(elements[index].repetition_type.unwrap())?;
1088-
let physical_type = PhysicalType::try_from(elements[index].type_.unwrap())?;
1089-
let length = elements[index].type_length.unwrap_or(-1);
1090-
let scale = elements[index].scale.unwrap_or(-1);
1091-
let precision = elements[index].precision.unwrap_or(-1);
1092-
let name = &elements[index].name;
1093-
let builder = Type::primitive_type_builder(name, physical_type)
1094-
.with_repetition(repetition)
1095-
.with_converted_type(converted_type)
1096-
.with_logical_type(logical_type)
1097-
.with_length(length)
1098-
.with_precision(precision)
1099-
.with_scale(scale)
1100-
.with_id(field_id);
1101-
Ok((index + 1, Arc::new(builder.build()?)))
1088+
if let Some(type_) = elements[index].type_ {
1089+
let physical_type = PhysicalType::try_from(type_)?;
1090+
let length = elements[index].type_length.unwrap_or(-1);
1091+
let scale = elements[index].scale.unwrap_or(-1);
1092+
let precision = elements[index].precision.unwrap_or(-1);
1093+
let name = &elements[index].name;
1094+
let builder = Type::primitive_type_builder(name, physical_type)
1095+
.with_repetition(repetition)
1096+
.with_converted_type(converted_type)
1097+
.with_logical_type(logical_type)
1098+
.with_length(length)
1099+
.with_precision(precision)
1100+
.with_scale(scale)
1101+
.with_id(field_id);
1102+
Ok((index + 1, Arc::new(builder.build()?)))
1103+
} else {
1104+
let mut builder = Type::group_type_builder(&elements[index].name)
1105+
.with_converted_type(converted_type)
1106+
.with_logical_type(logical_type)
1107+
.with_id(field_id);
1108+
if !is_root_node {
1109+
// Sometimes parquet-cpp and parquet-mr set repetition level REQUIRED or
1110+
// REPEATED for root node.
1111+
//
1112+
// We only set repetition for group types that are not top-level message
1113+
// type. According to parquet-format:
1114+
// Root of the schema does not have a repetition_type.
1115+
// All other types must have one.
1116+
builder = builder.with_repetition(repetition);
1117+
}
1118+
Ok((index + 1, Arc::new(builder.build().unwrap())))
1119+
}
11021120
}
11031121
Some(n) => {
11041122
let repetition = elements[index]
@@ -2138,4 +2156,16 @@ mod tests {
21382156
let result_schema = from_thrift(&thrift_schema).unwrap();
21392157
assert_eq!(result_schema, Arc::new(expected_schema));
21402158
}
2159+
2160+
#[test]
2161+
fn test_schema_from_thrift_group_has_no_child() {
2162+
let message_type = "message schema {}";
2163+
2164+
let expected_schema = parse_message_type(message_type).unwrap();
2165+
let mut thrift_schema = to_thrift(&expected_schema).unwrap();
2166+
thrift_schema[0].repetition_type = Some(Repetition::REQUIRED.into());
2167+
2168+
let result_schema = from_thrift(&thrift_schema).unwrap();
2169+
assert_eq!(result_schema, Arc::new(expected_schema));
2170+
}
21412171
}

0 commit comments

Comments
 (0)