[
https://issues.apache.org/jira/browse/HIVE-30206?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dimitri BEKALE updated HIVE-30206:
----------------------------------
Description:
{\{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.
was:
{\{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.
> 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
>
> {\{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
(v8.20.10#820010)
