Skip to content

Commit 243f8fa

Browse files
committed
fix(sandbox): cluster C — invalid date literals reject with typed format-hint message
- C1: pre-isthmus CastTemporalLiteralValidator surfaces malformed string-literal CAST(<lit> AS DATE/TIME/TIMESTAMP) and TIMESTAMPADD/TIMESTAMPDIFF string args as IllegalArgumentException with the typed format-hint wording ("<type>:<value> in unsupported format, please use '<pattern>'"), preventing the Arrow Parser error / StreamException opaque surface. - C2: TimestampFunctionAdapter, DateAdapter, TimeAdapter, DateFormat/TimeFormat adapters, and DATE_PART unit-aware operands now reject malformed inputs at plan time with the typed format-hint message. - C3: HOUR / DAY-of-month and str_to_date with invalid month or day reject with format-hint or out-of-range exception instead of silent success. - DATETIME(<invalid>) folds to a NULL TIMESTAMP at plan time (matches legacy SQL DATETIME-of-invalid semantics). The error surface is HTTP 400 with body {"error": {"reason":"Invalid Query", "details":"<type>:<value> in unsupported format, please use '<pattern>'", "type":"IllegalArgumentException"}}.
1 parent 91b7c0b commit 243f8fa

10 files changed

Lines changed: 572 additions & 32 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.be.datafusion;
10+
11+
import org.apache.calcite.rel.RelHomogeneousShuttle;
12+
import org.apache.calcite.rel.RelNode;
13+
import org.apache.calcite.rex.RexCall;
14+
import org.apache.calcite.rex.RexLiteral;
15+
import org.apache.calcite.rex.RexNode;
16+
import org.apache.calcite.rex.RexShuttle;
17+
import org.apache.calcite.sql.SqlKind;
18+
import org.apache.calcite.sql.type.SqlTypeName;
19+
20+
/** Pre-isthmus pass that rejects malformed CAST(string-lit AS DATE/TIME/TIMESTAMP) and TIMESTAMPADD/DIFF string args. */
21+
final class CastTemporalLiteralValidator {
22+
23+
private CastTemporalLiteralValidator() {}
24+
25+
static RelNode rewrite(RelNode root) {
26+
return root.accept(new RelHomogeneousShuttle() {
27+
@Override
28+
public RelNode visit(RelNode other) {
29+
RelNode visited = super.visit(other);
30+
return visited.accept(new ValidatingShuttle());
31+
}
32+
});
33+
}
34+
35+
/** Test-only entry point; production goes through {@link #rewrite}. */
36+
static RexShuttle newShuttle() {
37+
return new ValidatingShuttle();
38+
}
39+
40+
private static final class ValidatingShuttle extends RexShuttle {
41+
42+
@Override
43+
public RexNode visitCall(RexCall call) {
44+
RexCall recursed = (RexCall) super.visitCall(call);
45+
if (isCastFromString(recursed)) {
46+
DatetimeLiteralValidator.Kind kind = kindForTarget(recursed.getType().getSqlTypeName());
47+
if (kind != null) {
48+
DatetimeLiteralValidator.validate(recursed.getOperands().get(0), kind);
49+
}
50+
return recursed;
51+
}
52+
validateTimestampAddDiffOperands(recursed);
53+
return recursed;
54+
}
55+
56+
/** Substrait can't bind TIMESTAMPADD/DIFF over string args; pre-validate so the user sees the format-hint, not "Unable to convert call". */
57+
private static void validateTimestampAddDiffOperands(RexCall call) {
58+
String name = call.getOperator().getName();
59+
if (!"TIMESTAMPADD".equalsIgnoreCase(name) && !"TIMESTAMPDIFF".equalsIgnoreCase(name)) {
60+
return;
61+
}
62+
int firstArg = "TIMESTAMPADD".equalsIgnoreCase(name) ? 2 : 1;
63+
for (int i = firstArg; i < call.getOperands().size(); i++) {
64+
RexNode operand = call.getOperands().get(i);
65+
if (operand instanceof RexLiteral lit && SqlTypeName.CHAR_TYPES.contains(lit.getType().getSqlTypeName())) {
66+
DatetimeLiteralValidator.validate(operand, DatetimeLiteralValidator.Kind.TIMESTAMP);
67+
}
68+
}
69+
}
70+
71+
private static boolean isCastFromString(RexCall call) {
72+
if (call.getKind() != SqlKind.CAST && call.getKind() != SqlKind.SAFE_CAST) {
73+
return false;
74+
}
75+
if (call.getOperands().size() != 1) {
76+
return false;
77+
}
78+
if (!(call.getOperands().get(0) instanceof RexLiteral literal)) {
79+
return false;
80+
}
81+
return SqlTypeName.CHAR_TYPES.contains(literal.getType().getSqlTypeName());
82+
}
83+
84+
private static DatetimeLiteralValidator.Kind kindForTarget(SqlTypeName target) {
85+
return switch (target) {
86+
case DATE -> DatetimeLiteralValidator.Kind.DATE;
87+
case TIME -> DatetimeLiteralValidator.Kind.TIME;
88+
case TIMESTAMP, TIMESTAMP_WITH_LOCAL_TIME_ZONE -> DatetimeLiteralValidator.Kind.TIMESTAMP;
89+
default -> null;
90+
};
91+
}
92+
}
93+
}

sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DataFusionFragmentConvertor.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ private byte[] convertToSubstrait(RelNode fragment) {
460460
preprocessed = PplWindowCallRewriter.rewrite(preprocessed);
461461
preprocessed = ItemTypeRebuilder.rewrite(preprocessed);
462462
preprocessed = CastToVarcharRewriter.rewrite(preprocessed);
463+
preprocessed = CastTemporalLiteralValidator.rewrite(preprocessed);
463464
RelRoot root = RelRoot.of(preprocessed, SqlKind.SELECT);
464465
SubstraitRelVisitor visitor = createVisitor(preprocessed);
465466
Rel substraitRel;
@@ -491,6 +492,7 @@ private Rel convertStandalone(RelNode operator) {
491492
preprocessed = PplWindowCallRewriter.rewrite(preprocessed);
492493
preprocessed = ItemTypeRebuilder.rewrite(preprocessed);
493494
preprocessed = CastToVarcharRewriter.rewrite(preprocessed);
495+
preprocessed = CastTemporalLiteralValidator.rewrite(preprocessed);
494496
SubstraitRelVisitor visitor = createVisitor(preprocessed);
495497
return visitor.apply(preprocessed);
496498
}

sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DatePartAdapters.java

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.ArrayList;
2929
import java.util.List;
3030
import java.util.Locale;
31+
import java.util.Set;
3132

3233
/**
3334
* Date-part extractor adapters — rewrite {@code FN(ts)} to {@code date_part('<unit>', ts)}.
@@ -64,7 +65,7 @@ public RexNode adapt(RexCall original, List<FieldStorageInfo> fieldStorage, RelO
6465
List<RexNode> coerced = new ArrayList<>(original.getOperands().size());
6566
for (RexNode operand : original.getOperands()) {
6667
if (isCharacterOperand(operand)) {
67-
validateDatetimeLiteral(operand);
68+
validateDatetimeLiteralForUnit(operand, unit);
6869
coerced.add(castToTimestamp(operand, cluster));
6970
} else {
7071
coerced.add(operand);
@@ -105,23 +106,25 @@ static RexNode coerceCharacterOperandToTimestamp(RexNode operand, RelOptCluster
105106
return castToTimestamp(operand, cluster);
106107
}
107108

108-
/**
109-
* Eagerly parses string {@link RexLiteral} operands so an invalid literal surfaces as a
110-
* coordinator-side {@link IllegalArgumentException} during planning, before the value reaches
111-
* DataFusion's CAST kernel. The native error message ({@code "Arrow error: Parser error: ..."})
112-
* is dropped by Flight RPC serialization on the worker→coordinator hop, so without this check
113-
* users see {@code "Failed to start streaming fragment on ..."} instead of the legacy
114-
* {@code "timestamp:<v> in unsupported format"} wording.
115-
*
116-
* <p>Non-literal operands (column refs, expressions) and NULL literals pass through — column
117-
* value validation is a separate concern (Arrow CAST per-row error handling, tracked
118-
* separately).
119-
*
120-
* <p>Accept-set mirrors legacy {@code DateTimeParser.parse}: try {@link LocalDateTime} (date+time
121-
* with optional nanos), {@link LocalDate} (bare date), {@link LocalTime} (bare time), throw on
122-
* all-failed. Same try/catch-fall-through shape used by
123-
* {@link TimestampFunctionAdapter#parseTimestamp}.
124-
*/
109+
/** Time-only units reject bare-date literals (HOUR('2020-08-26') must throw, not return 0). */
110+
private static final Set<String> TIME_ONLY_UNITS = Set.of("hour", "minute", "second", "microsecond");
111+
112+
/** Date-part units reject bare-time literals (DAY('12:00:00') must throw, not silent-fail). */
113+
private static final Set<String> DATE_ONLY_UNITS = Set.of("year", "quarter", "month", "day", "week", "doy", "dow");
114+
115+
static void validateDatetimeLiteralForUnit(RexNode operand, String unit) {
116+
if (TIME_ONLY_UNITS.contains(unit)) {
117+
DatetimeLiteralValidator.validate(operand, DatetimeLiteralValidator.Kind.TIME);
118+
return;
119+
}
120+
if (DATE_ONLY_UNITS.contains(unit)) {
121+
DatetimeLiteralValidator.validate(operand, DatetimeLiteralValidator.Kind.DATE);
122+
return;
123+
}
124+
validateDatetimeLiteral(operand);
125+
}
126+
127+
/** Plan-time validation for string-literal operands; accepts datetime / date / time, rejects garbage. */
125128
static void validateDatetimeLiteral(RexNode operand) {
126129
if (!(operand instanceof RexLiteral literal)) {
127130
return;
@@ -144,9 +147,7 @@ static void validateDatetimeLiteral(RexNode operand) {
144147
LocalTime.parse(value);
145148
return;
146149
} catch (DateTimeParseException ignored) {
147-
// SQL plugin's ErrorMessageFactory.unwrapCause walks to the deepest cause for the response
148-
// type/details, so attaching DateTimeParseException as cause would surface its stock JDK
149-
// message instead of this one. Mirrors legacy DateTimeParser.parse, which throws causeless.
150+
// causeless — see DatetimeLiteralValidator#fail.
150151
throw new IllegalArgumentException(
151152
String.format(Locale.ROOT, "timestamp:%s in unsupported format, please use 'yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'", value)
152153
);

sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DateTimeAdapters.java

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.apache.calcite.plan.RelOptCluster;
1212
import org.apache.calcite.rex.RexBuilder;
1313
import org.apache.calcite.rex.RexCall;
14+
import org.apache.calcite.rex.RexLiteral;
1415
import org.apache.calcite.rex.RexNode;
1516
import org.apache.calcite.sql.SqlFunction;
1617
import org.apache.calcite.sql.SqlFunctionCategory;
@@ -24,6 +25,8 @@
2425
import org.opensearch.analytics.spi.FieldStorageInfo;
2526
import org.opensearch.analytics.spi.ScalarFunctionAdapter;
2627

28+
import java.time.LocalDateTime;
29+
import java.time.format.DateTimeParseException;
2730
import java.util.List;
2831

2932
/**
@@ -138,6 +141,7 @@ public RexNode adapt(RexCall original, List<FieldStorageInfo> fieldStorage, RelO
138141
List<RexNode> operands = original.getOperands();
139142
if (operands.size() == 1 && SqlTypeName.CHAR_TYPES.contains(operands.get(0).getType().getSqlTypeName())) {
140143
RexNode arg = operands.get(0);
144+
DatetimeLiteralValidator.validate(arg, DatetimeLiteralValidator.Kind.TIME);
141145
RexNode pattern = rexBuilder.makeLiteral(DATE_PREFIX_PATTERN);
142146
RexNode replacement = rexBuilder.makeLiteral("");
143147
RexNode stripped = rexBuilder.makeCall(
@@ -151,15 +155,23 @@ public RexNode adapt(RexCall original, List<FieldStorageInfo> fieldStorage, RelO
151155
}
152156
}
153157

154-
static final class DateAdapter extends AbstractNameMappingAdapter {
155-
DateAdapter() {
156-
super(LOCAL_DATE_OP, List.of(), List.of());
158+
static final class DateAdapter implements ScalarFunctionAdapter {
159+
@Override
160+
public RexNode adapt(RexCall original, List<FieldStorageInfo> fieldStorage, RelOptCluster cluster) {
161+
if (original.getOperands().size() == 1) {
162+
RexNode operand = original.getOperands().get(0);
163+
if (SqlTypeName.CHAR_TYPES.contains(operand.getType().getSqlTypeName())) {
164+
DatetimeLiteralValidator.validate(operand, DatetimeLiteralValidator.Kind.DATE);
165+
}
166+
}
167+
return cluster.getRexBuilder().makeCall(original.getType(), LOCAL_DATE_OP, original.getOperands());
157168
}
158169
}
159170

160171
/**
161172
* Single-arg {@code DATETIME(string)} strips the trailing offset (+HH:MM / -HHMM / Z) so the
162-
* result is wall-clock, not UTC-converted. Two-arg {@code DATETIME(string, tz)} and
173+
* result is wall-clock, not UTC-converted. Invalid string literals fold to NULL TIMESTAMP
174+
* (legacy SQL DATETIME-of-invalid semantics). Two-arg {@code DATETIME(string, tz)} and
163175
* non-string operands route through {@code to_timestamp} unchanged.
164176
*
165177
* <p>This belongs in the PPL frontend ({@code PPLBuiltinOperators.DATETIME}) so every
@@ -175,6 +187,10 @@ public RexNode adapt(RexCall original, List<FieldStorageInfo> fieldStorage, RelO
175187
List<RexNode> operands = original.getOperands();
176188
if (operands.size() == 1 && SqlTypeName.CHAR_TYPES.contains(operands.get(0).getType().getSqlTypeName())) {
177189
RexNode arg = operands.get(0);
190+
RexNode foldedNull = tryFoldInvalidLiteralToNull(arg, original, cluster);
191+
if (foldedNull != null) {
192+
return foldedNull;
193+
}
178194
RexNode pattern = rexBuilder.makeLiteral(OFFSET_SUFFIX_PATTERN);
179195
RexNode replacement = rexBuilder.makeLiteral("");
180196
RexNode stripped = rexBuilder.makeCall(
@@ -186,5 +202,23 @@ public RexNode adapt(RexCall original, List<FieldStorageInfo> fieldStorage, RelO
186202
}
187203
return rexBuilder.makeCall(original.getType(), LOCAL_TO_TIMESTAMP_OP, operands);
188204
}
205+
206+
/** Folds any non-full-datetime string literal to NULL TIMESTAMP after stripping the offset suffix. */
207+
private static RexNode tryFoldInvalidLiteralToNull(RexNode operand, RexCall original, RelOptCluster cluster) {
208+
if (!(operand instanceof RexLiteral literal)) {
209+
return null;
210+
}
211+
String value = literal.getValueAs(String.class);
212+
if (value == null) {
213+
return null;
214+
}
215+
String stripped = value.replaceAll(OFFSET_SUFFIX_PATTERN, "");
216+
try {
217+
LocalDateTime.parse(stripped.replace(' ', 'T'));
218+
return null;
219+
} catch (DateTimeParseException ignored) {
220+
return cluster.getRexBuilder().makeNullLiteral(original.getType());
221+
}
222+
}
189223
}
190224
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.be.datafusion;
10+
11+
import org.apache.calcite.rex.RexLiteral;
12+
import org.apache.calcite.rex.RexNode;
13+
14+
import java.time.LocalDate;
15+
import java.time.LocalDateTime;
16+
import java.time.LocalTime;
17+
import java.time.format.DateTimeParseException;
18+
import java.util.Locale;
19+
20+
/** Plan-time validation for string-literal datetime operands; rejects malformed input with the format-hint message. */
21+
final class DatetimeLiteralValidator {
22+
23+
private DatetimeLiteralValidator() {}
24+
25+
enum Kind {
26+
DATE("date", "yyyy-MM-dd"),
27+
TIME("time", "HH:mm:ss[.SSSSSSSSS]"),
28+
TIMESTAMP("timestamp", "yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]");
29+
30+
final String typeName;
31+
final String pattern;
32+
33+
Kind(String typeName, String pattern) {
34+
this.typeName = typeName;
35+
this.pattern = pattern;
36+
}
37+
}
38+
39+
static void validate(RexNode operand, Kind kind) {
40+
if (!(operand instanceof RexLiteral literal)) {
41+
return;
42+
}
43+
String value = literal.getValueAs(String.class);
44+
if (value == null) {
45+
return;
46+
}
47+
validate(value, kind);
48+
}
49+
50+
static void validate(String value, Kind kind) {
51+
boolean ok = switch (kind) {
52+
case DATE -> isParseableAsDate(value) || isParseableAsFullDatetime(value);
53+
case TIME -> isParseableAsTime(value) || isParseableAsFullDatetime(value);
54+
case TIMESTAMP -> isParseableAsFullDatetime(value) || isParseableAsDate(value);
55+
};
56+
if (!ok) {
57+
throw fail(value, kind);
58+
}
59+
}
60+
61+
private static boolean isParseableAsDate(String value) {
62+
try {
63+
LocalDate.parse(value);
64+
return true;
65+
} catch (DateTimeParseException ignored) {
66+
return false;
67+
}
68+
}
69+
70+
private static boolean isParseableAsTime(String value) {
71+
try {
72+
LocalTime.parse(value);
73+
return true;
74+
} catch (DateTimeParseException ignored) {
75+
return false;
76+
}
77+
}
78+
79+
private static boolean isParseableAsFullDatetime(String value) {
80+
try {
81+
LocalDateTime.parse(value.replace(' ', 'T'));
82+
return true;
83+
} catch (DateTimeParseException ignored) {
84+
return false;
85+
}
86+
}
87+
88+
/** Causeless: ErrorMessageFactory.unwrapCause would surface the JDK DateTimeParseException's stock message otherwise. */
89+
private static IllegalArgumentException fail(String value, Kind kind) {
90+
return new IllegalArgumentException(
91+
String.format(Locale.ROOT, "%s:%s in unsupported format, please use '%s'", kind.typeName, value, kind.pattern)
92+
);
93+
}
94+
}

0 commit comments

Comments
 (0)