Skip to content

Commit

Permalink
[SPARK-28495][SQL][FOLLOW-UP] Disallow conversions between timestamp …
Browse files Browse the repository at this point in the history
…and long in ASNI mode

### What changes were proposed in this pull request?

Disallow conversions between `timestamp` type and `long` type in table insertion with ANSI store assignment policy.

### Why are the changes needed?

In the PR #25581, timestamp type is allowed to be converted to long type, since timestamp type is represented by long type internally, and both legacy mode and strict mode allows the conversion.

After reconsideration, I think we should disallow it. As per ANSI SQL section "4.4.2 Characteristics of numbers":
> A number is assignable only to sites of numeric type.

In PostgreSQL, the conversion between timestamp and long is also disallowed.

### Does this PR introduce any user-facing change?

Conversion between timestamp and long is disallowed in table insertion with ANSI store assignment policy.

### How was this patch tested?

Unit test

Closes #25615 from gengliangwang/disallowTimeStampToLong.

Authored-by: Gengliang Wang <gengliang.wang@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
  • Loading branch information
gengliangwang authored and cloud-fan committed Aug 29, 2019
1 parent 137b20b commit 2465558
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
Expand Up @@ -158,17 +158,18 @@ object Cast {
case _ => false
}

/**
* Returns true iff we can cast the `from` type to `to` type as per the ANSI SQL.
* In practice, the behavior is mostly the same as PostgreSQL. It disallows certain unreasonable
* type conversions such as converting `string` to `int` or `double` to `boolean`.
*/
def canANSIStoreAssign(from: DataType, to: DataType): Boolean = (from, to) match {
case _ if from == to => true
case (_: NumericType, _: NumericType) => true
case (_: AtomicType, StringType) => true
case (_: CalendarIntervalType, StringType) => true
case (DateType, TimestampType) => true
case (TimestampType, DateType) => true
// Spark supports casting between long and timestamp, please see `longToTimestamp` and
// `timestampToLong` for details.
case (TimestampType, LongType) => true
case (LongType, TimestampType) => true

case (ArrayType(fromType, fn), ArrayType(toType, tn)) =>
resolvableNullability(fn, tn) && canANSIStoreAssign(fromType, toType)
Expand Down
Expand Up @@ -133,6 +133,18 @@ class ANSIDataTypeWriteCompatibilitySuite extends DataTypeWriteCompatibilityBase
assert(err.contains("Cannot safely cast"))
}
}

test("Conversions between timestamp and long are not allowed") {
assertSingleError(LongType, TimestampType, "longToTimestamp",
"Should not allow long to timestamp") { err =>
assert(err.contains("Cannot safely cast 'longToTimestamp': LongType to TimestampType"))
}

assertSingleError(TimestampType, LongType, "timestampToLong",
"Should not allow timestamp to long") { err =>
assert(err.contains("Cannot safely cast 'timestampToLong': TimestampType to LongType"))
}
}
}

abstract class DataTypeWriteCompatibilityBaseSuite extends SparkFunSuite {
Expand Down

0 comments on commit 2465558

Please sign in to comment.