You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "shrirangmhalgi (via GitHub)" <gi...@apache.org> on 2026/06/20 17:27:40 UTC
[PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
shrirangmhalgi opened a new pull request, #56634:
URL: https://github.com/apache/spark/pull/56634
### What changes were proposed in this pull request?
Add `TimeType` inference to `CSVInferSchema` and `JsonInferSchema`, following the existing `DateType/TimestampType` inference pattern. A `tryParseTime` step is inserted in the CSV type ladder between `DoubleType` and `DateType`, and an equivalent check is added to the JSON string inference path. Both are gated behind `spark.sql.timeType.enabled` (default false).
### Why are the changes needed?
`CSVInferSchema` and `JsonInferSchema` infer `DateType` / `TimestampNTZType` / `TimestampType` but never `TimeType`. With an explicit schema, TIME read/write already works; only auto-inference is missing. Time-only columns (e.g., `12:13:14`) currently infer as `StringType` even when the TIME type feature is enabled.
### Does this PR introduce _any_ user-facing change?
Yes. When `spark.sql.timeType.enabled` is set to `true`, `CSV` and `JSON` schema inference will recognize time-only string values (e.g., `12:13:14`, `23:59:59.123456`) as `TimeType` instead of `StringType`. When disabled (default), behavior is unchanged.
### How was this patch tested?
New tests in CSVInferSchemaSuite and JsonInferSchemaSuite:
- Time strings infer as `TimeType` when `spark.sql.timeType.enabled = true`
- Time strings do NOT infer as `TimeType` when disabled (existing behavior preserved)
- `Date` / `timestamp` strings are not misclassified as time
- All existing + new tests pass for both CSV and JSON
### Was this patch authored or co-authored using generative AI tooling?
Co-Authored using Claude Opus 4.6
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4759317795
@MaxGekk Could you please review this PR which adds `TimeType` inference to `CSV` and `JSON` schema inference, gated behind `spark.sql.timeType.enabled`? It follows the existing `DateType` / `TimestampType` pattern in both `CSVInferSchema` and `JsonInferSchema`.
Happy to pick up other TIME type sub-tasks too. Let me know if any are unblocked. Thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56634:
URL: https://github.com/apache/spark/pull/56634#discussion_r3447603358
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/json/JsonInferSchemaSuite.scala:
##########
@@ -120,4 +120,43 @@ class JsonInferSchemaSuite extends SparkFunSuite with SQLHelper {
"""{"a": "2884-06-24T02:45:51.138"}""",
StringType)
}
+
+ test("SPARK-57572: infer TimeType when timeType.enabled is true") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "12:13:14"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "23:59:59.123456"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ // Negative: date and timestamp strings should NOT infer as TimeType
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "2023-01-01"}""", TimestampType)
+ checkType(Map("inferTimestamp" -> "true"),
+ """{"a": "2024-06-24T02:45:51.138"}""", TimestampType)
+ }
+ }
+
+ test("SPARK-57572: TimeType not inferred when timeType.enabled is false") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "false") {
+ // With inferTimestamp, time-only strings fall through to timestamp (lenient parser)
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "12:13:14"}""", TimestampType)
+ // Without inferTimestamp, time-only strings become StringType
+ checkType(Map.empty[String, String], """{"a": "12:13:14"}""", StringType)
+ }
+ }
+
+ test("SPARK-57572: TimeType not inferred without inferTimestamp") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ // inferTimestamp defaults to false; time inference requires it
+ checkType(Map.empty[String, String], """{"a": "12:13:14"}""", StringType)
+ }
+ }
+
+ test("SPARK-57572: TimeType cross-row merge via compatibleType") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ // Single time value infers as TimeType
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "12:13:14"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ // Non-time value stays StringType even with timeType enabled
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "not-a-time"}""", StringType)
+ }
+ }
Review Comment:
The JSON "cross-row merge" test only checks a single value and a non-time value. Neither asserts that two time-valued rows merge to TimeType (the actual compatibleType path). It works, but the test as written doesn't lock it. Please consider adding a two-record case like [{"a":"12:13:14"},{"a":"23:59:59"}] -> TimeType and a [{"a":"12:13:14"},{"a":"2023-01-01"}] -> StringType to match the CSV coverage.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56634:
URL: https://github.com/apache/spark/pull/56634#discussion_r3447601466
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JsonInferSchema.scala:
##########
@@ -177,10 +179,10 @@ class JsonInferSchema(private val options: JSONOptions) extends Serializable wit
if (options.prefersDecimal && decimalTry.isDefined) {
decimalTry.get
} else if (options.inferTimestamp) {
- // For text-based format, it's ambiguous to infer a timestamp string without timezone, as
- // it can be both TIMESTAMP LTZ and NTZ. To avoid behavior changes with the new support
- // of NTZ, here we only try to infer NTZ if the config is set to use NTZ by default.
Review Comment:
Should we move this comment instead of deleting it?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4771295589
Fixed the CI failures in the latest commit:
- Updated `csv-functions.sql.out` golden file (`schema_of_csv('14:30:45'`) now correctly infers `TIME(6)` instead of `TIMESTAMP`)
- Excluded the e2e inference test from `CSVLegacyTimeParserSuite` (legacy parser doesn't support TIME - same exclusion pattern as existing entries)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4767055436
Waiting for CI.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56634:
URL: https://github.com/apache/spark/pull/56634#discussion_r3447449563
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/json/JsonInferSchemaSuite.scala:
##########
@@ -120,4 +120,19 @@ class JsonInferSchemaSuite extends SparkFunSuite with SQLHelper {
"""{"a": "2884-06-24T02:45:51.138"}""",
StringType)
}
+
+ test("SPARK-57572: infer TimeType when timeType.enabled is true") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ checkType(Map.empty[String, String], """{"a": "12:13:14"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ checkType(Map.empty[String, String], """{"a": "23:59:59.123456"}""",
Review Comment:
The enabled-case test only checks positive values. Consider adding a negative case — e.g. a timestamp/date string like `2884-06-24T02:45:51.138` should still infer as `StringType`/timestamp, not `TimeType` — mirroring the CSV enabled test's `2023-01-01` / `not-a-time` checks.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/csv/CSVInferSchemaSuite.scala:
##########
@@ -273,4 +273,38 @@ class CSVInferSchemaSuite extends SparkFunSuite with SQLHelper {
val inferSchema = new CSVInferSchema(options)
assert(inferSchema.inferField(NullType, "2884-06-24T02:45:51.138") == StringType)
}
+
+ test("SPARK-57572: infer TimeType when timeType.enabled is true") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ val options = new CSVOptions(Map.empty[String, String],
+ columnPruning = false, defaultTimeZoneId = "UTC")
+ val inferSchema = new CSVInferSchema(options)
+
+ // Basic time inference
+ assert(inferSchema.inferField(NullType, "12:13:14") == TimeType(TimeType.MICROS_PRECISION))
+ assert(inferSchema.inferField(NullType, "23:59:59") == TimeType(TimeType.MICROS_PRECISION))
+ assert(inferSchema.inferField(NullType, "00:00:00") == TimeType(TimeType.MICROS_PRECISION))
+
+ // Time with fractional seconds
+ assert(inferSchema.inferField(NullType, "12:13:14.123") ==
+ TimeType(TimeType.MICROS_PRECISION))
+
+ // Not a time -- should fall through to other types
+ assert(inferSchema.inferField(NullType, "not-a-time") == StringType)
+
+ // Date should NOT be inferred as time
+ assert(inferSchema.inferField(NullType, "2023-01-01") == DateType)
+ }
+ }
+
+ test("SPARK-57572: TimeType not inferred when timeType.enabled is false") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "false") {
+ val options = new CSVOptions(Map.empty[String, String],
+ columnPruning = false, defaultTimeZoneId = "UTC")
+ val inferSchema = new CSVInferSchema(options)
+
+ // Time string should fall through to StringType when disabled
+ assert(inferSchema.inferField(NullType, "12:13:14") != TimeType(TimeType.MICROS_PRECISION))
Review Comment:
`!=` passes for any non-Time type, so it doesn't pin down the behavior — and the comment is inaccurate: with the flag off, `12:13:14` does not fall through to `StringType`. The lenient default timestamp parser accepts time-only strings (`justTime`, filled with `LocalDate.now`), so it infers as `TimestampType`. Assert the real type and fix the comment:
```suggestion
// When disabled, a time-only string is not inferred as TimeType; the lenient timestamp
// parser accepts it (using the current date), so it infers as TimestampType.
assert(inferSchema.inferField(NullType, "12:13:14") == TimestampType)
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JsonInferSchema.scala:
##########
@@ -176,6 +178,9 @@ class JsonInferSchema(private val options: JSONOptions) extends Serializable wit
}
if (options.prefersDecimal && decimalTry.isDefined) {
decimalTry.get
+ } else if (isTimeTypeEnabled &&
Review Comment:
JSON time inference runs whenever `timeType.enabled` is on, but JSON *timestamp* inference is gated by `inferTimestamp` (default `false`). With the default, this means a string column infers as `TimeType` while never inferring `TimestampType`, and every string field pays a `timeFormatter.parse` attempt — the per-string temporal-parse cost that `inferTimestamp=false` exists to avoid (SPARK-32130). CSV avoids both by tying time to its existing `preferDate` opt-in. Consider moving this check inside the `if (options.inferTimestamp)` block so time requires the same temporal opt-in as timestamp, or note in the PR why JSON time should be independent.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56634:
URL: https://github.com/apache/spark/pull/56634#discussion_r3447607550
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVInferSchema.scala:
##########
@@ -194,12 +201,20 @@ class CSVInferSchema(val options: CSVOptions) extends Serializable {
if ((allCatch opt field.toDouble).isDefined || isInfOrNan(field)) {
DoubleType
} else if (options.preferDate) {
- tryParseDate(field)
+ tryParseTime(field)
} else {
tryParseTimestampNTZ(field)
}
}
+ private def tryParseTime(field: String): DataType = {
+ if (isTimeTypeEnabled && (allCatch opt timeFormatter.parse(field)).isDefined) {
+ TimeType(TimeType.MICROS_PRECISION)
Review Comment:
e.g. let's use `TimeType.DEFAULT_PRECISION` here to avoid hard-coding yet another "MICROS" occurrence
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4760917013
@HyukjinKwon Could you look at the PR, please.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #56634: [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference
URL: https://github.com/apache/spark/pull/56634
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4760023473
Thanks @MaxGekk for the thorough review. Addressed all the points in latest commit.
- Moved JSON time inference inside `if (options.inferTimestamp)` block so it requires the same temporal opt-in as timestamp (consistent with SPARK-32130 rationale).
- Fixed the weak CSV assertion to pin the actual type (`TimestampType`)
- Added negative cases in JSON (date/timestamp strings shouldn't become TimeType)
- **Cross-row merge test:** Added compatibleType coverage in `CSVInferSchemaSuite` (TimeType + non-time → StringType, TimeType + Date → StringType).
Ill file a separate JIRA for `XmlInferSchema` once this is in.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4761043195
Thanks @uros-b for the review! Addressed all the points in latest commit:
- Switched to `TimeType.DEFAULT_PRECISION` to avoid hardcoding micros
- Restored the `NTZ` comment (moved it to the new nesting level)
- Replaced the JSON merge test with proper `JsonInferSchema.compatibleType` assertions (Time+Time, Time+String, Time+Date, Time+Null)
- Added end-to-end `spark.read.csv` and `spark.read.json` inference round-trip tests in `CSVSuite` / `JsonSuite`
Good callout on the precision. Agreed we should keep it flexible for when nano support lands.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "shrirangmhalgi (via GitHub)" <gi...@apache.org>.
shrirangmhalgi commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4773724004
Thankyou @MaxGekk / @uros-b / @HyukjinKwon for the review.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56634:
URL: https://github.com/apache/spark/pull/56634#discussion_r3447604019
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/json/JsonInferSchemaSuite.scala:
##########
@@ -120,4 +120,43 @@ class JsonInferSchemaSuite extends SparkFunSuite with SQLHelper {
"""{"a": "2884-06-24T02:45:51.138"}""",
StringType)
}
+
+ test("SPARK-57572: infer TimeType when timeType.enabled is true") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "12:13:14"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "23:59:59.123456"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ // Negative: date and timestamp strings should NOT infer as TimeType
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "2023-01-01"}""", TimestampType)
+ checkType(Map("inferTimestamp" -> "true"),
+ """{"a": "2024-06-24T02:45:51.138"}""", TimestampType)
+ }
+ }
+
+ test("SPARK-57572: TimeType not inferred when timeType.enabled is false") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "false") {
+ // With inferTimestamp, time-only strings fall through to timestamp (lenient parser)
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "12:13:14"}""", TimestampType)
+ // Without inferTimestamp, time-only strings become StringType
+ checkType(Map.empty[String, String], """{"a": "12:13:14"}""", StringType)
+ }
+ }
+
+ test("SPARK-57572: TimeType not inferred without inferTimestamp") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ // inferTimestamp defaults to false; time inference requires it
+ checkType(Map.empty[String, String], """{"a": "12:13:14"}""", StringType)
+ }
+ }
+
+ test("SPARK-57572: TimeType cross-row merge via compatibleType") {
+ withSQLConf(SQLConf.TIME_TYPE_ENABLED.key -> "true") {
+ // Single time value infers as TimeType
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "12:13:14"}""",
+ TimeType(TimeType.MICROS_PRECISION))
+ // Non-time value stays StringType even with timeType enabled
+ checkType(Map("inferTimestamp" -> "true"), """{"a": "not-a-time"}""", StringType)
+ }
+ }
}
Review Comment:
On another note, we don't have any end-to-end read tests. All tests operate directly on CSVInferSchema/JsonInferSchema. There's no round-trip test through spark.read.csv(...) / spark.read.json(...) verifying that the inferred schema is TimeType and the values parse correctly. Since this is the first time inference can produce TimeType, a single round-trip test in CSVSuite/JsonSuite would guard the full inference+parse path.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56634:
URL: https://github.com/apache/spark/pull/56634#discussion_r3447606817
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVInferSchema.scala:
##########
@@ -194,12 +201,20 @@ class CSVInferSchema(val options: CSVOptions) extends Serializable {
if ((allCatch opt field.toDouble).isDefined || isInfOrNan(field)) {
DoubleType
} else if (options.preferDate) {
- tryParseDate(field)
+ tryParseTime(field)
} else {
tryParseTimestampNTZ(field)
}
}
+ private def tryParseTime(field: String): DataType = {
+ if (isTimeTypeEnabled && (allCatch opt timeFormatter.parse(field)).isDefined) {
+ TimeType(TimeType.MICROS_PRECISION)
Review Comment:
Let's just be careful about any possible silent precision loss for nano time (@MaxGekk is currently working on this).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57572][SQL] Infer TimeType during CSV and JSON schema inference [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #56634:
URL: https://github.com/apache/spark/pull/56634#issuecomment-4773640626
Merged to master and branch-4.x.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org