wombatu-kun commented on code in PR #19474:
URL: https://github.com/apache/hudi/pull/19474#discussion_r4479008054
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/TestDataSkippingUtils.scala:
##########
@@ -857,7 +923,128 @@ object TestDataSkippingUtils {
C_nullCount = 0)
),
Seq("file_1"))
+ // NOTE: Parsing (to_date / to_timestamp with a format) is pinned in
+ // testDateTimeFilterExpressionsSource instead: on the real read
path the optimizer
+ // always rewrites ParseToDate / ParseToTimestamp before Hudi sees
the filter
+ )
+ }
+ def testDateTimeFilterExpressionsSource():
java.util.stream.Stream[Arguments] = {
+ // NOTE: All timestamps in UTC. Column B holds 'yyyy-MM-dd' formatted
strings, for which
+ // lexicographic ordering coincides with chronological ordering
+ val input = Seq(
+ IndexRow("file_1", valueCount = 1,
+ B_minValue = "2022-03-07",
+ B_maxValue = "2022-03-08",
+ B_nullCount = 0),
+ IndexRow("file_2", valueCount = 1,
+ B_minValue = "2022-03-06",
+ B_maxValue = "2022-03-06",
+ B_nullCount = 0)
+ )
+ // Column B holding 'yyyy-MM-dd HH:mm:ss' strings, the default format of
unix_timestamp
+ val inputDateTime = Seq(
+ IndexRow("file_1", valueCount = 2,
+ B_minValue = "2022-03-07 00:00:00",
+ B_maxValue = "2022-03-08 23:59:59",
+ B_nullCount = 0),
+ IndexRow("file_2", valueCount = 2,
+ B_minValue = "2022-03-06 00:00:00",
+ B_maxValue = "2022-03-06 23:59:59",
+ B_nullCount = 0)
+ )
+ val inputTimestamps = Seq(
+ IndexRow("file_1", valueCount = 2,
+ C_minValue = new Timestamp(1646711448000L), // 2022-03-08 03:50:48
+ C_maxValue = new Timestamp(1646797848000L), // 2022-03-09 03:50:48
+ C_nullCount = 0),
+ IndexRow("file_2", valueCount = 2,
+ C_minValue = new Timestamp(1646625048000L), // 2022-03-07 03:50:48
+ C_maxValue = new Timestamp(1646711448000L), // 2022-03-08 03:50:48
+ C_nullCount = 0)
+ )
+ // Column A holding seconds since the epoch. Long.MaxValue seconds
overflow microseconds and
+ // format as '1969-12-31', below the min: file_overflow holds 2022-03-07
but would be pruned
+ val inputSeconds = Seq(
+ IndexRow("file_1", valueCount = 2, A_minValue = 1646611200L, A_maxValue
= 1646697599L, A_nullCount = 0),
+ IndexRow("file_2", valueCount = 2, A_minValue = 1646697600L, A_maxValue
= 1646783999L, A_nullCount = 0),
+ IndexRow("file_overflow", valueCount = 3, A_minValue = 0L, A_maxValue =
Long.MaxValue, A_nullCount = 0)
+ )
+ // US-format strings: the lexicographic min/max ('03/06/2022' /
'12/31/2021') do NOT
+ // correspond to the chronologically smallest/largest values
+ val inputUsFormat = Seq(
+ IndexRow("file_us", valueCount = 2,
+ B_minValue = "03/06/2022",
+ B_maxValue = "12/31/2021",
+ B_nullCount = 0)
+ )
+ // Variable-width year-first strings: '2022-11-5' < '2022-3-6'
lexicographically even though
+ // it is the chronologically larger value
+ val inputVariableWidth = Seq(
+ IndexRow("file_slop", valueCount = 2,
+ B_minValue = "2022-11-5",
+ B_maxValue = "2022-3-6",
+ B_nullCount = 0)
+ )
+ // The lexicographic max 'zzz' does not parse; the file still holds a
parseable matching value
+ val inputUnparseableMax = Seq(
+ IndexRow("file_mixed", valueCount = 2,
+ B_minValue = "2022-03-07",
+ B_maxValue = "zzz",
+ B_nullCount = 0)
+ )
+ // The max '2022-3-6' starts with a digit but does not parse with
'yyyy-MM-dd' (it only parses
+ // leniently, so the Spark 3 EXCEPTION parser policy throws on it, as ANSI
does); the file still
+ // holds, for ex, '2022-03-07'
+ val inputSloppyMax = Seq(
+ IndexRow("file_sloppy", valueCount = 3,
+ B_minValue = "2022-03-01",
+ B_maxValue = "2022-3-6",
+ B_nullCount = 0)
+ )
+ java.util.stream.Stream.of(
+ // to_date(B) is replaced with Cast(B as date), handled by the Cast rule
(regression pin; does
+ // not exercise GetTimestamp)
+ arguments("to_date(B) > '2022-03-06'", input, Seq("file_1")),
Review Comment:
Master's `CastRule` now denies string to date/timestamp casts, so once
master is merged in, `to_date(B)` and `to_timestamp(B)` (a bare `Cast` of `B`
after optimization) keep both files, and this case and the `to_timestamp(B) >
'2022-03-06 12:00:00'` one below fail. Could you merge master and expect
`Seq("file_1", "file_2")` for both, or drop the two pins?
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/dataskipping/rules/DateTimeOrdering.scala:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.hudi.dataskipping.rules
+
+import org.apache.spark.sql.catalyst.expressions.{Expression, Literal}
+import org.apache.spark.sql.catalyst.util.DateTimeUtils
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types.StringType
+import org.apache.spark.unsafe.types.UTF8String
+
+import scala.util.Try
+
+/**
+ * Shared guards of [[DateTimeFormatRule]]: when does turning a timestamp into
text (formatting) or
+ * text into a timestamp (parsing) preserve ordering?
+ *
+ * - Pattern: the format is a literal on the allowlist below. Every
allowlisted pattern is year-first
+ * and fixed-width, so the lexicographic order of the strings is the
chronological order of the
+ * values they denote, within years 0001 to 9999 (outside, the year gets a
sign or a fifth digit).
+ * - Policy: `spark.sql.legacy.timeParserPolicy` is not LEGACY. LEGACY parses
with SimpleDateFormat,
+ * which is lenient (for ex, '2022-3-6', '2022-03-06x' and ' 2022-03-06'
all parse with
+ * 'yyyy-MM-dd'), so parseable strings are no longer fixed-width.
+ * - Zone, parsing only: a time-of-day pattern parsed in the session time
zone needs a fixed-offset
+ * zone. Across a spring-forward gap, a wall clock inside the gap is
shifted forward past later ones
+ * (for ex, in America/Los_Angeles '2022-03-13 02:30:00' parses after
'2022-03-13 03:05:00'). A
+ * date-only parse is monotone in every zone (a gap shifts a start of day
by less than a day).
+ *
+ * NOTE: The zone side of formatting is guarded elsewhere: a time-of-day
pattern in a DST session
+ * zone by #20521, a local date that runs backwards at a fall-back
across midnight by #20526
+ */
+object DateTimeOrdering {
+
+ /** An allowlisted format pattern; `hasTimeOfDay` when it holds a field
finer than the day. */
+ case class Pattern(hasTimeOfDay: Boolean)
+
+ // NOTE: Year-first, fixed-width patterns only. Day-first or month-first
patterns (for ex,
+ // 'dd-MM-yyyy', 'MM/dd/yyyy'), unpadded fields (for ex, 'yyyy-M-d'),
and text fields (for ex,
+ // 'EEE', 'MMM') do not sort chronologically. 'SSS' parses 1 to 3
digits, whose order as a
+ // decimal fraction matches their lexicographic order
+ private val datePatterns: Set[String] = Set(
+ "yyyy",
+ "yyyy-MM",
+ "yyyy-MM-dd",
+ "yyyyMMdd",
+ "yyyy/MM/dd")
+
+ private val dateTimePatterns: Set[String] = Set(
+ "yyyy-MM-dd HH",
+ "yyyy-MM-dd HH:mm",
+ "yyyy-MM-dd HH:mm:ss",
+ "yyyy-MM-dd HH:mm:ss.SSS",
+ "yyyy-MM-dd'T'HH:mm:ss",
Review Comment:
Spark's datetime parser matches literals case-insensitively, so in a UTC
session `'2022-03-05t09:00:00'` parses with this pattern yet sorts after
`'2022-03-05T23:00:00'`, and a file holding both gets bounds 23:00..09:00 and
is pruned for `to_timestamp(s, "yyyy-MM-dd'T'HH:mm:ss") > '2022-03-05
12:00:00'`. Could the two `'T'` patterns be allowed for formatting only, since
`date_format` always writes an upper-case `T`?
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/dataskipping/rules/DateTimeOrdering.scala:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.hudi.dataskipping.rules
+
+import org.apache.spark.sql.catalyst.expressions.{Expression, Literal}
+import org.apache.spark.sql.catalyst.util.DateTimeUtils
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types.StringType
+import org.apache.spark.unsafe.types.UTF8String
+
+import scala.util.Try
+
+/**
+ * Shared guards of [[DateTimeFormatRule]]: when does turning a timestamp into
text (formatting) or
+ * text into a timestamp (parsing) preserve ordering?
+ *
+ * - Pattern: the format is a literal on the allowlist below. Every
allowlisted pattern is year-first
+ * and fixed-width, so the lexicographic order of the strings is the
chronological order of the
+ * values they denote, within years 0001 to 9999 (outside, the year gets a
sign or a fifth digit).
+ * - Policy: `spark.sql.legacy.timeParserPolicy` is not LEGACY. LEGACY parses
with SimpleDateFormat,
+ * which is lenient (for ex, '2022-3-6', '2022-03-06x' and ' 2022-03-06'
all parse with
+ * 'yyyy-MM-dd'), so parseable strings are no longer fixed-width.
+ * - Zone, parsing only: a time-of-day pattern parsed in the session time
zone needs a fixed-offset
+ * zone. Across a spring-forward gap, a wall clock inside the gap is
shifted forward past later ones
+ * (for ex, in America/Los_Angeles '2022-03-13 02:30:00' parses after
'2022-03-13 03:05:00'). A
+ * date-only parse is monotone in every zone (a gap shifts a start of day
by less than a day).
+ *
+ * NOTE: The zone side of formatting is guarded elsewhere: a time-of-day
pattern in a DST session
+ * zone by #20521, a local date that runs backwards at a fall-back
across midnight by #20526
+ */
+object DateTimeOrdering {
+
+ /** An allowlisted format pattern; `hasTimeOfDay` when it holds a field
finer than the day. */
+ case class Pattern(hasTimeOfDay: Boolean)
+
+ // NOTE: Year-first, fixed-width patterns only. Day-first or month-first
patterns (for ex,
+ // 'dd-MM-yyyy', 'MM/dd/yyyy'), unpadded fields (for ex, 'yyyy-M-d'),
and text fields (for ex,
+ // 'EEE', 'MMM') do not sort chronologically. 'SSS' parses 1 to 3
digits, whose order as a
+ // decimal fraction matches their lexicographic order
+ private val datePatterns: Set[String] = Set(
+ "yyyy",
+ "yyyy-MM",
+ "yyyy-MM-dd",
+ "yyyyMMdd",
+ "yyyy/MM/dd")
+
+ private val dateTimePatterns: Set[String] = Set(
+ "yyyy-MM-dd HH",
+ "yyyy-MM-dd HH:mm",
+ "yyyy-MM-dd HH:mm:ss",
+ "yyyy-MM-dd HH:mm:ss.SSS",
+ "yyyy-MM-dd'T'HH:mm:ss",
+ "yyyy-MM-dd'T'HH:mm:ss.SSS",
+ "yyyyMMddHHmmss",
+ "yyyy/MM/dd HH:mm:ss")
+
+ /**
+ * The allowlisted pattern of a format operand. Only literals: a default
format (for ex, the one of
+ * `unix_timestamp(s)`) is a literal too.
+ */
+ def pattern(format: Expression): Option[Pattern] = format match {
+ case Literal(value: UTF8String, _: StringType) =>
+ val p = value.toString
+ if (datePatterns.contains(p)) {
+ Some(Pattern(hasTimeOfDay = false))
+ } else if (dateTimePatterns.contains(p)) {
+ Some(Pattern(hasTimeOfDay = true))
+ } else {
+ None
+ }
+ case _ => None
+ }
+
+ /**
+ * Read when the rule runs, i.e. from the session that compiles the filter,
which is the one that
+ * evaluates it.
+ */
+ def isLegacyTimeParserPolicy: Boolean =
+
"LEGACY".equalsIgnoreCase(SQLConf.get.getConf(SQLConf.LEGACY_TIME_PARSER_POLICY))
Review Comment:
On Spark 4.1 and 4.2 `SQLConf.LEGACY_TIME_PARSER_POLICY` is an enum conf, so
`getConf` returns `LegacyBehaviorPolicy.Value` and this line does not compile
under the `spark4.1` / `spark4.2` profiles ("type mismatch; required: String").
Could you compare on `.toString`, e.g.
`"LEGACY".equalsIgnoreCase(SQLConf.get.getConf(SQLConf.LEGACY_TIME_PARSER_POLICY).toString)`?
--
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]
