[SPARK-35728][SQL][TESTS] Check multiply/divide of day-time intervals of any fields by numeric

[Peng-Lei]

What changes were proposed in this pull request?

1. The testcase is just cover the DayTimeIntervalType() */ numeric
2. Add testcase for following intervals */ numeric:
   INTERVAL DAY
   INTERVAL DAY TO HOUR
   INTERVAL DAY TO MINUTE
   INTERVAL HOUR
   INTERVAL HOUR TO MINUTE
   INTERVAL HOUR TO SECOND
   INTERVAL MINUTE
   INTERVAL MINUTE TO SECOND
   INTERVAL SECOND

Why are the changes needed?

Add testcase coverage.

Does this PR introduce any user-facing change?

no

How was this patch tested?

existed testcase

[Peng-Lei]

@MaxGekk
INTERVAL DAY * or / numeric should return INTERVAL DAY
INTERVAL DAY TO HOUR * or / numeric should return INTERVAL DAY TO HOUR
right ? but now return default DayTimeIntervalType()

[sarutak]

Let's remove the two occurrences of the following comment.

// TODO(SPARK-35728): Check multiply/divide of day-time intervals of any fields by numeric

[AmplabJenkins]

Can one of the admins verify this patch?

[MaxGekk]

@MaxGekk
INTERVAL DAY * or / numeric should return INTERVAL DAY
INTERVAL DAY TO HOUR * or / numeric should return INTERVAL DAY TO HOUR
right ?

@Peng-Lei Don't think so. Especially about /

but now return default DayTimeIntervalType()

This type doesn't loose info at least. I would consider DayTimeIntervalType(SECOND) but the default one DayTimeIntervalType() == DayTimeIntervalType(DAY, SECOND) is good enough.

[Peng-Lei]

@MaxGekk
INTERVAL DAY * or / numeric should return INTERVAL DAY
INTERVAL DAY TO HOUR * or / numeric should return INTERVAL DAY TO HOUR
right ?

@Peng-Lei Don't think so. Especially about /

but now return default DayTimeIntervalType()

This type doesn't loose info at least. I would consider DayTimeIntervalType(SECOND) but the default one DayTimeIntervalType() == DayTimeIntervalType(DAY, SECOND) is good enough.

@MaxGekk Thank you for your explanation. Special cases
eg: I think if INTERVAL DAY * numeric should return INTERVAL DAY doesn't loose info, but INTERVAL DAY / numeric maybe loose info. If INTERVAL DAY * numeric return INTERVAL DAY, when DateType +/- INTERVAL DAY is better?
expr: DateType +/- INTERVAL DAY * numeric

[MaxGekk]

The test failure seems to be related:

[info] - SPARK-34850: multiply day-time interval by numeric *** FAILED *** (4 milliseconds)
[info]   java.lang.IllegalArgumentException: requirement failed: Literal must have a corresponding value to interval day, but class Duration found.
[info]   at scala.Predef$.require(Predef.scala:281)
[info]   at org.apache.spark.sql.catalyst.expressions.Literal$.validateLiteralValue(literals.scala:235)

[Peng-Lei]

The test failure seems to be related:

[info] - SPARK-34850: multiply day-time interval by numeric *** FAILED *** (4 milliseconds)
[info]   java.lang.IllegalArgumentException: requirement failed: Literal must have a corresponding value to interval day, but class Duration found.
[info]   at scala.Predef$.require(Predef.scala:281)
[info]   at org.apache.spark.sql.catalyst.expressions.Literal$.validateLiteralValue(literals.scala:235)

Thank you. I will research it

[Peng-Lei]

The test failure seems to be related:

[info] - SPARK-34850: multiply day-time interval by numeric *** FAILED *** (4 milliseconds)
[info]   java.lang.IllegalArgumentException: requirement failed: Literal must have a corresponding value to interval day, but class Duration found.
[info]   at scala.Predef$.require(Predef.scala:281)
[info]   at org.apache.spark.sql.catalyst.expressions.Literal$.validateLiteralValue(literals.scala:235)

@MaxGekk I use Literal.create(duration, dt) instead of Literal(duration, dt),
because when we use Literal.create(duration, dt), it will CatalystTypeConverters.convertToCatalyst(duration) first,
then Literal(CatalystTypeConverters.convertToCatalyst(duration), dt),
If we Literal(duration, dt) directly, we could not convert duration as Long when doValidate

[MaxGekk]

I use Literal.create(duration, dt) instead of Literal(duration, dt)

@Peng-Lei Do you mean "instead of Literal(duration)"?

[MaxGekk]

Highly likely, the changes are not related to the build failure:

[error] /home/runner/work/spark/spark/sql/catalyst/src/test/scala-2.12/org/apache/spark/sql/catalyst/analysis/ExtractGeneratorSuite.scala:29:64: exception during macro expansion: 
[error] java.util.MissingResourceException: Can't find bundle for base name org.scalactic.ScalacticBundle, locale en
[error] 	at java.util.ResourceBundle.throwMissingResourceException(ResourceBundle.java:1581)

[MaxGekk]

+1, LGTM. GA passed (I built and ran the test locally). Merging to master.
Thank you, @Peng-Lei and @sarutak for your review.