Copilot commented on code in PR #2077:
URL: https://github.com/apache/auron/pull/2077#discussion_r2915617679
##########
native-engine/datafusion-ext-plans/src/flink/serde/pb_deserializer.rs:
##########
@@ -284,12 +284,11 @@ fn transfer_output_schema_to_pb_schema(
let index_start = field_name.find(".");
if let Some(index) = index_start {
let msg_field_name = &field_name[..index];
- let msg_field_desc =
- message_descriptor
- .get_field_by_name(msg_field_name)
- .expect(&format!(
- "nested field {msg_field_name} not exits in
message_descriptor"
- ));
+ let msg_field_desc = message_descriptor
+ .get_field_by_name(msg_field_name)
+ .unwrap_or_else(|| {
+ panic!("nested field {msg_field_name} not exists in
message_descriptor")
+ });
Review Comment:
Grammar: the panic message says "not exists". Consider changing to "does not
exist" for clearer/grammatically correct diagnostics (e.g., "nested field ...
does not exist in message_descriptor").
##########
native-engine/datafusion-ext-plans/src/flink/serde/pb_deserializer.rs:
##########
@@ -318,18 +317,19 @@ fn transfer_output_schema_to_pb_schema(
return df_execution_err!("not message field");
}
} else {
- let msg_field_desc =
- message_descriptor
- .get_field_by_name(field_name)
- .expect(&format!(
- "nested innermost field {field_name} not exits in
message_descriptor"
- ));
+ let msg_field_desc = message_descriptor
+ .get_field_by_name(field_name)
+ .unwrap_or_else(|| {
+ panic!(
+ "nested innermost field {field_name} not exists in
message_descriptor"
+ )
+ });
Review Comment:
Grammar: the panic message says "not exists". Consider changing to "does not
exist" for clearer/grammatically correct diagnostics.
##########
native-engine/datafusion-ext-plans/src/orc_exec.rs:
##########
@@ -1434,7 +1434,7 @@ mod tests {
col_name,
schema
.index_of(col_name)
- .expect(&format!("Column '{col_name}' not found")),
+ .unwrap_or_else(|_| panic!("Column '{col_name}' not
found")),
Review Comment:
`schema.index_of(col_name)` returns an error that’s currently discarded
(`|_|`). Consider including the error value in the panic message to make test
failures easier to diagnose.
```suggestion
.unwrap_or_else(|e| panic!("Column '{col_name}' not
found: {e}")),
```
##########
native-engine/datafusion-ext-plans/src/flink/serde/pb_deserializer.rs:
##########
@@ -318,18 +317,19 @@ fn transfer_output_schema_to_pb_schema(
return df_execution_err!("not message field");
}
} else {
- let msg_field_desc =
- message_descriptor
- .get_field_by_name(field_name)
- .expect(&format!(
- "nested innermost field {field_name} not exits in
message_descriptor"
- ));
+ let msg_field_desc = message_descriptor
+ .get_field_by_name(field_name)
+ .unwrap_or_else(|| {
+ panic!(
+ "nested innermost field {field_name} not exists in
message_descriptor"
+ )
+ });
pb_schema_fields.push(create_arrow_field(msg_field_desc.clone(), skip_fields));
}
} else {
let msg_field_desc = message_descriptor
.get_field_by_name(field.name())
- .expect(&format!("{} not exits in message_descriptor",
field.name()));
+ .unwrap_or_else(|| panic!("{} not exists in
message_descriptor", field.name()));
pb_schema_fields.push(create_arrow_field(msg_field_desc.clone(),
skip_fields));
Review Comment:
Grammar: the panic message says "not exists". Consider changing to "does not
exist" for clearer/grammatically correct diagnostics.
##########
native-engine/datafusion-ext-plans/src/shuffle/buffered_data.rs:
##########
@@ -301,7 +301,7 @@ fn sort_batches_by_partition_id(
Partitioning::HashPartitioning(..) => {
// compute partition indices
let hashes = evaluate_hashes(partitioning, &batch)
- .expect(&format!("error evaluating hashes with
{partitioning}"));
+ .unwrap_or_else(|_| panic!("error evaluating hashes
with {partitioning}"));
evaluate_partition_ids(hashes,
partitioning.partition_count())
Review Comment:
The error from `evaluate_hashes` is being discarded (`|_|`). Consider
capturing the error (`|e|`) and including it in the panic message so failures
have actionable context without reintroducing an eager allocation.
##########
native-engine/datafusion-ext-plans/src/flink/serde/pb_deserializer.rs:
##########
@@ -549,9 +549,7 @@ fn create_output_array_builders(
let field_name = field.name();
let field_desc = message_descriptor
.get_field_by_name(field_name)
- .expect(&format!(
- "Field {field_name} not exits in message_descriptor",
- ));
+ .unwrap_or_else(|| panic!("Field {field_name} not exists in
message_descriptor",));
Review Comment:
Grammar: the panic message says "not exists". Consider changing to "does not
exist" for clearer/grammatically correct diagnostics.
```suggestion
.unwrap_or_else(|| panic!("Field {field_name} does not exist in
message_descriptor",));
```
--
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]
