harsh-ande commented on code in PR #26905:
URL: https://github.com/apache/datafusion/pull/26905#discussion_r4418938503
##########
datafusion/spark/src/planner.rs:
##########
@@ -14,6 +16,20 @@ impl ExprPlanner for SparkFunctionPlanner {
&self,
args: Vec<Expr>,
) -> datafusion_common::Result<PlannerResult<Vec<Expr>>> {
+ // EXTRACT(SECOND FROM ts) must return DECIMAL(8, 6) like
date_part('SECOND', ts).
+ if let Some(part) = args.first().and_then(expr_string_literal)
+ && is_seconds_part(&part)
+ {
+ let date_expr = args.into_iter().nth(1).ok_or_else(|| {
+ datafusion_common::DataFusionError::Internal(
Review Comment:
Instead of building `DataFusionError::Internal(..)` directly, you can use
`internal_datafusion_err!(..)`.
##########
datafusion/spark/src/function/datetime/date_part.rs:
##########
@@ -114,3 +172,87 @@ impl ScalarUDFImpl for SparkDatePart {
Ok(ExprSimplifyResult::Simplified(date_part_expr))
}
}
+
+/// `date_part('SECOND', ts)` for Spark: seconds including the microsecond
+/// fraction as `DECIMAL(8, 6)`.
+#[derive(Debug, PartialEq, Eq, Hash)]
+struct SparkDatePartSeconds {
+ signature: Signature,
+}
+
+impl SparkDatePartSeconds {
+ fn new() -> Self {
+ Self {
+ signature: Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Timestamp,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Native(logical_date()),
+ )]),
+ ],
+ Volatility::Immutable,
+ ),
+ }
+ }
+}
+
+impl ScalarUDFImpl for SparkDatePartSeconds {
+ fn name(&self) -> &str {
+ // Keep the display name as date_part so EXPLAIN/DESCRIBE stay
familiar.
+ "date_part"
Review Comment:
I found one regression in this PR: the new seconds UDF is also named
`date_part` but it takes only the timestamp argument. As a result the default
column name is now `date_part(t.ts)` instead of
`date_part(Utf8("SECOND"),t.ts)`. If we have 2 seconds aliases in the same
SELECT, they now collide. Example:
> SELECT date_part('SECOND', ts), date_part('S', ts)
> FROM (VALUES (timestamp '2020-01-01 00:00:01.5')) AS t(ts);
This runs on `main` but fails with the following error in this branch -
`Projections require unique expression names but the expression
"date_part(t.ts)" at position 0 and "date_part(t.ts)" at position 1 have the
same name.`
Could the rewritten call keep the part literal as its first argument, and
ignored at runtime, so the expression name stays the same as before? An slt
case with this query would guard against it further regressing.
##########
datafusion/spark/src/planner.rs:
##########
@@ -27,4 +43,44 @@ impl ExprPlanner for SparkFunctionPlanner {
ScalarFunction::new_udf(crate::function::string::substring(),
args),
)))
}
+
+ fn plan_scalar(
+ &self,
+ expr: RawScalarExpr,
+ _schema: &datafusion_common::DFSchema,
+ ) -> datafusion_common::Result<PlannerResult<RawScalarExpr>> {
+ // date_part('SECOND', ts) / datepart(...) → DECIMAL(8, 6) seconds UDF.
+ // Done here so the return type is fixed before analysis; a
user-written
+ // cast on the part literal is invisible to return_field_from_args.
+ if expr.func.name() == "date_part"
+ && let Some(part) = expr.args.first().and_then(expr_string_literal)
+ && is_seconds_part(&part)
+ {
+ let date_expr = expr.args.into_iter().nth(1).ok_or_else(|| {
+ datafusion_common::DataFusionError::Internal(
Review Comment:
Same here
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
