Skip to content

Commit

Permalink
Revert automatic cast feature and corresponding changes.
Browse files Browse the repository at this point in the history
Signed-off-by: Yury Fridlyand <[email protected]>
  • Loading branch information
Yury-Fridlyand committed Aug 2, 2022
1 parent e511e30 commit 9f6b66c
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 115 deletions.
20 changes: 10 additions & 10 deletions core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ public enum ExprCoreType implements ExprType {
*/
UNDEFINED,

/**
* Numbers.
*/
BYTE(UNDEFINED),
SHORT(BYTE),
INTEGER(SHORT),
LONG(INTEGER),
FLOAT(LONG),
DOUBLE(FLOAT),

/**
* String.
*/
Expand All @@ -51,16 +61,6 @@ public enum ExprCoreType implements ExprType {
DATETIME(STRING),
INTERVAL(UNDEFINED),

/**
* Numbers.
*/
BYTE(UNDEFINED),
SHORT(BYTE),
INTEGER(SHORT, DATE),
LONG(INTEGER),
FLOAT(LONG),
DOUBLE(FLOAT, DATETIME, TIME),

/**
* Struct.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.opensearch.sql.expression.function.FunctionDSL.impl;
import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling;

import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -106,9 +105,7 @@ private static FunctionResolver castToInt() {
impl(nullMissingHandling(
(v) -> new ExprIntegerValue(v.integerValue())), INTEGER, DOUBLE),
impl(nullMissingHandling(
(v) -> new ExprIntegerValue(v.booleanValue() ? 1 : 0)), INTEGER, BOOLEAN),
impl(nullMissingHandling(
(v) -> new ExprIntegerValue(Double.parseDouble(v.dateValue().format(DateTimeFormatter.ofPattern("uuuuMMdd"))))), INTEGER, DATE)
(v) -> new ExprIntegerValue(v.booleanValue() ? 1 : 0)), INTEGER, BOOLEAN)
);
}

Expand Down Expand Up @@ -141,11 +138,7 @@ private static FunctionResolver castToDouble() {
impl(nullMissingHandling(
(v) -> new ExprDoubleValue(v.doubleValue())), DOUBLE, DOUBLE),
impl(nullMissingHandling(
(v) -> new ExprDoubleValue(v.booleanValue() ? 1D : 0D)), DOUBLE, BOOLEAN),
impl(nullMissingHandling(
(v) -> new ExprDoubleValue(Double.parseDouble(v.datetimeValue().format(DateTimeFormatter.ofPattern("uuuuMMddHHmmss.nnnnnn"))))), DOUBLE, DATETIME),
impl(nullMissingHandling(
(v) -> new ExprDoubleValue(Double.parseDouble(v.timeValue().format(DateTimeFormatter.ofPattern("HHmmss.nnnnnn"))))), DOUBLE, TIME)
(v) -> new ExprDoubleValue(v.booleanValue() ? 1D : 0D)), DOUBLE, BOOLEAN)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,6 @@ public void now_like_functions(Function<Expression[], FunctionExpression> functi
function.apply(new Expression[]{DSL.literal(3)}),
AstDSL.function(name, intLiteral(3)));
}

assertAnalyzeEqual(
dsl.add(function.apply(new Expression[]{}), DSL.literal(0)),
AstDSL.function("+", AstDSL.function(name), intLiteral(0))
);
}

protected Expression analyze(UnresolvedExpression unresolvedExpression) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ private static Stream<Arguments> functionNames() {
var dsl = new DSL(new ExpressionConfig().functionRepository());
return Stream.of(
Arguments.of((Function<Expression[], FunctionExpression>)dsl::now,
"now", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now, "uuuuMMddHHmmss.nnnnnn"),
"now", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::current_timestamp,
"current_timestamp", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now, "uuuuMMddHHmmss.nnnnnn"),
"current_timestamp", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::localtimestamp,
"localtimestamp", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now, "uuuuMMddHHmmss.nnnnnn"),
"localtimestamp", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::localtime,
"localtime", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now, "uuuuMMddHHmmss.nnnnnn"),
"localtime", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::sysdate,
"sysdate", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now, "uuuuMMddHHmmss.nnnnnn"),
"sysdate", DATETIME, true, (Supplier<Temporal>)LocalDateTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::curtime,
"curtime", TIME, true, (Supplier<Temporal>)LocalTime::now, "HHmmss.nnnnnn"),
"curtime", TIME, true, (Supplier<Temporal>)LocalTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::current_time,
"current_time", TIME, true, (Supplier<Temporal>)LocalTime::now, "HHmmss.nnnnnn"),
"current_time", TIME, true, (Supplier<Temporal>)LocalTime::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::curdate,
"curdate", DATE, false, (Supplier<Temporal>)LocalDate::now, "uuuuMMdd"),
"curdate", DATE, false, (Supplier<Temporal>)LocalDate::now),
Arguments.of((Function<Expression[], FunctionExpression>)dsl::current_date,
"current_date", DATE, false, (Supplier<Temporal>)LocalDate::now, "uuuuMMdd"));
"current_date", DATE, false, (Supplier<Temporal>)LocalDate::now));
}

private ExprCoreType getCastRule(ExprCoreType from) {
Expand Down Expand Up @@ -92,8 +92,7 @@ public void the_test(Function<Expression[], FunctionExpression> function,
String name,
ExprCoreType resType,
Boolean hasFsp,
Supplier<Temporal> referenceGetter,
String formatterPattern) {
Supplier<Temporal> referenceGetter) {
// Check return types:
// `func()`
FunctionExpression expr = function.apply(new Expression[]{});
Expand All @@ -106,15 +105,9 @@ public void the_test(Function<Expression[], FunctionExpression> function,
expr = function.apply(new Expression[]{DSL.literal(6)});
assertEquals(resType, expr.type());
}
// Check return type of automatic cast:
// `func() + x`
expr = dsl.add(function.apply(new Expression[]{}), DSL.literal(0));
assertEquals(getCastRule(resType), expr.type());

// Check how calculations are precise:
// `func()`
var a = extractValue(function.apply(new Expression[]{}));
var b = referenceGetter.get();
assertTrue(Math.abs(getDiff(
extractValue(function.apply(new Expression[]{})),
referenceGetter.get()
Expand All @@ -126,13 +119,5 @@ public void the_test(Function<Expression[], FunctionExpression> function,
referenceGetter.get()
)) <= 1);
}
// `func() + x`
assertTrue(Math.abs(
dsl.add(
function.apply(new Expression[]{}),
DSL.literal(0)
).valueOf(null).doubleValue() -
Double.parseDouble(DateTimeFormatter.ofPattern(formatterPattern).format(referenceGetter.get()))
) <= 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,6 @@ class WideningTypeRuleTest {
.put(STRING, DATE, 1)
.put(STRING, TIME, 1)
.put(STRING, DATETIME, 1)
.put(STRING, INTEGER, 2)
.put(STRING, LONG, 3)
.put(STRING, FLOAT, 4)
.put(STRING, DOUBLE, 2)

.put(DATE, INTEGER, 1)
.put(DATE, LONG, 2)
.put(DATE, FLOAT, 3)
.put(DATE, DOUBLE, 4)

.put(DATETIME, DOUBLE, 1)
.put(TIME, DOUBLE, 1)

.put(UNDEFINED, BYTE, 1)
.put(UNDEFINED, SHORT, 2)
.put(UNDEFINED, INTEGER, 3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDateTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDateTime::parse)
.put("serializationPattern", "uuuu-MM-dd HH:mm:ss")
.put("castPattern", "uuuuMMddHHmmss")
.build(),
ImmutableMap.builder()
.put("name", "current_timestamp")
Expand All @@ -503,7 +502,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDateTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDateTime::parse)
.put("serializationPattern", "uuuu-MM-dd HH:mm:ss")
.put("castPattern", "uuuuMMddHHmmss")
.build(),
ImmutableMap.builder()
.put("name", "localtimestamp")
Expand All @@ -513,7 +511,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDateTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDateTime::parse)
.put("serializationPattern", "uuuu-MM-dd HH:mm:ss")
.put("castPattern", "uuuuMMddHHmmss")
.build(),
ImmutableMap.builder()
.put("name", "localtime")
Expand All @@ -523,7 +520,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDateTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDateTime::parse)
.put("serializationPattern", "uuuu-MM-dd HH:mm:ss")
.put("castPattern", "uuuuMMddHHmmss")
.build(),
ImmutableMap.builder()
.put("name", "sysdate")
Expand All @@ -533,7 +529,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDateTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDateTime::parse)
.put("serializationPattern", "uuuu-MM-dd HH:mm:ss")
.put("castPattern", "uuuuMMddHHmmss")
.build(),
ImmutableMap.builder()
.put("name", "curtime")
Expand All @@ -543,7 +538,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalTime::parse)
.put("serializationPattern", "HH:mm:ss")
.put("castPattern", "HHmmss")
.build(),
ImmutableMap.builder()
.put("name", "current_time")
Expand All @@ -553,7 +547,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalTime::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalTime::parse)
.put("serializationPattern", "HH:mm:ss")
.put("castPattern", "HHmmss")
.build(),
ImmutableMap.builder()
.put("name", "curdate")
Expand All @@ -563,7 +556,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDate::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDate::parse)
.put("serializationPattern", "uuuu-MM-dd")
.put("castPattern", "uuuuMMdd")
.build(),
ImmutableMap.builder()
.put("name", "current_date")
Expand All @@ -573,7 +565,6 @@ private List<ImmutableMap<Object, Object>> nowLikeFunctionsData() {
.put("referenceGetter", (Supplier<Temporal>) LocalDate::now)
.put("parser", (BiFunction<CharSequence, DateTimeFormatter, Temporal>) LocalDate::parse)
.put("serializationPattern", "uuuu-MM-dd")
.put("castPattern", "uuuuMMdd")
.build()
);
}
Expand Down Expand Up @@ -602,18 +593,13 @@ public void testNowLikeFunctions() throws IOException {
BiFunction<CharSequence, DateTimeFormatter, Temporal> parser =
(BiFunction<CharSequence, DateTimeFormatter, Temporal>) funcData.get("parser");
String serializationPatternStr = (String) funcData.get("serializationPattern");
String castPatternStr = (String) funcData.get("castPattern");

var serializationPattern = new DateTimeFormatterBuilder()
.appendPattern(serializationPatternStr)
.optionalStart()
.appendFraction(ChronoField.NANO_OF_SECOND, 0, 6, true)
.toFormatter();
var castPattern = new DateTimeFormatterBuilder()
.appendPattern(castPatternStr)
.optionalStart()
.appendFraction(ChronoField.NANO_OF_SECOND, 0, 6, true)
.toFormatter();

Temporal reference = referenceGetter.get();
double delta = 2d; // acceptable time diff, secs
if (reference instanceof LocalDate)
Expand All @@ -622,17 +608,16 @@ public void testNowLikeFunctions() throws IOException {

var calls = new ArrayList<String>() {{
add(name + "()");
add(name + "() + 0");
}};
if (hasShortcut)
calls.add(name);
if (hasFsp)
calls.add(name + "(0)");

// Column order is: func(), func() + 0, func, func(0)
// shortcut ^ fsp ^
// Column order is: func(), func, func(0)
// shortcut ^ fsp ^
// Query looks like:
// source=people2 | eval `now()`=now(), `now() + 0`=now() + 0 | fields `now()`, `now() + 0`;
// source=people2 | eval `now()`=now() | fields `now()`;
JSONObject result = executeQuery("source=" + TEST_INDEX_PEOPLE2
+ " | eval " + calls.stream().map(c -> String.format("`%s`=%s", c, c)).collect(Collectors.joining(","))
+ " | fields " + calls.stream().map(c -> String.format("`%s`", c)).collect(Collectors.joining(",")));
Expand All @@ -648,9 +633,6 @@ public void testNowLikeFunctions() throws IOException {
assertEquals(0,
getDiff(reference, parser.apply(row.getString(column++), serializationPattern)), delta);

assertEquals(0,
Double.parseDouble(castPattern.format(reference)) - row.getDouble(column++), delta);

if (hasShortcut) {
assertEquals(0,
getDiff(reference, parser.apply(row.getString(column++), serializationPattern)), delta);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void ifnullWithNullInputTest() {
verifySchema(response,
schema("IFNULL(null, firstname)", "IFNULL1", "keyword"),
schema("IFNULL(firstname, null)", "IFNULL2", "keyword"),
schema("IFNULL(null, null)", "IFNULL3", "keyword"));
schema("IFNULL(null, null)", "IFNULL3", "byte"));
verifyDataRows(response,
rows("Hattie", "Hattie", LITERAL_NULL.value()),
rows( "Elinor", "Elinor", LITERAL_NULL.value())
Expand Down
Loading

0 comments on commit 9f6b66c

Please sign in to comment.