Skip to content

Commit 0a8f154

Browse files
Fix: Error rather than silently ignore extra parameter passed to ceil/floor (#18265)
## Which issue does this PR close? <!-- --> - Closes #18175 ## Rationale for this change <!-- --> The Ceil/Floor calls via SQL was being parsed such that they were taking 2 arguments instead of 1, the second of which is not currently needed and the second argument was being ignored and passed silently. ## What changes are included in this PR? <!-- --> The second parameter(`field`) which was being passed if is of the `CeilFloorKind` enum from `sqlparser` crate . Neither of the enum's two variants (`DateTimeField` and `Scale`)are being implemented hence they have been ignored with apporpriate error type and only succeeds if the `DateTimeField` has `NoDateTime` variant i,e it is treated as empty. ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> All Unit Tests pass successfully. --------- Co-authored-by: Andrew Lamb <[email protected]>
1 parent 4ecccde commit 0a8f154

File tree

2 files changed

+42
-11
lines changed

2 files changed

+42
-11
lines changed

datafusion/sql/src/expr/mod.rs

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ use datafusion_expr::planner::{
2020
PlannerResult, RawBinaryExpr, RawDictionaryExpr, RawFieldAccessExpr,
2121
};
2222
use sqlparser::ast::{
23-
AccessExpr, BinaryOperator, CastFormat, CastKind, DataType as SQLDataType,
24-
DictionaryField, Expr as SQLExpr, ExprWithAlias as SQLExprWithAlias, MapEntry,
25-
StructField, Subscript, TrimWhereField, TypedString, Value, ValueWithSpan,
23+
AccessExpr, BinaryOperator, CastFormat, CastKind, CeilFloorKind,
24+
DataType as SQLDataType, DateTimeField, DictionaryField, Expr as SQLExpr,
25+
ExprWithAlias as SQLExprWithAlias, MapEntry, StructField, Subscript, TrimWhereField,
26+
TypedString, Value, ValueWithSpan,
2627
};
2728

2829
use datafusion_common::{
@@ -498,14 +499,28 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
498499
self.sql_grouping_sets_to_expr(exprs, schema, planner_context)
499500
}
500501

501-
SQLExpr::Floor {
502-
expr,
503-
field: _field,
504-
} => self.sql_fn_name_to_expr(*expr, "floor", schema, planner_context),
505-
SQLExpr::Ceil {
506-
expr,
507-
field: _field,
508-
} => self.sql_fn_name_to_expr(*expr, "ceil", schema, planner_context),
502+
SQLExpr::Floor { expr, field } => match field {
503+
CeilFloorKind::DateTimeField(DateTimeField::NoDateTime) => {
504+
self.sql_fn_name_to_expr(*expr, "floor", schema, planner_context)
505+
}
506+
CeilFloorKind::DateTimeField(_) => {
507+
not_impl_err!("FLOOR with datetime is not supported")
508+
}
509+
CeilFloorKind::Scale(_) => {
510+
not_impl_err!("FLOOR with scale is not supported")
511+
}
512+
},
513+
SQLExpr::Ceil { expr, field } => match field {
514+
CeilFloorKind::DateTimeField(DateTimeField::NoDateTime) => {
515+
self.sql_fn_name_to_expr(*expr, "ceil", schema, planner_context)
516+
}
517+
CeilFloorKind::DateTimeField(_) => {
518+
not_impl_err!("CEIL with datetime is not supported")
519+
}
520+
CeilFloorKind::Scale(_) => {
521+
not_impl_err!("CEIL with scale is not supported")
522+
}
523+
},
509524
SQLExpr::Overlay {
510525
expr,
511526
overlay_what,

datafusion/sqllogictest/test_files/scalar.slt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,14 @@ select ceil(a), ceil(b), ceil(c) from small_floats;
309309
1 0 0
310310
1 0 1
311311

312+
# ceil with scale parameter(Scale not supported)
313+
query error DataFusion error: This feature is not implemented: CEIL with scale is not supported
314+
select ceil(100.1234, 1)
315+
316+
# ceil with datetime parameter (not supported)
317+
query error DataFusion error: This feature is not implemented: CEIL with datetime is not supported
318+
select ceil(100.1234 to year)
319+
312320
## degrees
313321

314322
# degrees scalar function
@@ -448,6 +456,14 @@ select floor(a), floor(b), floor(c) from signed_integers;
448456
2 -1000 123
449457
4 NULL NULL
450458

459+
# floor with scale parameter(Scale not supported)
460+
query error DataFusion error: This feature is not implemented: FLOOR with scale is not supported
461+
select floor(a, 1)
462+
463+
# floor with datetime parameter ( not supported)
464+
query error DataFusion error: This feature is not implemented: FLOOR with datetime is not supported
465+
select floor(a to year)
466+
451467
## ln
452468

453469
# ln scalar function

0 commit comments

Comments
 (0)