Dimitri BEKALE created HIVE-30206:
-------------------------------------
Summary: CAST ... FORMAT: an FX pattern whose separator before TZH
ends in "-" can never be parsed
Key: HIVE-30206
URL: https://issues.apache.org/jira/browse/HIVE-30206
Project: Hive
Issue Type: Bug
Affects Versions: 4.2.1, 4.0.0
Reporter: Dimitri BEKALE
{\{HiveSqlDateTimeFormatter.parseSeparator}} compares the separator run found
in the input against the separator run in the pattern when \{{FX}} is on, at
[HiveSqlDateTimeFormatter.java:1453|https://github.com/apache/hive/blob/295a0e79169c1eee7f2b2746526b7ab1f723d958/common/src/java/org/apache/hadoop/hive/common/format/datetime/HiveSqlDateTimeFormatter.java#L1453]:
{code:java}
if (formatExact && !token.string.equals(separatorsFound.toString())) {
throw new IllegalArgumentException("FX on and separator found: " +
separatorsFound
+ " does not match expected separator: " + token.string);
}
{code}
The two sides are built under different rules. Per the class javadoc (lines
339-342), if the last
separator character is \{{-}} and is immediately followed by a \{{tzh}} token,
it is the time zone's negative sign and is *not* counted as a separator, unless
it is the only separator character. So \{{separatorsFound}} omits that hyphen,
while \{{token.string}} - the raw run from the pattern - keeps it. When a
pattern's separator run before \{{tzh}} is longer than one character and ends
in \{{-}}, the two can never match and FX always throws.
h3. Reproducer
{code:java}
// parses
new HiveSqlDateTimeFormatter("yyyy-mm-dd -tzh:tzm",
true).parseTimestamp("2026-10-20 -07:00");
// same pattern with FX: throws
new HiveSqlDateTimeFormatter("FXyyyy-mm-dd -tzh:tzm",
true).parseTimestamp("2026-10-20 -07:00");
// IllegalArgumentException: FX on and separator found: does not match
expected separator: -
{code}
Two controls isolate the cause: \{{FXyyyy-mm-dd -hh24:mi}} with \{{"2026-10-20
-10:30"}} parses (same separator shape, no \{{tzh}}), and
\{{FXyyyy-mm-dd-tzh:tzm}} with \{{"2026-10-20-07:00"}} parses (single \{{-}},
so the javadoc's "only separator character" exemption applies).
The pattern is not merely awkward, it is unusable: trying every separator
filler of length 1-4 built from the seven valid separator characters (2,800
inputs) against \{{FXyyyy-mm-dd -tzh:tzm}}, exactly one parses - \{{" --"}} -
and the natural \{{" -"}} does not.
h3. Impact
Reachable from SQL through \{{CAST (... AS TIMESTAMP FORMAT '...')}}. The user
gets
{\{IllegalArgumentException}} on a pattern that is accepted when FX is off, and
no input they are likely to write will satisfy it. Severity is minor: the
affected patterns are a narrow shape (a
multi-character separator run ending in \{{-}} immediately before \{{tzh}}),
and everything else about FX and \{{tzh}} works.
h3. Origin
{\{git blame}} attributes line 1453 to \{{03aae630d9fd}} (2019-07-29,
HIVE-21578, "Introduce SQL:2016 formats FM, FX, and nested strings"). Present
verbatim on \{{rel/release-4.0.0}} (line 1446) and \{{rel/release-4.2.1}} (line
1452) as well as master.
{\{TestHiveSqlDateTimeFormatter}} has FX assertions in \{{testFx()}} and
\{{tzh}} assertions elsewhere, but no test combines the two, which is
presumably why this has gone unnoticed.
h3. Proposed fix
Apply the same hyphen rule to the expected string before comparing:
{code}
- if (formatExact && !token.string.equals(separatorsFound.toString())) {
+ String expectedSeparator = token.string;
+ if (expectedSeparator.length() > 1 && expectedSeparator.endsWith("-")
+ && nextTokenIs("tzh", token)) {
+ expectedSeparator = expectedSeparator.substring(0,
expectedSeparator.length() - 1);
+ }
+ if (formatExact && !expectedSeparator.equals(separatorsFound.toString())) {
throw new IllegalArgumentException("FX on and separator found: " +
separatorsFound
- + " does not match expected separator: " + token.string);
+ + " does not match expected separator: " + expectedSeparator);
}
{code}
With this, \{{"2026-10-20 -07:00"}} and \{{"2026-10-20 +07:00"}} parse, while
\{{"2026-10-20 07:00"}} is still rejected, so FX exactness is kept. A test for
\{{testFx()}}:
{code:java}
// FX with a separator ending in "-" before tzh: the "-" is the tzh sign, not a
separator
checkParseTimestamp("FXyyyy-mm-dd -tzh:tzm", "2026-10-20 -07:00", "2026-10-20
00:00:00");
checkParseTimestamp("FXyyyy-mm-dd -tzh:tzm", "2026-10-20 +07:00", "2026-10-20
00:00:00");
verifyBadParseString("FXyyyy-mm-dd -tzh:tzm", "2026-10-20 07:00");
{code}
I replayed all 154 parse assertions in \{{TestHiveSqlDateTimeFormatter}}
against the patched class with the suite's fixed clock and all of them still
pass, but I have not run the full Hive build, so please treat this as a
proposal. I am happy to open a PR if that is welcome, and equally happy to be
told the intended behaviour here is different.
h3. Provenance
This was found during an academic study of LLM-assisted code review at Jeonbuk
National University, Jeonju, South Korea. An automated LLM-based reviewer
produced the initial analysis; I verified it by reading the source at
\{{295a0e7916}} and by compiling and running the formatter against the inputs
above. I believe it is a genuine defect but a "not a bug" outcome is useful to
me as a correction.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
