rgbuilds commented on code in PR #26452: URL: https://github.com/apache/datafusion/pull/26452#discussion_r4409648815
########## datafusion/sqllogictest/test_files/parquet_cast_pruning.slt: ########## @@ -0,0 +1,192 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: Agreed. I moved this regression section into `push_down_filter_parquet.slt` and removed the standalone SLT file. ########## datafusion/sqllogictest/test_files/parquet_cast_pruning.slt: ########## @@ -0,0 +1,192 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Regression tests for casts that cannot be applied to min/max statistics when +# pruning Parquet files. +# +# Pruning must never change a query's answer. A cast whose output does not +# preserve the ordering of its input can invert the bounds derived from +# statistics, which makes pruning discard row groups that do contain matching +# rows. Every predicate below is therefore run twice: once against an +# in-memory table, which is never pruned and so gives the ground truth, and +# once against the same rows stored as Parquet. The two must agree. + +# Disable the listing cache so DataFusion picks up changes from COPY statements. +statement ok +set datafusion.runtime.list_files_cache_limit = "0K"; Review Comment: You’re right, this isn’t necessary here because each file is written before the external table is created. Removed the cache override and reset. ########## datafusion/sqllogictest/test_files/parquet_cast_pruning.slt: ########## @@ -0,0 +1,192 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Regression tests for casts that cannot be applied to min/max statistics when +# pruning Parquet files. +# +# Pruning must never change a query's answer. A cast whose output does not +# preserve the ordering of its input can invert the bounds derived from +# statistics, which makes pruning discard row groups that do contain matching +# rows. Every predicate below is therefore run twice: once against an +# in-memory table, which is never pruned and so gives the ground truth, and +# once against the same rows stored as Parquet. The two must agree. + +# Disable the listing cache so DataFusion picks up changes from COPY statements. +statement ok +set datafusion.runtime.list_files_cache_limit = "0K"; + +########## +# Casting a timestamp to a time-of-day wraps at midnight +# +# https://github.com/apache/datafusion/issues/26451 +########## + +# Three timestamps at 23:00, 00:00 and 01:00, written as a single row group so +# that the row group's min/max bracket midnight. Casting those bounds to +# time-of-day turns them into 23:00:00 and 01:00:00, which excludes every value +# that actually lies between them. +statement ok +CREATE TABLE source_data(ts TIMESTAMP) AS VALUES + (TIMESTAMP '2024-03-10 23:00:00'), + (TIMESTAMP '2024-03-11 00:00:00'), + (TIMESTAMP '2024-03-11 01:00:00'); + +statement ok +COPY (SELECT ts FROM source_data) +TO 'test_files/scratch/parquet_cast_pruning/timestamps.parquet' +STORED AS PARQUET +OPTIONS ('format.max_row_group_size' '3'); + +statement ok +CREATE EXTERNAL TABLE timestamps +STORED AS PARQUET +LOCATION 'test_files/scratch/parquet_cast_pruning/timestamps.parquet'; + +# Equality against the row at midnight. +query P +SELECT ts FROM source_data WHERE CAST(ts AS TIME) = TIME '00:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00 + +query P +SELECT ts FROM timestamps WHERE CAST(ts AS TIME) = TIME '00:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00 + +# Range predicate spanning the wrap point. +query P +SELECT ts FROM source_data WHERE CAST(ts AS TIME) < TIME '02:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00 +2024-03-11T01:00:00 + +query P +SELECT ts FROM timestamps WHERE CAST(ts AS TIME) < TIME '02:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00 +2024-03-11T01:00:00 + +# The same predicates spelled with arrow_cast, which also reaches Time32. +query P +SELECT ts FROM source_data +WHERE arrow_cast(ts, 'Time32(Second)') = arrow_cast('00:00:00', 'Time32(Second)') +ORDER BY ts; +---- +2024-03-11T00:00:00 + +query P +SELECT ts FROM timestamps +WHERE arrow_cast(ts, 'Time32(Second)') = arrow_cast('00:00:00', 'Time32(Second)') +ORDER BY ts; +---- +2024-03-11T00:00:00 + +query P +SELECT ts FROM source_data +WHERE arrow_cast(ts, 'Time32(Second)') < arrow_cast('02:00:00', 'Time32(Second)') +ORDER BY ts; +---- +2024-03-11T00:00:00 +2024-03-11T01:00:00 + +query P +SELECT ts FROM timestamps +WHERE arrow_cast(ts, 'Time32(Second)') < arrow_cast('02:00:00', 'Time32(Second)') +ORDER BY ts; +---- +2024-03-11T00:00:00 +2024-03-11T01:00:00 + +statement ok +DROP TABLE timestamps; + +########## +# The same wrap happens at the local midnight of a timezone-aware column +########## + +# The same three local wall-clock times, tagged +05:00, so the instants +# actually stored are 18:00, 19:00 and 20:00 UTC on 2024-03-10. Those are +# monotonic and fall inside a single UTC day, so the min/max bounds only +# invert if the cast honours the column's timezone. +statement ok +CREATE TABLE source_data_tz AS +SELECT arrow_cast(ts, 'Timestamp(Nanosecond, Some("+05:00"))') AS ts +FROM source_data; + +statement ok +DROP TABLE source_data; + +statement ok +COPY (SELECT ts FROM source_data_tz) +TO 'test_files/scratch/parquet_cast_pruning/timestamps_tz.parquet' +STORED AS PARQUET +OPTIONS ('format.max_row_group_size' '3'); + +statement ok +CREATE EXTERNAL TABLE timestamps_tz +STORED AS PARQUET +LOCATION 'test_files/scratch/parquet_cast_pruning/timestamps_tz.parquet'; + +# Pin the shape of the data this section depends on: the local times have to +# straddle a local midnight while the stored instants stay monotonic within a +# single UTC day. Without this, a change to how a timezone is attached would +# leave the queries below passing while no longer exercising the wrap at all. +query TIT +SELECT + ts::string AS local_time, + to_unixtime(ts) AS stored_instant, + CAST(ts AS TIME)::string AS time_of_day +FROM timestamps_tz ORDER BY ts; +---- +2024-03-10T23:00:00+05:00 1710093600 23:00:00 +2024-03-11T00:00:00+05:00 1710097200 00:00:00 +2024-03-11T01:00:00+05:00 1710100800 01:00:00 + +query P +SELECT ts FROM source_data_tz WHERE CAST(ts AS TIME) = TIME '00:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00+05:00 + +query P +SELECT ts FROM timestamps_tz WHERE CAST(ts AS TIME) = TIME '00:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00+05:00 + +query P +SELECT ts FROM source_data_tz WHERE CAST(ts AS TIME) < TIME '02:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00+05:00 +2024-03-11T01:00:00+05:00 + +query P +SELECT ts FROM timestamps_tz WHERE CAST(ts AS TIME) < TIME '02:00:00' ORDER BY ts; +---- +2024-03-11T00:00:00+05:00 +2024-03-11T01:00:00+05:00 + +statement ok +DROP TABLE timestamps_tz; + +statement ok +DROP TABLE source_data_tz; + +# Config reset. +statement ok +RESET datafusion.runtime.list_files_cache_limit; + +statement ok +RESET datafusion.catalog.create_default_catalog_and_schema; Review Comment: Correct, this setting is unchanged by the test. Removed the leftover reset. -- 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]
