cloud-fan commented on code in PR #56425:
URL: https://github.com/apache/spark/pull/56425#discussion_r3398956098
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala:
##########
@@ -75,7 +75,16 @@ private case class OracleDialect() extends JdbcDialect with
SQLConfHelper with N
override def visitSQLFunction(funcName: String, inputs: Array[String]):
String = {
funcName match {
case "TRUNC" =>
- s"TRUNC(${inputs(0)}, 'IW')"
+ // Map Spark's trunc format strings to Oracle equivalents.
+ // inputs(1) arrives as e.g. "'MONTH'" (with quotes from
LiteralValue.toString).
+ val oracleFormat = inputs(1) match {
Review Comment:
Spark's trunc format is case-insensitive (`parseTruncLevel` uppercases
before matching, DateTimeUtils.scala:587), but this match is exact-case, so a
valid format like `trunc(d, 'week')` falls through to the passthrough arm and
compiles to `TRUNC(d, 'week')` — Oracle's TRUNC format model has no `WEEK`
element, so the pushed query fails at runtime. The existing integration test
pushes exactly this shape (`trunc(date1, 'week')`,
OracleIntegrationSuite.scala:337); it worked before this PR because the
hardcoded `'IW'` happened to be correct for week, and the docker integration
suites don't run in PR CI, so this won't show up as a CI failure here. Match on
`inputs(1).toUpperCase(Locale.ROOT)` instead.
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala:
##########
@@ -75,7 +75,16 @@ private case class OracleDialect() extends JdbcDialect with
SQLConfHelper with N
override def visitSQLFunction(funcName: String, inputs: Array[String]):
String = {
funcName match {
case "TRUNC" =>
- s"TRUNC(${inputs(0)}, 'IW')"
+ // Map Spark's trunc format strings to Oracle equivalents.
+ // inputs(1) arrives as e.g. "'MONTH'" (with quotes from
LiteralValue.toString).
Review Comment:
The quotes on this path come from `JDBCSQLBuilder.visitLiteral` ->
`compileValue` (JdbcDialects.scala:384); `LiteralValue.toString` isn't invoked
here.
```suggestion
// inputs(1) arrives quoted, e.g. "'MONTH'" (see
JDBCSQLBuilder.visitLiteral).
```
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala:
##########
@@ -75,7 +75,16 @@ private case class OracleDialect() extends JdbcDialect with
SQLConfHelper with N
override def visitSQLFunction(funcName: String, inputs: Array[String]):
String = {
funcName match {
case "TRUNC" =>
- s"TRUNC(${inputs(0)}, 'IW')"
+ // Map Spark's trunc format strings to Oracle equivalents.
+ // inputs(1) arrives as e.g. "'MONTH'" (with quotes from
LiteralValue.toString).
+ val oracleFormat = inputs(1) match {
+ case "'WEEK'" => "'IW'"
+ case "'MONTH'" | "'MM'" | "'MON'" => "'MM'"
+ case "'QUARTER'" => "'Q'"
+ case "'YEAR'" | "'YYYY'" | "'YY'" => "'YYYY'"
+ case other => other
Review Comment:
Passing unmapped formats through diverges from Spark semantics: `trunc`
returns NULL for formats it doesn't recognize or that are too fine for dates
(`TruncDate` returns null below `MIN_LEVEL_OF_DATE_TRUNC`), but Oracle either
errors or silently returns a real value — e.g. `'DD'` truncates to day, and
`'DAY'` truncates to the start of the week. A non-literal format argument
(`trunc(d, fmt_col)`) is also pushed down and lands here as a column reference.
Throw in this arm instead, like `visitAggregateFunction` below (or
`visitExtract`'s `visitUnexpectedExpr`): `compileExpression` catches it and
returns None, so the expression simply isn't pushed and Spark evaluates `trunc`
itself with the right semantics.
--
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]
