Skip to content

[SPARK-39469][SQL] Infer date type for CSV schema inference #36871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jonathancui123 wants to merge 21 commits into from
Closed

[SPARK-39469][SQL] Infer date type for CSV schema inference #36871

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5048253
infer date type in csv
Jonathancui123 Jun 15, 2022
5058d92
fix
Jonathancui123 Jun 15, 2022
f16e5e1
parse dates from timestamp column
Jonathancui123 Jun 15, 2022
c2b5fdc
cleanup
Jonathancui123 Jun 15, 2022
9514c2c
test date and timestamp same format
Jonathancui123 Jun 15, 2022
be7aabd
e2e test WIP
Jonathancui123 Jun 16, 2022
db3b442
e2e test data
Jonathancui123 Jun 16, 2022
7d98686
fix e2e date test
Jonathancui123 Jun 17, 2022
966bdb6
added inferDate flag
Jonathancui123 Jun 21, 2022
638064b
Updated documentation and fixed tests
Jonathancui123 Jun 22, 2022
50a91a6
added new error for inferDate with legacy parser and fixed feedback
Jonathancui123 Jun 22, 2022
d71558d
Update CSVInferSchema.scala
Jonathancui123 Jun 23, 2022
601dfc8
Update CSVOptions.scala
Jonathancui123 Jun 23, 2022
5aa4ab6
change inferDate error type and improve docs
Jonathancui123 Jun 23, 2022
2282c59
fix error class
Jonathancui123 Jun 23, 2022
2484b77
fix sqlState code and typo
Jonathancui123 Jun 24, 2022
762e0d8
Update CSVInferSchema.scala
Jonathancui123 Jun 24, 2022
2c93af5
updated docs and test parsing dates in TimestampNTZ column
Jonathancui123 Jun 28, 2022
41fa8eb
Fix case where dateFormat is not specified
Jonathancui123 Jun 30, 2022
e1170d0
allow legacy parser with inferDate
Jonathancui123 Jul 12, 2022
1e8f938
Revert "allow legacy parser with inferDate"
Jonathancui123 Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
parse dates from timestamp column
  • Loading branch information
