[
https://issues.apache.org/jira/browse/HIVE-30206?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated HIVE-30206:
----------------------------------
Labels: pull-request-available (was: )
> 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.0.0, 4.2.1
> Reporter: Dimitri BEKALE
> Priority: Minor
> Labels: pull-request-available
>
> {\{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 to 342), if the
> last separator character is a hyphen 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 a hyphen, 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 (same separator shape without \{{tzh}}; single
> hyphen, so the
> javadoc's "only separator character" exemption applies):
> {noformat}
> OK pattern="FXyyyy-mm-dd -hh24:mi" input="2026-10-20 -10:30"
> OK pattern="FXyyyy-mm-dd-tzh:tzm" input="2026-10-20-07:00"
> {noformat}
> The pattern is not merely awkward, it is unusable. Trying every separator
> filler of length 1 to 4 built from the seven valid separator characters
> (2,800 inputs) against the pattern above, exactly one parses, namely space
> followed by two hyphens; the natural input with space and one hyphen does not:
> {noformat}
> OK "2026-10-20 --07:00"
> THROW "2026-10-20 -07:00"
> {noformat}
> 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 a hyphen 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, the inputs with a space and a hyphen or a plus sign parse, while
> the input with two spaces is still rejected, so FX exactness is kept:
> {noformat}
> OK "2026-10-20 -07:00"
> OK "2026-10-20 +07:00"
> THROW "2026-10-20 07:00"
> {noformat}
> 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 rather than a CI-validated patch. 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
(v10.3.26#10030026)
