findepi commented on code in PR #17135:
URL: https://github.com/apache/datafusion/pull/17135#discussion_r2272672694
##########
datafusion/physical-expr/src/async_scalar_function.rs:
##########
@@ -192,13 +192,14 @@ impl AsyncFuncExpr {
);
}
- let datas = ColumnarValue::values_to_arrays(&result_batches)?
+ let data_vec = ColumnarValue::values_to_arrays(&result_batches)?
Review Comment:
there is no such word `datas`, but it's likely as meaningful as `data_vec`
it could be allowed (no need to change anything)
##########
datafusion/sql/src/expr/function.rs:
##########
@@ -94,7 +94,7 @@ struct FunctionArgs {
/// WITHIN GROUP clause, if any
within_group: Vec<OrderByExpr>,
/// Was the function called without parenthesis, i.e. could this also be a
column reference?
- function_without_paranthesis: bool,
+ function_without_parenthesis: bool,
Review Comment:
This actually should be `parentheses` (plural)
A function call syntax includes two parentheses and there might be a special
syntax for a call without any parentheses (eg `SELECT current_user`), but there
is no syntax to call a function with just 1 parenthesis.
##########
datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs:
##########
@@ -403,7 +403,7 @@ async fn run_aggregate_test(input1: Vec<RecordBatch>,
group_by_columns: Vec<&str
Left Plan:\n{}\n\
Right Plan:\n{}\n\
schema:\n{schema}\n\
- Left Ouptut:\n{}\n\
Review Comment:
❤️
##########
datafusion/sql/src/expr/function.rs:
##########
@@ -94,7 +94,7 @@ struct FunctionArgs {
/// WITHIN GROUP clause, if any
within_group: Vec<OrderByExpr>,
/// Was the function called without parenthesis, i.e. could this also be a
column reference?
- function_without_paranthesis: bool,
+ function_without_parenthesis: bool,
Review Comment:
```suggestion
function_without_parentheses: bool,
```
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -738,8 +738,8 @@ fn plan_update() {
}
#[rstest]
-#[case::missing_assignement_target("UPDATE person SET doesnotexist = true")]
-#[case::missing_assignement_expression("UPDATE person SET age = doesnotexist +
42")]
+#[case::missing_assignment_target("UPDATE person SET doesnotexist = true")]
+#[case::missing_assignment_expression("UPDATE person SET age = doesnotexist +
42")]
Review Comment:
This was clearly a typo. In `rstest`? In our code? Did it work, or should we
just delete these lines?
I would feel better if this change is backed out from this PR, because I
don't understand its consequences
```suggestion
#[case::missing_assignement_target("UPDATE person SET doesnotexist = true")]
#[case::missing_assignement_expression("UPDATE person SET age = doesnotexist
+ 42")]
```
##########
typos.toml:
##########
@@ -0,0 +1,39 @@
+[default.extend-words]
+Pn = "Pn"
+fo = "fo"
+flate = "flate"
+nd = "nd"
+Nd = "Nd"
+YOUY = "YOUY"
+typ = "typ"
Review Comment:
this is perhaps fine as `type` is reserved. worth adding a comment line that
`typ` stands for `type`
##########
datafusion/physical-plan/src/metrics/baseline.rs:
##########
@@ -173,15 +173,15 @@ impl SpillMetrics {
#[derive(Debug, Clone)]
pub struct SplitMetrics {
/// Number of times an input [`RecordBatch`] was split
- pub batches_splitted: Count,
Review Comment:
This looks like an `api change`
##########
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:
##########
@@ -398,7 +398,7 @@ fn roundtrip_window() -> Result<()> {
let args = vec![cast(col("a", &schema)?, &schema, DataType::Float64)?];
let sum_expr = AggregateExprBuilder::new(sum_udaf(), args)
.schema(Arc::clone(&schema))
- .alias("SUM(a) RANGE BETWEEN CURRENT ROW AND UNBOUNDED PRECEEDING")
+ .alias("SUM(a) RANGE BETWEEN CURRENT ROW AND UNBOUNDED PROCEEDING")
Review Comment:
`PRECEDING`
```suggestion
.alias("SUM(a) RANGE BETWEEN CURRENT ROW AND UNBOUNDED PRECEDING")
```
##########
typos.toml:
##########
@@ -0,0 +1,39 @@
+[default.extend-words]
+Pn = "Pn"
+fo = "fo"
+flate = "flate"
+nd = "nd"
+Nd = "Nd"
+YOUY = "YOUY"
+typ = "typ"
+ba = "ba"
+lits = "lits"
+ECT = "ECT"
+Ue = "Ue"
+Iy = "Iy"
+hte = "hte"
+numer = "numer"
+abd = "abd"
+aroun = "aroun"
+carefull = "carefull"
Review Comment:
most of these words are either typos, or some arbitrary abbreviations.
Let's not add this file in this PR.
Let's add it in a PR that adds the checking workflow.
##########
typos.toml:
##########
@@ -0,0 +1,39 @@
+[default.extend-words]
+Pn = "Pn"
+fo = "fo"
+flate = "flate"
+nd = "nd"
+Nd = "Nd"
+YOUY = "YOUY"
+typ = "typ"
+ba = "ba"
+lits = "lits"
+ECT = "ECT"
+Ue = "Ue"
+Iy = "Iy"
+hte = "hte"
+numer = "numer"
+abd = "abd"
+aroun = "aroun"
+carefull = "carefull"
+abov = "abov"
+Ois = "Ois"
+alo = "alo"
+precentage = "precentage"
+datas = "datas"
+hom = "hom"
+alph = "alph"
+wih = "wih"
+Ded = "Ded"
+Serializeable = "Serializeable"
Review Comment:
Does the checker have a way to exclude given work at given use place? or
only global excludes are available?
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