@Jonathancui123
Jonathancui123 committed Jul 12, 2022
commit f16e5e169b99fdeffdc0925a4cbed20b672628ce
6 changes: 3 additions & 3 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVInferSchema.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class CSVInferSchema(val options: CSVOptions) extends Serializable {
case _: DecimalType => tryParseDecimal(field)
case DoubleType => tryParseDouble(field)
// Temporary NOTE: DateTimeType is private to [sql] package
case DateType | TimestampNTZType | TimestampType => tryParseDate(field)
case DateType | TimestampNTZType | TimestampType => tryParseDateTime(field)
case BooleanType => tryParseBoolean(field)
case StringType => StringType
case other: DataType =>
Expand Down Expand Up @@ -177,11 +177,11 @@ class CSVInferSchema(val options: CSVOptions) extends Serializable {
if ((allCatch opt field.toDouble).isDefined || isInfOrNan(field)) {
DoubleType
} else {
tryParseDate(field)
tryParseDateTime(field)
}
}

private def tryParseDate(field: String): DataType = {
private def tryParseDateTime(field: String): DataType = {
if ((allCatch opt dateFormatter.parse(field)).isDefined) {
Copy link
Contributor

@brkyvz brkyvz Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very expensive way

DateType
} else {
Expand Down
29 changes: 18 additions & 11 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ class UnivocityParser(

private val decimalParser = ExprUtils.getDecimalParser(options.locale)

private def convertToDate(datum: String): Int = {
try {
dateFormatter.parse(datum)
} catch {
case NonFatal(e) =>
// If fails to parse, then tries the way used in 2.0 and 1.x for backwards
// compatibility.
val str = DateTimeUtils.cleanLegacyTimestampStr(UTF8String.fromString(datum))
DateTimeUtils.stringToDate(str).getOrElse(throw e)
}
}

/**
* Create a converter which converts the string value to a value according to a desired type.
* Currently, we do not support complex types (`ArrayType`, `MapType`, `StructType`).
Expand Down Expand Up @@ -206,28 +218,23 @@ class UnivocityParser(
// If fails to parse, then tries the way used in 2.0 and 1.x for backwards
// compatibility.
val str = DateTimeUtils.cleanLegacyTimestampStr(UTF8String.fromString(datum))
DateTimeUtils.stringToTimestamp(str, options.zoneId).getOrElse(throw e)
DateTimeUtils.stringToTimestamp(str, options.zoneId).getOrElse(convertToDate(datum))
}
}

case _: TimestampNTZType => (d: String) =>
nullSafeDatum(d, name, nullable, options) { datum =>
timestampNTZFormatter.parseWithoutTimeZone(datum, false)
}

case _: DateType => (d: String) =>
nullSafeDatum(d, name, nullable, options) { datum =>
try {
dateFormatter.parse(datum)
timestampNTZFormatter.parseWithoutTimeZone(datum, false)
} catch {
case NonFatal(e) =>
// If fails to parse, then tries the way used in 2.0 and 1.x for backwards
// compatibility.
val str = DateTimeUtils.cleanLegacyTimestampStr(UTF8String.fromString(datum))
DateTimeUtils.stringToDate(str).getOrElse(throw e)
convertToDate(datum)
}
}

case _: DateType => (d: String) =>
nullSafeDatum(d, name, nullable, options)(convertToDate)

case _: StringType => (d: String) =>
nullSafeDatum(d, name, nullable, options)(UTF8String.fromString)

Expand Down
15 changes: 15 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/csv/UnivocityParserSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,4 +358,19 @@ class UnivocityParserSuite extends SparkFunSuite with SQLHelper {
Map("timestampFormat" -> "invalid", "dateFormat" -> "invalid"), false, "UTC")
check(new UnivocityParser(StructType(Seq.empty), optionsWithPattern))
}

test("dates should be parsed correctly in a timestamp column") {
def checkDate(dataType: DataType): Unit = {
val timestampsOptions =
new CSVOptions(Map("timestampFormat" -> "dd/MM/yyyy HH:mm",
"timestampNTZFormat" -> "dd-MM-yyyy HH:mm", "dateFormat" -> "dd_MM_yyyy"),
Copy link
Member

@HyukjinKwon HyukjinKwon Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One test we might need would be:

timestampFormat" -> "dd/MM/yyyy HH:mm and dateFormat -> dd/MM/yyyy to make sure timestamps are not parsed as date types without conflicting.

Copy link
Contributor

@bersprockets bersprockets Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make sure timestamps are not parsed as date types without conflicting.

That's actually what happens:

Before this PR:

scala> val csvInput = Seq("0,2012-01-01 12:00:00", "1,2021-07-01 15:00:00").toDS()
csvInput: org.apache.spark.sql.Dataset[String] = [value: string]

scala> val df = spark.read.option("inferSchema", "true").csv(csvInput)
df: org.apache.spark.sql.DataFrame = [_c0: int, _c1: timestamp]

scala> df.printSchema
root
 |-- _c0: integer (nullable = true)
 |-- _c1: timestamp (nullable = true)

scala>

After this PR:

scala> val csvInput = Seq("0,2012-01-01 12:00:00", "1,2021-07-01 15:00:00").toDS()
csvInput: org.apache.spark.sql.Dataset[String] = [value: string]

scala> val df = spark.read.option("inferSchema", "true").csv(csvInput)
df: org.apache.spark.sql.DataFrame = [_c0: int, _c1: date]

scala> df.printSchema
root
 |-- _c0: integer (nullable = true)
 |-- _c1: date (nullable = true)

scala>

It looks like some tests fail too, like CSVInferSchemaSuite, and CSVv1Suite, possibly others (I ran these two suites on my laptop. For some reason, the github actions didn't run tests for this PR. Maybe @Jonathancui123 needs to turn them on in his fork?).

We should probably 1. add either SQL configuration or an option e.g., infersDate

I think you would need something like that: when set, the date formatter could use the slower, more strict method of parsing (so "2012-01-01 12:00:00" wouldn't parse as a date).

Edit: To do a strict parsing, one might need to use ParsePosition and check that the whole date/time value string was consumed. Even after setting lenient=false SimpleDateFormat.parse didn't complain about extra characters that weren't consumed.

Copy link
Contributor Author

@Jonathancui123 Jonathancui123 Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed inference mistakes in the following code snippet and comment

false, "UTC")
val dateString = "08_09_2001"
val date = days(2001, 9, 8)
val parser = new UnivocityParser(new StructType(), timestampsOptions)
assert(parser.makeConverter("d", dataType).apply(dateString) == date)
}
checkDate(TimestampType)
checkDate(TimestampNTZType)
}
}