Skip to content
This repository was archived by the owner on Jan 20, 2023. It is now read-only.

Commit 330c809

Browse files
jimexistalamb
jimexist
and
alamb
authoredJun 18, 2021
fix clippy warnings (apache#581)
* fix clippy warnings * fix test case * upgrade tarpaulin * Disable coverage job while it is failing Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
1 parent 4a55364 commit 330c809

File tree

12 files changed

+68
-68
lines changed

12 files changed

+68
-68
lines changed
 

‎.github/workflows/rust.yml

+39-37
Original file line numberDiff line numberDiff line change
@@ -321,41 +321,43 @@ jobs:
321321
# Ignore MIRI errors until we can get a clean run
322322
cargo miri test || true
323323
324-
coverage:
325-
name: Coverage
326-
runs-on: ubuntu-latest
327-
strategy:
328-
matrix:
329-
arch: [amd64]
330-
rust: [stable]
331-
steps:
332-
- uses: actions/checkout@v2
333-
with:
334-
submodules: true
335-
- name: Cache Cargo
336-
uses: actions/cache@v2
337-
with:
338-
path: /home/runner/.cargo
339-
# this key is not equal because the user is different than on a container (runner vs github)
340-
key: cargo-coverage-cache-
341-
- name: Cache Rust dependencies
342-
uses: actions/cache@v2
343-
with:
344-
path: /home/runner/target
345-
# this key is not equal because coverage uses different compilation flags.
346-
key: ${{ runner.os }}-${{ matrix.arch }}-target-coverage-cache-${{ matrix.rust }}-
347-
- name: Run coverage
348-
run: |
349-
export ARROW_TEST_DATA=$(pwd)/testing/data
350-
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
324+
# Coverage job was failing. https://github.com/apache/arrow-datafusion/issues/590 tracks re-instating it
351325

352-
# 2020-11-15: There is a cargo-tarpaulin regression in 0.17.0
353-
# see https://github.com/xd009642/tarpaulin/issues/618
354-
cargo install --version 0.16.0 cargo-tarpaulin
355-
cargo tarpaulin --out Xml
356-
env:
357-
CARGO_HOME: "/home/runner/.cargo"
358-
CARGO_TARGET_DIR: "/home/runner/target"
359-
- name: Report coverage
360-
continue-on-error: true
361-
run: bash <(curl -s https://codecov.io/bash)
326+
# coverage:
327+
# name: Coverage
328+
# runs-on: ubuntu-latest
329+
# strategy:
330+
# matrix:
331+
# arch: [amd64]
332+
# rust: [stable]
333+
# steps:
334+
# - uses: actions/checkout@v2
335+
# with:
336+
# submodules: true
337+
# - name: Cache Cargo
338+
# uses: actions/cache@v2
339+
# with:
340+
# path: /home/runner/.cargo
341+
# # this key is not equal because the user is different than on a container (runner vs github)
342+
# key: cargo-coverage-cache-
343+
# - name: Cache Rust dependencies
344+
# uses: actions/cache@v2
345+
# with:
346+
# path: /home/runner/target
347+
# # this key is not equal because coverage uses different compilation flags.
348+
# key: ${{ runner.os }}-${{ matrix.arch }}-target-coverage-cache-${{ matrix.rust }}-
349+
# - name: Run coverage
350+
# run: |
351+
# export ARROW_TEST_DATA=$(pwd)/testing/data
352+
# export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
353+
354+
# # 2020-11-15: There is a cargo-tarpaulin regression in 0.17.0
355+
# # see https://github.com/xd009642/tarpaulin/issues/618
356+
# cargo install --version 0.16.0 cargo-tarpaulin
357+
# cargo tarpaulin --out Xml
358+
# env:
359+
# CARGO_HOME: "/home/runner/.cargo"
360+
# CARGO_TARGET_DIR: "/home/runner/target"
361+
# - name: Report coverage
362+
# continue-on-error: true
363+
# run: bash <(curl -s https://codecov.io/bash)

‎ballista/rust/core/src/serde/scheduler/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ impl PartitionStats {
142142
]
143143
}
144144

145-
pub fn to_arrow_arrayref(&self) -> Result<Arc<StructArray>, BallistaError> {
145+
pub fn to_arrow_arrayref(self) -> Result<Arc<StructArray>, BallistaError> {
146146
let mut field_builders = Vec::new();
147147

148148
let mut num_rows_builder = UInt64Builder::new(1);

‎datafusion-cli/src/print_format.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ mod tests {
151151

152152
#[test]
153153
fn test_from_str_failure() {
154-
assert_eq!(true, "pretty".parse::<PrintFormat>().is_err());
154+
assert!("pretty".parse::<PrintFormat>().is_err());
155155
}
156156

157157
#[test]

‎datafusion/src/execution/context.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -1131,18 +1131,15 @@ mod tests {
11311131
let ctx = create_ctx(&tmp_dir, 1)?;
11321132

11331133
let schema: Schema = ctx.table("test").unwrap().schema().clone().into();
1134-
assert_eq!(schema.field_with_name("c1")?.is_nullable(), false);
1134+
assert!(!schema.field_with_name("c1")?.is_nullable());
11351135

11361136
let plan = LogicalPlanBuilder::scan_empty("", &schema, None)?
11371137
.project(vec![col("c1")])?
11381138
.build()?;
11391139

11401140
let plan = ctx.optimize(&plan)?;
11411141
let physical_plan = ctx.create_physical_plan(&Arc::new(plan))?;
1142-
assert_eq!(
1143-
physical_plan.schema().field_with_name("c1")?.is_nullable(),
1144-
false
1145-
);
1142+
assert!(!physical_plan.schema().field_with_name("c1")?.is_nullable());
11461143
Ok(())
11471144
}
11481145

‎datafusion/src/logical_plan/dfschema.rs

+2
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,14 @@ where
248248
}
249249

250250
impl ToDFSchema for Schema {
251+
#[allow(clippy::wrong_self_convention)]
251252
fn to_dfschema(self) -> Result<DFSchema> {
252253
DFSchema::try_from(self)
253254
}
254255
}
255256

256257
impl ToDFSchema for SchemaRef {
258+
#[allow(clippy::wrong_self_convention)]
257259
fn to_dfschema(self) -> Result<DFSchema> {
258260
// Attempt to use the Schema directly if there are no other
259261
// references, otherwise clone

‎datafusion/src/logical_plan/display.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ impl<'a, 'b> PlanVisitor for GraphvizVisitor<'a, 'b> {
197197
// id [label="foo"]
198198
let label = if self.with_schema {
199199
format!(
200-
"{}\\nSchema: {}",
200+
r"{}\nSchema: {}",
201201
plan.display(),
202202
display_schema(&plan.schema().as_ref().to_owned().into())
203203
)

‎datafusion/src/optimizer/utils.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ mod tests {
551551
stringified_plans,
552552
..
553553
} => {
554-
assert_eq!(*verbose, true);
554+
assert!(*verbose);
555555

556556
let expected_stringified_plans = vec![
557557
StringifiedPlan::new(PlanType::LogicalPlan, "..."),

‎datafusion/src/physical_plan/expressions/not.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ mod tests {
129129

130130
let expr = not(col("a"), &schema)?;
131131
assert_eq!(expr.data_type(&schema)?, DataType::Boolean);
132-
assert_eq!(expr.nullable(&schema)?, true);
132+
assert!(expr.nullable(&schema)?);
133133

134134
let input = BooleanArray::from(vec![Some(true), None, Some(false)]);
135135
let expected = &BooleanArray::from(vec![Some(false), None, Some(true)]);

‎datafusion/src/physical_plan/hash_join.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -684,11 +684,10 @@ fn build_join_indexes(
684684
&keys_values,
685685
)? {
686686
left_indices.append_value(i)?;
687-
right_indices.append_value(row as u32)?;
688687
} else {
689688
left_indices.append_null()?;
690-
right_indices.append_value(row as u32)?;
691689
}
690+
right_indices.append_value(row as u32)?;
692691
}
693692
}
694693
None => {

‎datafusion/src/physical_plan/regex_expressions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub fn regexp_match<T: StringOffsetSizeTrait>(args: &[ArrayRef]) -> Result<Array
6262
/// used by regexp_replace
6363
fn regex_replace_posix_groups(replacement: &str) -> String {
6464
lazy_static! {
65-
static ref CAPTURE_GROUPS_RE: Regex = Regex::new("(\\\\)(\\d*)").unwrap();
65+
static ref CAPTURE_GROUPS_RE: Regex = Regex::new(r"(\\)(\d*)").unwrap();
6666
}
6767
CAPTURE_GROUPS_RE
6868
.replace_all(replacement, "$${$2}")

‎datafusion/src/scalar.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,7 @@ mod tests {
11231123
let array = value.to_array();
11241124
let array = array.as_any().downcast_ref::<UInt64Array>().unwrap();
11251125
assert_eq!(array.len(), 1);
1126-
assert_eq!(false, array.is_null(0));
1126+
assert!(!array.is_null(0));
11271127
assert_eq!(array.value(0), 13);
11281128

11291129
let value = ScalarValue::UInt64(None);
@@ -1139,7 +1139,7 @@ mod tests {
11391139
let array = value.to_array();
11401140
let array = array.as_any().downcast_ref::<UInt32Array>().unwrap();
11411141
assert_eq!(array.len(), 1);
1142-
assert_eq!(false, array.is_null(0));
1142+
assert!(!array.is_null(0));
11431143
assert_eq!(array.value(0), 13);
11441144

11451145
let value = ScalarValue::UInt32(None);

‎datafusion/src/sql/planner.rs

+16-16
Original file line numberDiff line numberDiff line change
@@ -1653,7 +1653,7 @@ mod tests {
16531653
let err = logical_plan(sql).expect_err("query should have failed");
16541654
assert_eq!(
16551655
format!(
1656-
"Plan(\"Invalid identifier \\\'doesnotexist\\\' for schema {}\")",
1656+
r#"Plan("Invalid identifier 'doesnotexist' for schema {}")"#,
16571657
PERSON_COLUMN_NAMES
16581658
),
16591659
format!("{:?}", err)
@@ -1665,7 +1665,7 @@ mod tests {
16651665
let sql = "SELECT age, age FROM person";
16661666
let err = logical_plan(sql).expect_err("query should have failed");
16671667
assert_eq!(
1668-
"Plan(\"Projections require unique expression names but the expression \\\"#age\\\" at position 0 and \\\"#age\\\" at position 1 have the same name. Consider aliasing (\\\"AS\\\") one of them.\")",
1668+
r##"Plan("Projections require unique expression names but the expression \"#age\" at position 0 and \"#age\" at position 1 have the same name. Consider aliasing (\"AS\") one of them.")"##,
16691669
format!("{:?}", err)
16701670
);
16711671
}
@@ -1675,7 +1675,7 @@ mod tests {
16751675
let sql = "SELECT *, age FROM person";
16761676
let err = logical_plan(sql).expect_err("query should have failed");
16771677
assert_eq!(
1678-
"Plan(\"Projections require unique expression names but the expression \\\"#age\\\" at position 3 and \\\"#age\\\" at position 8 have the same name. Consider aliasing (\\\"AS\\\") one of them.\")",
1678+
r##"Plan("Projections require unique expression names but the expression \"#age\" at position 3 and \"#age\" at position 8 have the same name. Consider aliasing (\"AS\") one of them.")"##,
16791679
format!("{:?}", err)
16801680
);
16811681
}
@@ -1714,7 +1714,7 @@ mod tests {
17141714
let err = logical_plan(sql).expect_err("query should have failed");
17151715
assert_eq!(
17161716
format!(
1717-
"Plan(\"Invalid identifier \\\'doesnotexist\\\' for schema {}\")",
1717+
r#"Plan("Invalid identifier 'doesnotexist' for schema {}")"#,
17181718
PERSON_COLUMN_NAMES
17191719
),
17201720
format!("{:?}", err)
@@ -1727,7 +1727,7 @@ mod tests {
17271727
let err = logical_plan(sql).expect_err("query should have failed");
17281728
assert_eq!(
17291729
format!(
1730-
"Plan(\"Invalid identifier \\\'x\\\' for schema {}\")",
1730+
r#"Plan("Invalid identifier 'x' for schema {}")"#,
17311731
PERSON_COLUMN_NAMES
17321732
),
17331733
format!("{:?}", err)
@@ -2200,7 +2200,7 @@ mod tests {
22002200
let err = logical_plan(sql).expect_err("query should have failed");
22012201
assert_eq!(
22022202
format!(
2203-
"Plan(\"Invalid identifier \\\'doesnotexist\\\' for schema {}\")",
2203+
r#"Plan("Invalid identifier 'doesnotexist' for schema {}")"#,
22042204
PERSON_COLUMN_NAMES
22052205
),
22062206
format!("{:?}", err)
@@ -2212,7 +2212,7 @@ mod tests {
22122212
let sql = "SELECT MIN(age), MIN(age) FROM person";
22132213
let err = logical_plan(sql).expect_err("query should have failed");
22142214
assert_eq!(
2215-
"Plan(\"Projections require unique expression names but the expression \\\"#MIN(age)\\\" at position 0 and \\\"#MIN(age)\\\" at position 1 have the same name. Consider aliasing (\\\"AS\\\") one of them.\")",
2215+
r##"Plan("Projections require unique expression names but the expression \"#MIN(age)\" at position 0 and \"#MIN(age)\" at position 1 have the same name. Consider aliasing (\"AS\") one of them.")"##,
22162216
format!("{:?}", err)
22172217
);
22182218
}
@@ -2242,7 +2242,7 @@ mod tests {
22422242
let sql = "SELECT MIN(age) AS a, MIN(age) AS a FROM person";
22432243
let err = logical_plan(sql).expect_err("query should have failed");
22442244
assert_eq!(
2245-
"Plan(\"Projections require unique expression names but the expression \\\"#MIN(age) AS a\\\" at position 0 and \\\"#MIN(age) AS a\\\" at position 1 have the same name. Consider aliasing (\\\"AS\\\") one of them.\")",
2245+
r##"Plan("Projections require unique expression names but the expression \"#MIN(age) AS a\" at position 0 and \"#MIN(age) AS a\" at position 1 have the same name. Consider aliasing (\"AS\") one of them.")"##,
22462246
format!("{:?}", err)
22472247
);
22482248
}
@@ -2272,7 +2272,7 @@ mod tests {
22722272
let sql = "SELECT state AS a, MIN(age) AS a FROM person GROUP BY state";
22732273
let err = logical_plan(sql).expect_err("query should have failed");
22742274
assert_eq!(
2275-
"Plan(\"Projections require unique expression names but the expression \\\"#state AS a\\\" at position 0 and \\\"#MIN(age) AS a\\\" at position 1 have the same name. Consider aliasing (\\\"AS\\\") one of them.\")",
2275+
r##"Plan("Projections require unique expression names but the expression \"#state AS a\" at position 0 and \"#MIN(age) AS a\" at position 1 have the same name. Consider aliasing (\"AS\") one of them.")"##,
22762276
format!("{:?}", err)
22772277
);
22782278
}
@@ -2293,7 +2293,7 @@ mod tests {
22932293
let err = logical_plan(sql).expect_err("query should have failed");
22942294
assert_eq!(
22952295
format!(
2296-
"Plan(\"Invalid identifier \\\'doesnotexist\\\' for schema {}\")",
2296+
r#"Plan("Invalid identifier 'doesnotexist' for schema {}")"#,
22972297
PERSON_COLUMN_NAMES
22982298
),
22992299
format!("{:?}", err)
@@ -2306,7 +2306,7 @@ mod tests {
23062306
let err = logical_plan(sql).expect_err("query should have failed");
23072307
assert_eq!(
23082308
format!(
2309-
"Plan(\"Invalid identifier \\\'doesnotexist\\\' for schema {}\")",
2309+
r#"Plan("Invalid identifier 'doesnotexist' for schema {}")"#,
23102310
PERSON_COLUMN_NAMES
23112311
),
23122312
format!("{:?}", err)
@@ -2318,7 +2318,7 @@ mod tests {
23182318
let sql = "SELECT INTERVAL '100000000000000000 day'";
23192319
let err = logical_plan(sql).expect_err("query should have failed");
23202320
assert_eq!(
2321-
"NotImplemented(\"Interval field value out of range: \\\"100000000000000000 day\\\"\")",
2321+
r#"NotImplemented("Interval field value out of range: \"100000000000000000 day\"")"#,
23222322
format!("{:?}", err)
23232323
);
23242324
}
@@ -2328,7 +2328,7 @@ mod tests {
23282328
let sql = "SELECT INTERVAL '1 year 1 day'";
23292329
let err = logical_plan(sql).expect_err("query should have failed");
23302330
assert_eq!(
2331-
"NotImplemented(\"DF does not support intervals that have both a Year/Month part as well as Days/Hours/Mins/Seconds: \\\"1 year 1 day\\\". Hint: try breaking the interval into two parts, one with Year/Month and the other with Days/Hours/Mins/Seconds - e.g. (NOW() + INTERVAL \\\'1 year\\\') + INTERVAL \\\'1 day\\\'\")",
2331+
r#"NotImplemented("DF does not support intervals that have both a Year/Month part as well as Days/Hours/Mins/Seconds: \"1 year 1 day\". Hint: try breaking the interval into two parts, one with Year/Month and the other with Days/Hours/Mins/Seconds - e.g. (NOW() + INTERVAL '1 year') + INTERVAL '1 day'")"#,
23322332
format!("{:?}", err)
23332333
);
23342334
}
@@ -2391,7 +2391,7 @@ mod tests {
23912391
let sql = "SELECT state, MIN(age), MIN(age) FROM person GROUP BY state";
23922392
let err = logical_plan(sql).expect_err("query should have failed");
23932393
assert_eq!(
2394-
"Plan(\"Projections require unique expression names but the expression \\\"#MIN(age)\\\" at position 1 and \\\"#MIN(age)\\\" at position 2 have the same name. Consider aliasing (\\\"AS\\\") one of them.\")",
2394+
r##"Plan("Projections require unique expression names but the expression \"#MIN(age)\" at position 1 and \"#MIN(age)\" at position 2 have the same name. Consider aliasing (\"AS\") one of them.")"##,
23952395
format!("{:?}", err)
23962396
);
23972397
}
@@ -2451,7 +2451,7 @@ mod tests {
24512451
"SELECT ((age + 1) / 2) * (age + 9), MIN(first_name) FROM person GROUP BY age + 1";
24522452
let err = logical_plan(sql).expect_err("query should have failed");
24532453
assert_eq!(
2454-
"Plan(\"Projection references non-aggregate values\")",
2454+
r#"Plan("Projection references non-aggregate values")"#,
24552455
format!("{:?}", err)
24562456
);
24572457
}
@@ -2462,7 +2462,7 @@ mod tests {
24622462
let sql = "SELECT age, MIN(first_name) FROM person GROUP BY age + 1";
24632463
let err = logical_plan(sql).expect_err("query should have failed");
24642464
assert_eq!(
2465-
"Plan(\"Projection references non-aggregate values\")",
2465+
r#"Plan("Projection references non-aggregate values")"#,
24662466
format!("{:?}", err)
24672467
);
24682468
}

0 commit comments

Comments
 (0)
This repository has been archived.