Skip to content

Commit eb5af73

Browse files
res-lifeChong Gao
andauthored
Support DST timezones conversion for ORC [databricks] (#14544)
Fixes #13437. #### Depends on: * part 1: NVIDIA/cudf-spark-jni#4539 * part 2: NVIDIA/cudf-spark-jni#4635 * part 3: NVIDIA/cudf-spark-jni#4733 * ORC 2015 base-offset borrow fix: NVIDIA/cudf-spark-jni#4809 * part 4: NVIDIA/cudf-spark-jni#4812 * part 5: NVIDIA/cudf-spark-jni#4813 ### Description #### Context ORC OSS uses java.util.TimeZone to do rebase, it does not use java.time.ZoneId API. The java.util.TimeZone and java.time.ZoneId have inconsistent behavior. cuDF have `java.time` compatible impl, but does not have `java.util` compatible impl `java.util.TimeZone.getOffset` and `java.time.ZoneId.getOffset` are not always consistent. <details> <summary>For more details, click to expand</summary> ```java static void testDiffBehaviorBetweenTwoAPIs() { // diff in `Africa/Casablanca` timezone at 6424721300000: 0 vs 3600000 String tzId = "Africa/Casablanca"; long epochMillis = 6424721300000L; int offsetMillis_1 = java.util.TimeZone.getTimeZone(tzId).getOffset(epochMillis); int offsetMillis_2 = java.time.ZoneId.of(tzId, ZoneId.SHORT_IDS).getRules().getOffset(Instant.ofEpochMilli(epochMillis)).getTotalSeconds() * 1000; if (offsetMillis_1 != offsetMillis_2) { // print: get diff!! 0 vs 3600000 System.out.println("get diff!! " + offsetMillis_1 + " vs " + offsetMillis_2); } } ``` </details> #### Solution 1 [not feasible], use cuDF with ignoreTimezoneInStripeFooter=False. cuDF manages the ORC writer timezone decode. Problem: for far-future timestamps projected into the synthetic 400-year cycle, dates before the first synthetic DST transition were incorrectly using the first cycle entry, which is the DST offset. That causes exactly the +1 hour winter drift you saw for America/Los_Angeles with years like 8770. cuDF has `java.time` compatible impl instead of `java.util`. We can not get correct result. So this solution is not feasible. #### Solution 2, develope kernel, use cuDF with ignoreTimezoneInStripeFooter=True cuDF does not manage the ORC writer timezone decode. Decode as UTC in cuDF. All time rebasing logic is handled by customized JNI kernel which is compatible to `java.util`. #### changes - Remove the orc_timezone_info.data file, get the timezone info dynamically. Why: A pre-built timezone info file can not handle for all Java versions, in future the timezone info may change. - Implements `java.util.Timezone` logic - Add OrcTimezoneSuite to test reader/writer timezone combinations. #### Related cuDF issue * rapidsai/cudf#21993 ### perf number | Test | CPU avg (ms) | GPU avg (ms) | Speedup | |------|-------------:|-------------:|--------:| | **cross-tz** (LA→UTC) | 23,662 | 13,815 | 1.71x | | **same-tz-baseline** (LA→LA) | 79,699 | 13,766 | 5.79x | <details> <summary>How to run, refer to `OrcTimezonePerfSuite.scala`, click to expand</summary> ```bash argLine="-DenableOrcTimeZonePerf=true \ -DorcPerfWriterTZ=America/Los_Angeles \ -DorcPerfReaderTZ=UTC \ -DorcPerfRows=1073741824" \ mvn test -Dbuildver=350 \ -DwildcardSuites=com.nvidia.spark.rapids.timezone.OrcTimezonePerfSuite ``` </details> ### Checklists Documentation - [x] Updated for new or modified user-facing features or behaviors - [ ] No user-facing change Testing - [x] Added or modified tests to cover new code paths - [ ] Covered by existing tests (Please provide the names of the existing tests in the PR description.) - [ ] Not required Performance - [x] Tests ran and results are added in the PR description - [ ] Issue filed with a link in the PR description - [ ] Not required Signed-off-by: Chong Gao <chongg@nvidia.com> --------- Signed-off-by: Chong Gao <res_life@163.com> Signed-off-by: Chong Gao <chongg@nvidia.com> Co-authored-by: Chong Gao <res_life@163.com>
1 parent c406ee0 commit eb5af73

8 files changed

Lines changed: 908 additions & 201 deletions

File tree

integration_tests/src/main/python/orc_test.py

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,6 @@ def test_basic_read(std_input_path, name, read_func, v1_enabled_list, orc_impl,
158158
MapGen(StructGen([['child0', byte_gen], ['child1', long_gen]], nullable=False),
159159
StructGen([['child0', byte_gen], ['child1', long_gen]]))]
160160

161-
non_utc_allow_orc_scan=['ColumnarToRowExec', 'FileSourceScanExec', 'BatchScanExec'] if is_not_utc() else []
162-
163161
orc_gens_list = [orc_basic_gens,
164162
orc_array_gens_sample,
165163
orc_struct_gens_sample,
@@ -195,7 +193,6 @@ def test_orc_fallback(spark_tmp_path, read_func, disable_conf):
195193
@pytest.mark.parametrize('reader_confs', reader_opt_confs, ids=idfn)
196194
@pytest.mark.parametrize('v1_enabled_list', ['', 'orc'])
197195
@tz_sensitive_test
198-
@allow_non_gpu(*non_utc_allow_orc_scan)
199196
def test_read_round_trip(spark_tmp_path, orc_gens, read_func, reader_confs, v1_enabled_list):
200197
gen_list = [('_c' + str(i), gen) for i, gen in enumerate(orc_gens)]
201198
data_path = spark_tmp_path + '/ORC_DATA'
@@ -224,7 +221,6 @@ def test_read_round_trip(spark_tmp_path, orc_gens, read_func, reader_confs, v1_e
224221
@pytest.mark.parametrize('read_func', [read_orc_df, read_orc_sql])
225222
@pytest.mark.parametrize('v1_enabled_list', ["", "orc"])
226223
@pytest.mark.parametrize('reader_confs', reader_opt_confs, ids=idfn)
227-
@allow_non_gpu(*non_utc_allow_orc_scan)
228224
def test_pred_push_round_trip(spark_tmp_path, orc_gen, read_func, v1_enabled_list, reader_confs):
229225
data_path = spark_tmp_path + '/ORC_DATA'
230226
# Append two struct columns to verify nested predicate pushdown.
@@ -281,7 +277,6 @@ def test_compress_read_round_trip(spark_tmp_path, compress, v1_enabled_list, rea
281277

282278
@pytest.mark.parametrize('v1_enabled_list', ["", "orc"])
283279
@pytest.mark.parametrize('reader_confs', reader_opt_confs, ids=idfn)
284-
@allow_non_gpu(*non_utc_allow_orc_scan)
285280
def test_simple_partitioned_read(spark_tmp_path, v1_enabled_list, reader_confs):
286281
# Once https://github.com/NVIDIA/spark-rapids/issues/131 is fixed
287282
# we should go with a more standard set of generators
@@ -351,7 +346,6 @@ def test_partitioned_read_just_partitions(spark_tmp_path, v1_enabled_list, reade
351346

352347
@pytest.mark.parametrize('v1_enabled_list', ["", "orc"])
353348
@pytest.mark.parametrize('reader_confs', reader_opt_confs, ids=idfn)
354-
@allow_non_gpu(*non_utc_allow_orc_scan)
355349
def test_merge_schema_read(spark_tmp_path, v1_enabled_list, reader_confs):
356350
# Once https://github.com/NVIDIA/spark-rapids/issues/131 is fixed
357351
# we should go with a more standard set of generators
@@ -693,7 +687,6 @@ def test_read_struct_without_stream(spark_tmp_path):
693687
@pytest.mark.parametrize('reader_confs', reader_opt_confs, ids=idfn)
694688
@pytest.mark.parametrize('v1_enabled_list', ["", "orc"])
695689
@pytest.mark.parametrize('case_sensitive', ["false", "true"])
696-
@allow_non_gpu(*non_utc_allow_orc_scan)
697690
def test_read_with_more_columns(spark_tmp_path, orc_gen, reader_confs, v1_enabled_list, case_sensitive):
698691
struct_gen = StructGen([('nested_col', orc_gen)])
699692
# Map is not supported yet.
@@ -878,7 +871,6 @@ def test_orc_read_varchar_as_string(std_input_path):
878871
@pytest.mark.parametrize('gens', orc_gens_list, ids=idfn)
879872
@pytest.mark.parametrize('keep_order', [True, pytest.param(False, marks=pytest.mark.ignore_order(local=True))])
880873
@tz_sensitive_test
881-
@allow_non_gpu(*non_utc_allow_orc_scan)
882874
def test_read_round_trip_for_multithreaded_combining(spark_tmp_path, gens, keep_order):
883875
gen_list = [('_c' + str(i), gen) for i, gen in enumerate(gens)]
884876
data_path = spark_tmp_path + '/ORC_DATA'
@@ -893,7 +885,6 @@ def test_read_round_trip_for_multithreaded_combining(spark_tmp_path, gens, keep_
893885

894886

895887
@pytest.mark.parametrize('keep_order', [True, pytest.param(False, marks=pytest.mark.ignore_order(local=True))])
896-
@allow_non_gpu(*non_utc_allow_orc_scan)
897888
def test_simple_partitioned_read_for_multithreaded_combining(spark_tmp_path, keep_order):
898889
# Use every type except boolean, see https://github.com/NVIDIA/spark-rapids/issues/11762 and
899890
# https://github.com/rapidsai/cudf/issues/6763 .
@@ -1129,17 +1120,39 @@ def test_orc_not_support_timestamp_ltz(std_input_path):
11291120
conf={},
11301121
error_message=expected_error_message)
11311122

1123+
# Timestamp writes: in UTC the GPU writes on the GPU; in a non-UTC JVM the GPU write must fall
1124+
# back to CPU. cuDF's ORC writer always stamps writerTimezone="UTC" in the stripe footer and
1125+
# cannot record the JVM writer timezone (https://github.com/rapidsai/cudf/issues/23422), so a
1126+
# GPU-written non-UTC file would be read back shifted by the zone offset by a CPU ORC reader.
1127+
# The `tz_sensitive_test` mark runs this in both UTC and non-UTC JVM timezones.
1128+
non_utc_orc_write_allow = ['DataWritingCommandExec'] if is_not_utc() else []
1129+
1130+
@tz_sensitive_test
1131+
@ignore_order(local=True)
1132+
@allow_non_gpu(*non_utc_orc_write_allow)
1133+
def test_orc_gpu_write_cpu_read_timestamp_in_non_utc_timezone(spark_tmp_path):
1134+
data_path = spark_tmp_path + "/ORC_GPU_WRITE_TZ"
1135+
write_func = lambda spark, path: (
1136+
spark.range(3)
1137+
.selectExpr("CAST(1593604800 + id AS TIMESTAMP) AS ts")
1138+
.write.orc(path))
1139+
read_func = lambda spark, path: spark.read.orc(path)
1140+
if is_not_utc():
1141+
# Non-UTC: the timestamp write must fall back to CPU (DataWritingCommandExec).
1142+
assert_gpu_fallback_write(write_func, read_func, data_path, 'DataWritingCommandExec')
1143+
else:
1144+
assert_gpu_and_cpu_writes_are_equal_collect(write_func, read_func, data_path)
1145+
1146+
11321147
@pytest.mark.parametrize("reader_confs", reader_opt_confs, ids=idfn)
1133-
# Setting end timestamp as None almost always generate ts >= 2200 year.
1134-
# Setting end timestamp < 2200 to test running columnarly on GPU;
1135-
@pytest.mark.parametrize('end_timestamp', [None, datetime(2199, 1, 1, tzinfo=timezone.utc)], ids=idfn)
11361148
@pytest.mark.parametrize('v1_enabled_list', ["", "orc"])
11371149
@pytest.mark.parametrize("timezone_pair", [("UTC", "Asia/Shanghai"), ("Asia/Shanghai", "UTC"), ("Asia/Shanghai", "America/Los_Angeles")], ids=idfn)
11381150
@tz_sensitive_test
1139-
def test_orc_non_utc_timezone(reader_confs, end_timestamp, spark_tmp_path, v1_enabled_list, timezone_pair):
1151+
def test_orc_reader_writer_the_same_timezone(reader_confs, spark_tmp_path, v1_enabled_list, timezone_pair):
11401152
d_gen = DateGen(start=date(1590, 1, 1))
11411153
# Update start year to 1590 when https://github.com/NVIDIA/spark-rapids/issues/13272 is fixed.
1142-
ts_gen = TimestampGen(start=datetime(1970, 1, 1, tzinfo=timezone.utc), end=end_timestamp, nullable=True)
1154+
# The default end covers the full timestamp range through year 9999, including years > 2200.
1155+
ts_gen = TimestampGen(start=datetime(1970, 1, 2, tzinfo=timezone.utc), nullable=True)
11431156
date_timestamp_gens = [('c1', d_gen), ('c2', ts_gen)]
11441157

11451158
(write_timezone, read_timezone) = timezone_pair
@@ -1154,14 +1167,13 @@ def test_orc_non_utc_timezone(reader_confs, end_timestamp, spark_tmp_path, v1_en
11541167
'spark.rapids.sql.format.orc.enabled': True,
11551168
'spark.rapids.sql.format.orc.read.enabled': True,
11561169
'spark.sql.session.timeZone': read_timezone,
1157-
# ignore write timezone when reading, this is for test purpose only
1158-
# The `tz_sensitive_test` mark guarantees the write and read are in the same timezone
1159-
'spark.rapids.sql.orc.read.ignore.write.timezone': True
11601170
})
11611171

11621172
# write on CPU
11631173
cpu_write_path = spark_tmp_path + "/ORC_DATA_CPU"
11641174
with_cpu_session(lambda spark: gen_df(spark, date_timestamp_gens).write.orc(cpu_write_path), conf=write_confs)
1175+
1176+
# read on GPU and CPU
11651177
assert_gpu_and_cpu_are_equal_collect(read_orc_df(cpu_write_path), conf=read_confs)
11661178

11671179
@pytest.mark.skip(reason='https://github.com/NVIDIA/spark-rapids/issues/13272: CPU can not read ORC file generated by GPU when timestamp is less than 1970 year')

0 commit comments

Comments
 (0)