Skip to content
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.

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

build: modify code to comply with latest clippy requirement #9725

Merged
merged 4 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion datafusion/common/src/scalar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4539,7 +4539,8 @@ mod tests {
// The alignment requirements differ across architectures and
// thus the size of the enum appears to as well

assert_eq!(std::mem::size_of::<ScalarValue>(), 48);
// The value can be changed depending on rust version
assert_eq!(std::mem::size_of::<ScalarValue>(), 64);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting change -- basically it means that all ScalarValue based code will potentially consume significantly more memory (as now each ScalarValue takes 64 bytes rather than 48). Interestingly, this is the same behavior as aarch64 in previous rust versions (you can see this test is cfg'd off with

    #[cfg(not(target_arch = "aarch64"))]

I'll make a follow on PR to remove this cfg

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use tokio::task::JoinSet;
/// same results
#[tokio::test(flavor = "multi_thread")]
async fn streaming_aggregate_test() {
let test_cases = vec![
let test_cases = [
vec!["a"],
vec!["b", "a"],
vec!["c", "a"],
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/expr_rewriter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ mod test {
let expr = col("a") + col("b");
let schema_a =
make_schema_with_empty_metadata(vec![make_field("\"tableA\"", "a")]);
let schemas = vec![schema_a];
let schemas = [schema_a];
let schemas = schemas.iter().collect::<Vec<_>>();

let error =
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions/benches/regx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn data(rng: &mut ThreadRng) -> StringArray {
}

fn regex(rng: &mut ThreadRng) -> StringArray {
let samples = vec![
let samples = [
".*([A-Z]{1}).*".to_string(),
"^(A).*".to_string(),
r#"[\p{Letter}-]+"#.to_string(),
Expand All @@ -60,7 +60,7 @@ fn regex(rng: &mut ThreadRng) -> StringArray {
}

fn flags(rng: &mut ThreadRng) -> StringArray {
let samples = vec![Some("i".to_string()), Some("im".to_string()), None];
let samples = [Some("i".to_string()), Some("im".to_string()), None];
let mut sb = StringBuilder::new();
for _ in 0..1000 {
let sample = samples.choose(rng).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions/benches/to_char.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn data(rng: &mut ThreadRng) -> Date32Array {
}

fn patterns(rng: &mut ThreadRng) -> StringArray {
let samples = vec![
let samples = [
"%Y:%m:%d".to_string(),
"%d-%m-%Y".to_string(),
"%d%m%Y".to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,12 @@ struct ConstEvaluator<'a> {
input_batch: RecordBatch,
}

#[allow(dead_code)]
/// The simplify result of ConstEvaluator
enum ConstSimplifyResult {
// Expr was simplifed and contains the new expression
Simplified(ScalarValue),
// Evalaution encountered an error, contains the original expression
// Evaluation encountered an error, contains the original expression
SimplifyRuntimeError(DataFusionError, Expr),
}

Expand Down
6 changes: 3 additions & 3 deletions datafusion/physical-expr/src/equivalence/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,19 +535,19 @@ mod tests {

#[test]
fn test_remove_redundant_entries_eq_group() -> Result<()> {
let entries = vec![
let entries = [
EquivalenceClass::new(vec![lit(1), lit(1), lit(2)]),
// This group is meaningless should be removed
EquivalenceClass::new(vec![lit(3), lit(3)]),
EquivalenceClass::new(vec![lit(4), lit(5), lit(6)]),
];
// Given equivalences classes are not in succinct form.
// Expected form is the most plain representation that is functionally same.
let expected = vec![
let expected = [
EquivalenceClass::new(vec![lit(1), lit(2)]),
EquivalenceClass::new(vec![lit(4), lit(5), lit(6)]),
];
let mut eq_groups = EquivalenceGroup::new(entries);
let mut eq_groups = EquivalenceGroup::new(entries.to_vec());
eq_groups.remove_redundant_entries();

let eq_groups = eq_groups.classes;
Expand Down
4 changes: 2 additions & 2 deletions datafusion/physical-expr/src/equivalence/ordering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ mod tests {
// Generate a data that satisfies properties given
let table_data_with_properties =
generate_table_for_eq_properties(&eq_properties, N_ELEMENTS, N_DISTINCT)?;
let col_exprs = vec![
let col_exprs = [
col("a", &test_schema)?,
col("b", &test_schema)?,
col("c", &test_schema)?,
Expand Down Expand Up @@ -815,7 +815,7 @@ mod tests {
Operator::Plus,
col("b", &test_schema)?,
)) as Arc<dyn PhysicalExpr>;
let exprs = vec![
let exprs = [
col("a", &test_schema)?,
col("b", &test_schema)?,
col("c", &test_schema)?,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-expr/src/equivalence/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,7 @@ mod tests {
Operator::Plus,
col("b", &test_schema)?,
)) as Arc<dyn PhysicalExpr>;
let exprs = vec![
let exprs = [
col("a", &test_schema)?,
col("b", &test_schema)?,
col("c", &test_schema)?,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-plan/src/sorts/partial_sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ mod tests {
#[tokio::test]
async fn test_partial_sort2() -> Result<()> {
let task_ctx = Arc::new(TaskContext::default());
let source_tables = vec![
let source_tables = [
test::build_table_scan_i32(
("a", &vec![0, 0, 0, 0, 1, 1, 1, 1]),
("b", &vec![1, 1, 3, 3, 4, 4, 2, 2]),
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-plan/src/union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ mod tests {
let col_e = &col("e", &schema)?;
let col_f = &col("f", &schema)?;
let options = SortOptions::default();
let test_cases = vec![
let test_cases = [
//-----------TEST CASE 1----------//
(
// First child orderings
Expand Down
1 change: 1 addition & 0 deletions datafusion/substrait/src/serializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use substrait::proto::Plan;
use std::fs::OpenOptions;
use std::io::{Read, Write};

#[allow(clippy::suspicious_open_options)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@waynexia please check this

I suppressed the warning but clippy requires .truncate to be called before file creation

  --> datafusion/substrait/src/serializer.rs:32:39
   |
32 |     let mut file = OpenOptions::new().create(true).write(true).open(path)?;
   |                                       ^^^^^^^^^^^^- help: add: `.truncate(true)`
   |
   = help: if you intend to overwrite an existing file entirely, call `.truncate(true)`
   = help: if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`
   = help: alternatively, use `.append(true)` to append to the file instead of overwriting it
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_open_options
   = note: `-D clippy::suspicious-open-options` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #9727 to track

pub async fn serialize(sql: &str, ctx: &SessionContext, path: &str) -> Result<()> {
let protobuf_out = serialize_bytes(sql, ctx).await;
let mut file = OpenOptions::new().create(true).write(true).open(path)?;
Expand Down
Loading