spark-test commented on code in PR #60211:
URL: https://github.com/apache/spark/pull/60211#discussion_r4504452172
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/orc/OrcFileFormat.scala:
##########
@@ -202,6 +202,8 @@ case class OrcFileFormat() extends FileFormat
case _: AnsiIntervalType => false
case _: TimeType => false
case _: GeometryType | _: GeographyType => false
+ // Hive has no TIMESTAMP_NTZ type, so the Hive ORC serde cannot store it.
Review Comment:
**Nit (P3):** This reason doesn't hold for this format. The same serializer
already stores `TIMESTAMP_NTZ(7..9)` as a Hive `timestamp` with wall-clock
semantics: `toHiveCompatibleDataType` maps the nanosecond timestamp types to
`TimestampType`, `HiveInspectors` converts them through `LocalDateTime`, and
the SPARK-57455 test in `HiveOrcSourceSuite` round-trips them across a
time-zone change. Microsecond `TIMESTAMP_NTZ` fails only because
`toHiveCompatibleDataType` has no mapping for `TimestampNTZType`, so
`timestamp_ntz` reaches Hive's type parser, which is also how the PR
description explains it. As written, the comment suggests that no
`TIMESTAMP_NTZ` variant can be stored here and may steer maintainers away from
the existing mapping. For example:
```suggestion
// OrcSerializer has no Hive mapping for microsecond TIMESTAMP_NTZ,
unlike TIMESTAMP_NTZ(7..9).
```
**Verification:**
- **Inspection:** The comment beside the Hive ORC TIMESTAMP_NTZ rejection
states that the serializer has no microsecond TIMESTAMP_NTZ type mapping,
consistent with the nanosecond TIMESTAMP_NTZ types being stored as a Hive
timestamp.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/orc/OrcFileFormat.scala:
##########
@@ -202,6 +202,8 @@ case class OrcFileFormat() extends FileFormat
case _: AnsiIntervalType => false
case _: TimeType => false
case _: GeometryType | _: GeographyType => false
+ // Hive has no TIMESTAMP_NTZ type, so the Hive ORC serde cannot store it.
+ case _: TimestampNTZType => false
Review Comment:
**Non-blocking (P2):** This arm is also the read-side check for the Hive ORC
format, not only the write-side one. The format doesn't override
`supportReadDataType`, so `DataSourceUtils.verifySchema` applies it in
`DataSource.resolveRelation`, both to path reads with a user-specified schema
and to catalog ORC table reads, which resolve with `readOnly = false`. It
checks the full data schema, so a query fails even when it doesn't select the
`TIMESTAMP_NTZ` column.
That removes reads that worked on the base branch. The native
implementation, which is the default, stores `TIMESTAMP_NTZ` as an ORC LONG
column holding the micros (`OrcUtils.orcTypeDescription`, `OrcSerializer`), and
the Hive reader decoded it through the `LongObjectInspector` arm of
`HiveInspectors.unwrapperFor`, returning the stored values. With this change,
under `spark.sql.orc.impl=hive` or the legacy `org.apache.spark.sql.hive.orc`
names:
```sql
-- written with the default spark.sql.orc.impl=native
CREATE TABLE t (id INT, ts TIMESTAMP_NTZ) USING ORC;
INSERT INTO t VALUES (1, TIMESTAMP_NTZ'2020-01-01 00:00:00');
-- later, in a session with spark.sql.orc.impl=hive
SELECT id FROM t; -- now fails with UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE
```
The same happens for `spark.read.schema(...).format("orc").load(path)` over
such files, and for `TIMESTAMP_NTZ` nested in structs, arrays or maps. The only
recovery is to switch implementations. The read case in the new test is a
different input: that file stores a Hive `timestamp` column, which the base
branch decoded as instant micros reinterpreted as wall-clock values, so failing
explicitly there is an improvement worth keeping, in line with native ORC's
`cannotConvertOrcTimestampToTimestampNTZError`.
**Recommended change:** Move the TIMESTAMP_NTZ rejection out of the Hive ORC
supportDataType, which also gates reads and CREATE TABLE, and into the write
path. Drop the new TimestampNTZType arm and its comment, and make the Hive
OrcFileFormat.prepareWrite reject a data schema that contains TIMESTAMP_NTZ at
any nesting level, recursing through struct, array and map types and a
user-defined type's SQL type as the current check does, with the same
UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE error (format ORC, the offending top-level
column and its full type), commented with the actual reason: the Hive ORC
serializer has no TIMESTAMP_NTZ type mapping. In the reader, send required
fields that contain TIMESTAMP_NTZ through the data-type-aware unwrapper and
give it a case for a Hive timestamp read as TIMESTAMP_NTZ, so that such a field
fails with the ORC timestamp-to-TIMESTAMP_NTZ conversion error that native ORC
already raises, while LONG-backed values written by native ORC keep their
direct d
ecode. In the Hive ORC query tests, keep the write assertions, expect CREATE
TABLE ... USING ORC to succeed and the following INSERT to fail with the same
error, expect the conversion failure for the Hive timestamp read, and add reads
of native-written TIMESTAMP_NTZ data.
**Why this works:** FileFormatWriter.write calls prepareWrite on the driver
right after its schema check and before job setup, overwrite deletion or any
task, so a TIMESTAMP_NTZ write still fails early with the same clear error.
Relation resolution no longer rejects TIMESTAMP_NTZ for this format, so catalog
tables and user-specified schemas resolve as on the base branch. The reader
then separates the two physical encodings. Native ORC stores TIMESTAMP_NTZ as
ORC LONG micros, which decode directly to the stored values. A Hive timestamp
column decodes through an instant and would come back shifted, so that case
fails explicitly instead of returning wrong values.
**Scope:** Relocate the Hive ORC TIMESTAMP_NTZ rejection from the shared
type check to the write path, add a reader check for Hive timestamp columns
requested as TIMESTAMP_NTZ, and update the Hive ORC query tests.
**Compatibility:** Native ORC TIMESTAMP_NTZ support, Hive ORC round trips of
TIMESTAMP_NTZ(7..9), and the clear driver-side error for TIMESTAMP_NTZ writes
under the Hive implementation stay as they are.
**Risks:** CREATE TABLE ... USING ORC with a TIMESTAMP_NTZ column under the
Hive implementation succeeds again, as on the base branch, so the clear error
moves from table creation to the first write. This gives up the early DDL
failure the PR describes; this format cannot keep that failure without also
keeping the read regression, because table creation and catalog-table reads go
through the same relation check. Reading a Hive timestamp column with a
user-specified TIMESTAMP_NTZ schema fails during execution with the ORC
timestamp-to-TIMESTAMP_NTZ conversion error, rather than at analysis with
UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE. One premise cannot be settled from the
Spark source: that Hive's ORC reader exposes the ORC LONG column of a
native-written file through a long object inspector, which is what made these
values readable on the base branch and what the restored reads rely on. The
regression test that reads native-written TIMESTAMP_NTZ data under the Hive
implementation mu
st confirm it.
**Constraints:** The write-time error keeps the condition and parameters of
the current rejection (format ORC, the quoted top-level column name and its
full SQL type) and is raised on the driver before job setup, overwrite deletion
or any task. No read returns Hive timestamp values reinterpreted as
TIMESTAMP_NTZ; such reads fail explicitly.
**Exclusions:**
- Storing microsecond TIMESTAMP_NTZ through a Hive type mapping is left to
the separate question of whether this format should support the type; if the
author chooses support, the mapping replaces this fix.
- ALTER TABLE ... ADD COLUMNS still does not consult the type check, as for
every other type this format rejects, and a later write gets the clear error.
**Success:** With spark.sql.orc.impl=hive, writing a TIMESTAMP_NTZ column,
top-level or nested, fails on the driver with
UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE naming the column and its type, before any
task runs. With spark.sql.orc.impl=hive, TIMESTAMP_NTZ data written by the
native ORC implementation reads back with its stored values, through a catalog
table or a user-specified schema, including queries that do not select the
TIMESTAMP_NTZ column. With spark.sql.orc.impl=hive, reading a Hive timestamp
column with a TIMESTAMP_NTZ schema fails with an explicit conversion error
instead of returning shifted values. The native ORC implementation and the
TIMESTAMP_NTZ(7..9) types behave as on the base branch.
**Verification:**
- **Regression:** Write TIMESTAMP_NTZ data with the native implementation,
at top level and nested in a struct, an array and a map, then read it under the
Hive implementation through a catalog table and through a user-specified
schema, including a query that selects only a non-TIMESTAMP_NTZ column, and
assert that the stored values come back.
- **Behavior:** Under the Hive implementation, writing a TIMESTAMP_NTZ
column at top level and nested in a struct, array or map fails with
UNSUPPORTED_DATA_TYPE_FOR_DATASOURCE and the expected column and type
parameters.
- **Behavior:** Under the Hive implementation, reading a Hive-written
TIMESTAMP_LTZ column with a TIMESTAMP_NTZ schema fails with an explicit
conversion error rather than returning values.
- **Compatibility:** Under the Hive implementation, CREATE TABLE ... USING
ORC with a TIMESTAMP_NTZ column succeeds as on the base branch and the
following INSERT fails with the clear error, while native-implementation
TIMESTAMP_NTZ round trips and Hive-implementation TIMESTAMP_NTZ(7..9) round
trips stay unchanged.
--
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]
