Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sql): fix comparing Date columns with date literals #3862

Merged
merged 38 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
fa7cd43
chore(sql): implement Date comparisons
jerrinot Oct 17, 2023
23ca87d
support dates without any explicit timezone
jerrinot Oct 18, 2023
b0f2c03
explicit and implicit String -> Date casting tries to use the same pa…
jerrinot Oct 18, 2023
ad5ed44
more tests
jerrinot Oct 18, 2023
9cd8356
more tests
jerrinot Oct 18, 2023
0c9ad67
more tests, also removed (dead?) paths in FunctionParser
jerrinot Oct 19, 2023
7861dd3
superfluous check removed
jerrinot Oct 19, 2023
f954113
test
jerrinot Oct 19, 2023
2835a07
feat(sql): add first and last boolean aggregation functions (#3873)
rexf Oct 20, 2023
91824cf
chore(core): growable DirectByteSink in native code via byte_sink.h (…
amunra Oct 20, 2023
ff20138
fix(core): fix intermittent writing failures (#3844)
jerrinot Oct 20, 2023
319ad4e
fix(core): fix write error appearing after symbol column drop (#3876)
ideoma Oct 20, 2023
3a81fd4
feat(sql): add first and last string aggregation functions (#3879)
rexf Oct 23, 2023
0a04dda
chore(core): fixed flaky DirectByteSinkTest (#3881)
amunra Oct 23, 2023
531d590
feat(core): add metrics around worker (#3872)
marregui Oct 23, 2023
bd82f44
chore(core): growable DirectByteSink in native code via byte_sink.h (…
marregui Oct 25, 2023
d9fabf4
chore(core): fixed flaky DirectByteSinkTest (#3881)
amunra Oct 23, 2023
aab69a3
feat(core): add metrics around worker (#3872)
marregui Oct 25, 2023
5bd3ffd
improve exception message
marregui Oct 24, 2023
df50ba9
fix tests
marregui Oct 24, 2023
f268df7
improve readability
marregui Oct 25, 2023
a570817
fix(sql): rename column from `name` to `table_name` in cursor functio…
marregui Oct 24, 2023
41fefc2
chore(core): service account cookie (#3866)
glasstiger Oct 25, 2023
ccfef4a
chore(core): growable DirectByteSink in native code via byte_sink.h (…
marregui Oct 25, 2023
88202b3
chore(core): fixed flaky DirectByteSinkTest (#3881)
amunra Oct 23, 2023
a287a9c
a few renames to improve clarity
marregui Oct 25, 2023
c11e0ef
more improvements
marregui Oct 25, 2023
242638f
chore(core): growable DirectByteSink in native code via byte_sink.h (…
marregui Oct 25, 2023
89d8ffc
chore(core): fixed flaky DirectByteSinkTest (#3881)
amunra Oct 23, 2023
8f64afa
Merge branch 'master' into jh_date_functions
marregui Oct 25, 2023
6e1f117
remove debris
marregui Oct 25, 2023
9ff8c32
Merge remote-tracking branch 'upstream/master' into jh_date_functions
jerrinot Oct 26, 2023
08a6963
experiment: avoid special-casing in FunctionParser
jerrinot Oct 26, 2023
4ac28f4
Revert "experiment: avoid special-casing in FunctionParser"
jerrinot Oct 27, 2023
1a904f5
Merge branch 'master' into jh_date_functions
jerrinot Oct 27, 2023
bf5694c
Merge branch 'master' into jh_date_functions
jerrinot Oct 27, 2023
0b6b8ad
Merge remote-tracking branch 'upstream/master' into jh_date_functions
jerrinot Oct 27, 2023
9797707
Merge branch 'master' into jh_date_functions
jerrinot Oct 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 24 additions & 15 deletions core/src/main/java/io/questdb/griffin/FunctionParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import io.questdb.cairo.ColumnType;
import io.questdb.cairo.ImplicitCastException;
import io.questdb.cairo.sql.*;
import io.questdb.griffin.engine.functions.AbstractUnaryTimestampFunction;
import io.questdb.griffin.engine.functions.CursorFunction;
import io.questdb.griffin.engine.functions.GroupByFunction;
import io.questdb.griffin.engine.functions.bind.IndexedParameterLinkFunction;
Expand All @@ -41,6 +40,7 @@
import io.questdb.log.Log;
import io.questdb.log.LogFactory;
import io.questdb.std.*;
import io.questdb.std.datetime.millitime.DateFormatUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -403,7 +403,7 @@ private Function checkAndCreateFunction(
return function;
}

private long convertToTimestamp(CharSequence str, int position) throws SqlException {
private long parseTimestamp(CharSequence str, int position) throws SqlException {
try {
return IntervalUtils.parseFloorPartialTimestamp(str);
} catch (NumericException e) {
Expand Down Expand Up @@ -676,6 +676,10 @@ private Function createFunction(
overloadPossible |= argTypeTag == ColumnType.STRING && arg.isConstant() &&
sigArgTypeTag == ColumnType.TIMESTAMP && !factory.isGroupBy();

// Implicit cast from STRING to DATE
overloadPossible |= argTypeTag == ColumnType.STRING && arg.isConstant() &&
sigArgTypeTag == ColumnType.DATE && !factory.isGroupBy();

// Implicit cast from STRING to GEOHASH
overloadPossible |= argTypeTag == ColumnType.STRING &&
sigArgTypeTag == ColumnType.GEOHASH && !factory.isGroupBy();
Expand Down Expand Up @@ -781,26 +785,31 @@ private Function createFunction(
}
} else if ((argTypeTag == ColumnType.STRING || argTypeTag == ColumnType.SYMBOL) && sigArgTypeTag == ColumnType.TIMESTAMP) {
// convert arguments if necessary
bziobrowski marked this conversation as resolved.
Show resolved Hide resolved
assert arg.isConstant(); // casting from String/Symbol to Timestamp is supported for constants only
int position = argPositions.getQuick(k);
if (arg.isConstant()) {
long timestamp = convertToTimestamp(arg.getStr(null), position);
args.set(k, TimestampConstant.newInstance(timestamp));
} else {
AbstractUnaryTimestampFunction castFn;
if (argTypeTag == ColumnType.STRING) {
castFn = new CastStrToTimestampFunctionFactory.Func(arg);
} else {
castFn = new CastSymbolToTimestampFunctionFactory.Func(arg);
}
args.setQuick(k, castFn);
}
long timestamp = parseTimestamp(arg.getStr(null), position);
args.set(k, TimestampConstant.newInstance(timestamp));
} else if ((argTypeTag == ColumnType.STRING || argTypeTag == ColumnType.SYMBOL) && sigArgTypeTag == ColumnType.DATE) {
// convert arguments if necessary
assert arg.isConstant(); // casting from String/Symbol to Date is supported for constants only
int position = argPositions.getQuick(k);
long millis = parseDate(arg.getStr(null), position);
args.set(k, DateConstant.newInstance(millis));
} else if (argTypeTag == ColumnType.UUID && sigArgTypeTag == ColumnType.STRING) {
args.setQuick(k, new CastUuidToStrFunctionFactory.Func(arg));
}
}
return checkAndCreateFunction(candidate, args, argPositions, node, configuration);
}

private static long parseDate(CharSequence str, int position) throws SqlException {
try {
return DateFormatUtils.parseDate(str);
} catch (NumericException e) {
throw SqlException.invalidDate(position);
}
}

@Nullable
private Function createImplicitCastOrNull(int position, Function function, int toType) throws SqlException {
int fromType = function.getType();
Expand Down Expand Up @@ -952,7 +961,7 @@ private Function functionToConstant0(Function function) {
if (function instanceof DateConstant) {
return function;
} else {
return DateConstant.getInstance(function.getDate(null));
return DateConstant.newInstance(function.getDate(null));
}
case ColumnType.STRING:
if (function instanceof StrConstant) {
Expand Down
29 changes: 4 additions & 25 deletions core/src/main/java/io/questdb/griffin/SqlUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@
public class SqlUtil {

static final CharSequenceHashSet disallowedAliases = new CharSequenceHashSet();
private static final DateFormat[] DATE_FORMATS;
private static final DateFormat[] DATE_FORMATS_FOR_TIMESTAMP;
private static final int DATE_FORMATS_FOR_TIMESTAMP_SIZE;
private static final int DATE_FORMATS_SIZE;
private static final ThreadLocal<Long256ConstantFactory> LONG256_FACTORY = new ThreadLocal<>(Long256ConstantFactory::new);

public static void addSelectStar(
Expand Down Expand Up @@ -451,21 +449,11 @@ public static char implicitCastStrAsChar(CharSequence value) {
}

public static long implicitCastStrAsDate(CharSequence value) {
if (value != null) {
final int hi = value.length();
for (int i = 0; i < DATE_FORMATS_SIZE; i++) {
try {
return DATE_FORMATS[i].parse(value, 0, hi, DateFormatUtils.enLocale);
} catch (NumericException ignore) {
}
}
try {
return Numbers.parseLong(value, 0, hi);
} catch (NumericException e) {
throw ImplicitCastException.inconvertibleValue(value, ColumnType.STRING, ColumnType.DATE);
}
try {
return DateFormatUtils.parseDate(value);
} catch (NumericException e) {
throw ImplicitCastException.inconvertibleValue(value, ColumnType.STRING, ColumnType.DATE);
}
return Numbers.LONG_NaN;
}

public static double implicitCastStrAsDouble(CharSequence value) {
Expand Down Expand Up @@ -714,15 +702,6 @@ Long256Constant pop() {
final DateFormatCompiler milliCompiler = new DateFormatCompiler();
final DateFormat pgDateTimeFormat = milliCompiler.compile("y-MM-dd HH:mm:ssz");

DATE_FORMATS = new DateFormat[]{
pgDateTimeFormat,
PG_DATE_Z_FORMAT,
PG_DATE_MILLI_TIME_Z_FORMAT,
UTC_FORMAT
};

DATE_FORMATS_SIZE = DATE_FORMATS.length;

// we are using "millis" compiler deliberately because clients encode millis into strings
DATE_FORMATS_FOR_TIMESTAMP = new DateFormat[]{
PG_DATE_Z_FORMAT,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*******************************************************************************
* ___ _ ____ ____
* / _ \ _ _ ___ ___| |_| _ \| __ )
* | | | | | | |/ _ \/ __| __| | | | _ \
* | |_| | |_| | __/\__ \ |_| |_| | |_) |
* \__\_\\__,_|\___||___/\__|____/|____/
*
* Copyright (c) 2014-2019 Appsicle
* Copyright (c) 2019-2023 QuestDB
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
******************************************************************************/

package io.questdb.griffin.engine.functions;

import io.questdb.cairo.sql.Function;

public abstract class AbstractUnaryDateFunction extends DateFunction implements UnaryFunction {
protected final Function arg;

public AbstractUnaryDateFunction(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,15 @@

package io.questdb.griffin.engine.functions.cast;

import io.questdb.cairo.sql.Function;
import io.questdb.griffin.PlanSink;
import io.questdb.griffin.engine.functions.DateFunction;
import io.questdb.griffin.engine.functions.UnaryFunction;
import io.questdb.griffin.engine.functions.AbstractUnaryDateFunction;

public abstract class AbstractCastToDateFunction extends DateFunction implements UnaryFunction {
public abstract class AbstractCastToDateFunction extends AbstractUnaryDateFunction {

public AbstractCastToDateFunction(Function arg) {
super(arg);
}

@Override
public void toPlan(PlanSink sink) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,8 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

private static class Func extends AbstractCastToDateFunction {
private final Function arg;

public Func(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,8 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

public static class CastByteToDateFunction extends AbstractCastToDateFunction {
private final Function arg;

public CastByteToDateFunction(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,8 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

public static class CastCharToDateFunction extends AbstractCastToDateFunction {
private final Function arg;

public CastCharToDateFunction(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,8 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

private static class Func extends AbstractCastToDateFunction {
private final Function arg;

public Func(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,9 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

private static class Func extends AbstractCastToDateFunction {
private final Function arg;

public Func(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,9 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

public static class CastIntToDateFunction extends AbstractCastToDateFunction {
private final Function arg;

public CastIntToDateFunction(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,8 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

public static class CastLongToDateFunction extends AbstractCastToDateFunction {
private final Function arg;

public CastLongToDateFunction(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,9 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
}

public static class CastShortToDateFunction extends AbstractCastToDateFunction {
private final Function arg;

public CastShortToDateFunction(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,17 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
return new Func(args.getQuick(0));
}

private static class Func extends AbstractCastToDateFunction {
private final Function arg;
public static class Func extends AbstractCastToDateFunction {

public Func(Function arg) {
this.arg = arg;
}

@Override
public Function getArg() {
return arg;
super(arg);
}

@Override
public long getDate(Record rec) {
final CharSequence value = arg.getStr(rec);
try {
return value == null ? Numbers.LONG_NaN : DateFormatUtils.parseUTCDate(value);
return value == null ? Numbers.LONG_NaN : DateFormatUtils.parseDate(value);
} catch (NumericException e) {
return Numbers.LONG_NaN;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,12 @@ public Function newInstance(int position, ObjList<Function> args, IntList argPos
return new Func(args.getQuick(0));
}

private static class Func extends AbstractCastToDateFunction {
private final Function arg;
public static class Func extends AbstractCastToDateFunction {

public Func(Function arg) {
this.arg = arg;
super(arg);
}

@Override
public Function getArg() {
return arg;
}

@Override
public long getDate(Record rec) {
Expand Down