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 all 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
268 changes: 108 additions & 160 deletions core/src/main/java/io/questdb/cairo/ColumnType.java

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions core/src/main/java/io/questdb/cairo/GeoHashes.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public static void appendBinary(long hash, int bits, CharSinkBase<?> sink) {
public static void appendBinaryStringUnsafe(long hash, int bits, CharSinkBase<?> sink) {
// Below assertion can happen if there is corrupt metadata
// which should not happen in production code since reader and writer check table metadata
assert bits > 0 && bits <= ColumnType.GEO_HASH_MAX_BITS_LENGTH;
assert bits > 0 && bits <= ColumnType.GEOLONG_MAX_BITS;
for (int i = bits - 1; i >= 0; --i) {
sink.putAscii(((hash >> i) & 1) == 1 ? '1' : '0');
}
Expand Down Expand Up @@ -142,15 +142,15 @@ public static byte encodeChar(char c) {
}

public static long fromBitString(CharSequence bits, int start) throws NumericException {
return fromBitString(bits, start, Math.min(bits.length(), ColumnType.GEO_HASH_MAX_BITS_LENGTH + start));
return fromBitString(bits, start, Math.min(bits.length(), ColumnType.GEOLONG_MAX_BITS + start));
}

public static long fromBitStringNl(CharSequence bits, int start) throws NumericException {
int len = bits.length();
if (len - start <= 0) {
return NULL;
}
return fromBitString(bits, start, Math.min(bits.length(), ColumnType.GEO_HASH_MAX_BITS_LENGTH + start));
return fromBitString(bits, start, Math.min(bits.length(), ColumnType.GEOLONG_MAX_BITS + start));
}

public static long fromCoordinatesDeg(double lat, double lon, int bits) throws NumericException {
Expand All @@ -160,7 +160,7 @@ public static long fromCoordinatesDeg(double lat, double lon, int bits) throws N
if (lon < -180.0 || lon > 180.0) {
throw NumericException.INSTANCE;
}
if (bits < 0 || bits > ColumnType.GEO_HASH_MAX_BITS_LENGTH) {
if (bits < 0 || bits > ColumnType.GEOLONG_MAX_BITS) {
throw NumericException.INSTANCE;
}
return fromCoordinatesDegUnsafe(lat, lon, bits);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void write(TableWriter.Row row, int column, DirectUtf8Sequence value) {
}

static {
for (int b = 1; b <= ColumnType.GEO_HASH_MAX_BITS_LENGTH; b++) {
for (int b = 1; b <= ColumnType.GEOLONG_MAX_BITS; b++) {
int type = ColumnType.getGeoHashTypeWithBits(b);
typeToAdapterMap.put(type, new GeoHashAdapter(type));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static int extractGeoHashSuffix(int position, CharSequence tok) throws Sq
if (tokLen > 1) {
if (tokLen >= 3 && tok.charAt(tokLen - 3) == '/') { // '/dd'
short bits = (short) (10 * tok.charAt(tokLen - 2) + tok.charAt(tokLen - 1) - 528); // 10 * 48 + 48
if (bits >= 1 && bits <= ColumnType.GEO_HASH_MAX_BITS_LENGTH) {
if (bits >= 1 && bits <= ColumnType.GEOLONG_MAX_BITS) {
return Numbers.encodeLowHighShorts((short) 3, bits);
}
throw SqlException.$(position, "invalid bits size for GEOHASH constant: ").put(tok);
Expand Down
121 changes: 60 additions & 61 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 @@ -274,7 +274,7 @@ public void visit(ExpressionNode node) throws SqlException {
if (argCount == 0) {
switch (node.type) {
case ExpressionNode.LITERAL:
functionStack.push(createColumn(node.position, node.token));
functionStack.push(createColumn(node.position, node.token, metadata));
break;
case ExpressionNode.BIND_VARIABLE:
functionStack.push(createBindVariable0(node.position, node.token));
Expand Down Expand Up @@ -374,6 +374,14 @@ private static SqlException invalidFunction(ExpressionNode node, ObjList<Functio
return ex;
}

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

private Function checkAndCreateFunction(
FunctionFactory factory,
@Transient ObjList<Function> args,
Expand All @@ -396,32 +404,23 @@ private Function checkAndCreateFunction(
}

if (function == null) {
LOG.error().$("NULL function").$(" [signature=").$(factory.getSignature()).$(",class=").$(factory.getClass().getName()).$(']').$();
LOG.error().$("NULL function")
.$(" [signature=").$(factory.getSignature())
.$(", class=").$(factory.getClass().getName()).$(']')
.$();
Misc.freeObjList(args);
throw SqlException.position(position).put("bad function factory (NULL), check log");
}
return function;
}

private long convertToTimestamp(CharSequence str, int position) throws SqlException {
try {
return IntervalUtils.parseFloorPartialTimestamp(str);
} catch (NumericException e) {
throw SqlException.invalidDate(position);
}
}

private Function createBindVariable0(int position, CharSequence name) throws SqlException {
if (name.charAt(0) != ':') {
return parseIndexedParameter(position, name);
}
return createNamedParameter(position, name);
}

private Function createColumn(int position, CharSequence columnName) throws SqlException {
return createColumn(position, columnName, metadata);
}

private Function createConstant(int position, final CharSequence tok) throws SqlException {
final int len = tok.length();

Expand All @@ -430,21 +429,18 @@ private Function createConstant(int position, final CharSequence tok) throws Sql
}

if (Chars.isQuoted(tok)) {
if (len == 3) {
// this is 'x' - char
return CharConstant.newInstance(tok.charAt(1));
}

if (len == 2) {
// empty
return StrConstant.EMPTY;
switch (len) {
case 3: // this is 'x' - char
return CharConstant.newInstance(tok.charAt(1));
case 2: // this is '' - char
return StrConstant.EMPTY;
default:
return new StrConstant(tok);
}
return new StrConstant(tok);
}

// special case E'str'
// we treat it like normal string for now
if (len > 2 && tok.charAt(0) == 'E' && tok.charAt(1) == '\'') {
// special case E'str' - we treat it like normal string for now
if (len > 2 && tok.charAt(0) == 'E' && tok.charAt(1) == '\'' && tok.charAt(len - 1) == '\'') {
return new StrConstant(Chars.toString(tok, 2, len - 1));
}

Expand Down Expand Up @@ -544,7 +540,7 @@ private Function createFunction(
FunctionFactoryDescriptor candidateDescriptor = null;
boolean candidateSigVarArgConst = false;
int candidateSigArgCount = 0;
int candidateSigArgTypeSum = -1;
int candidateSigArgTypeScore = -1;
int bestMatch = MATCH_NO_MATCH;

undefinedVariables.clear();
Expand All @@ -565,7 +561,6 @@ private Function createFunction(
final boolean sigVarArg;
final boolean sigVarArgConst;


if (sigArgCount > 0) {
final int lastSigArgMask = descriptor.getArgTypeMask(sigArgCount - 1);
sigVarArg = FunctionFactoryDescriptor.toType(lastSigArgMask) == ColumnType.VAR_ARG;
Expand All @@ -579,25 +574,22 @@ private Function createFunction(
sigArgCount--;
}

// this is no-arg function, match right away
if (argCount == 0 && sigArgCount == 0) {
// this is no-arg function, match right away
return checkAndCreateFunction(factory, args, argPositions, node, configuration);
}

// otherwise, is number of arguments the same?
if (candidateDescriptor == null) {
candidateDescriptor = descriptor;
}
if (sigArgCount == argCount || (sigVarArg && argCount >= sigArgCount)) {
int match = MATCH_NO_MATCH; // no match
if (sigArgCount == 0) {
match = MATCH_EXACT_MATCH;
}

int sigArgTypeSum = 0;
for (int k = 0; k < sigArgCount; k++) {
final Function arg = args.getQuick(k);
final int sigArgTypeMask = descriptor.getArgTypeMask(k);
// otherwise, is number of arguments the same?
if (sigArgCount == argCount || (sigVarArg && argCount >= sigArgCount)) {
int match = sigArgCount == 0 ? MATCH_EXACT_MATCH : MATCH_NO_MATCH;
int sigArgTypeScore = 0;
for (int argIdx = 0; argIdx < sigArgCount; argIdx++) {
final Function arg = args.getQuick(argIdx);
final int sigArgTypeMask = descriptor.getArgTypeMask(argIdx);

if (FunctionFactoryDescriptor.isConstant(sigArgTypeMask) && !arg.isConstant()) {
match = MATCH_NO_MATCH; // no match
Expand Down Expand Up @@ -639,28 +631,28 @@ private Function createFunction(
boolean overloadPossible = false;
// we do not want to use any overload when checking the output of a cast() function.
// the output must be the exact type as specified by a user. that's the whole point of casting.
// for all other functions else we want to explore possible casting opportunities
// for all other functions, else, we want to explore possible casting opportunities
//
// output of a cast() function is always the 2nd argument in a function signature
if (k != 1 || !Chars.equals("cast", node.token)) {
if (argIdx != 1 || !Chars.equals("cast", node.token)) {
int overloadDistance = ColumnType.overloadDistance(argTypeTag, sigArgType); // NULL to any is 0

if (argTypeTag == ColumnType.STRING &&
sigArgTypeTag == ColumnType.CHAR) {
if (arg.isConstant()) {
// string longer than 1 char can't be cast to char implicitly
if (arg.getStrLen(null) > 1) {
overloadDistance = ColumnType.NO_OVERLOAD;
overloadDistance = ColumnType.OVERLOAD_NONE;
}
} else {
// prefer CHAR -> STRING to STRING -> CHAR conversion
overloadDistance = 2 * overloadDistance;
}
}

sigArgTypeSum += overloadDistance;
sigArgTypeScore += overloadDistance;
// Overload with cast to higher precision
overloadPossible = overloadDistance != ColumnType.NO_OVERLOAD;
overloadPossible = overloadDistance != ColumnType.OVERLOAD_NONE;

// Overload when arg is double NaN to func which accepts INT, LONG
overloadPossible |= argTypeTag == ColumnType.DOUBLE &&
Expand All @@ -676,6 +668,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 @@ -721,12 +717,12 @@ private Function createFunction(


if (match != MATCH_EXACT_MATCH) {
if (candidateSigArgTypeSum > sigArgTypeSum || bestMatch < match) {
if (candidateSigArgTypeScore > sigArgTypeScore || bestMatch < match) {
candidate = factory;
candidateDescriptor = descriptor;
candidateSigArgCount = sigArgCount;
candidateSigVarArgConst = sigVarArgConst;
candidateSigArgTypeSum = sigArgTypeSum;
candidateSigArgTypeScore = sigArgTypeScore;
}
bestMatch = match;
} else {
Expand Down Expand Up @@ -779,20 +775,15 @@ private Function createFunction(
} else if (sigArgTypeTag == ColumnType.INT) {
args.setQuick(k, IntConstant.NULL);
}
} else if ((argTypeTag == ColumnType.STRING || argTypeTag == ColumnType.SYMBOL) && sigArgTypeTag == ColumnType.TIMESTAMP) {
// convert arguments if necessary
int position = argPositions.getQuick(k);
if (arg.isConstant()) {
long timestamp = convertToTimestamp(arg.getStr(null), position);
} else if ((argTypeTag == ColumnType.STRING || argTypeTag == ColumnType.SYMBOL) && arg.isConstant()) {
if (sigArgTypeTag == ColumnType.TIMESTAMP) {
int position = argPositions.getQuick(k);
long timestamp = parseTimestamp(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);
} else if (sigArgTypeTag == ColumnType.DATE) {
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));
Expand Down Expand Up @@ -952,7 +943,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 Expand Up @@ -1010,6 +1001,14 @@ private Function parseIndexedParameter(int position, CharSequence name) throws S
}
}

private long parseTimestamp(CharSequence str, int position) throws SqlException {
try {
return IntervalUtils.parseFloorPartialTimestamp(str);
} catch (NumericException e) {
throw SqlException.invalidDate(str, position);
}
}

static {
for (int i = 0, n = SqlCompilerImpl.sqlControlSymbols.size(); i < n; i++) {
FunctionFactoryCache.invalidFunctionNames.add(SqlCompilerImpl.sqlControlSymbols.getQuick(i));
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/questdb/griffin/GeoHashUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static int parseGeoHashBits(int position, int start, CharSequence sizeStr
throw SqlException.position(position)
.put("invalid GEOHASH size units, must be 'c', 'C' for chars, or 'b', 'B' for bits");
}
if (size < 1 || size > ColumnType.GEO_HASH_MAX_BITS_LENGTH) {
if (size < 1 || size > ColumnType.GEOLONG_MAX_BITS) {
throw SqlException.position(position)
.put("invalid GEOHASH type precision range, must be [1, 60] bits, provided=")
.put(size);
Expand All @@ -85,7 +85,7 @@ public static ConstantFunction parseGeoHashConstant(int position, final CharSequ
// geohash from binary constant
// minus leading '##', truncates tail bits if over 60
int bits = len - 2;
if (bits <= ColumnType.GEO_HASH_MAX_BITS_LENGTH) {
if (bits <= ColumnType.GEOLONG_MAX_BITS) {
return Constants.getGeoHashConstant(
GeoHashes.fromBitStringNl(tok, 2),
bits
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/io/questdb/griffin/SqlException.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ public static SqlException invalidColumn(int position, CharSequence column) {
return position(position).put("Invalid column: ").put(column);
}

public static SqlException invalidDate(CharSequence str, int position) {
return position(position).put("Invalid date [str=").put(str).put(']');
}

public static SqlException invalidDate(int position) {
return position(position).put("Invalid date");
}
Expand Down