Skip to content

Commit c8f236e

Browse files
puzpuzpuzpuzpuzpuz
authored
fix(sql): fix SELECT DISTINCT queries with table name prefixed columns (#6061)
1 parent 4320881 commit c8f236e

7 files changed

Lines changed: 1092 additions & 70 deletions

File tree

‎core/src/main/java/io/questdb/griffin/SqlOptimiser.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4337,20 +4337,20 @@ private QueryModel rewriteDistinct(QueryModel model) throws SqlException {
43374337
if (model.isDistinct()) {
43384338
// bingo
43394339
// create wrapper models
4340-
QueryModel wrapperNested = queryModelPool.next();
4340+
final QueryModel wrapperNested = queryModelPool.next();
43414341
wrapperNested.setNestedModel(model);
4342-
QueryModel wrapperModel = queryModelPool.next();
4342+
final QueryModel wrapperModel = queryModelPool.next();
43434343
wrapperModel.setNestedModel(wrapperNested);
43444344

4345-
ObjList<QueryColumn> bottomUpColumns = model.getBottomUpColumns();
4345+
final ObjList<QueryColumn> bottomUpColumns = model.getBottomUpColumns();
43464346
// Columns might be aliased, if they are, we have to create a new
43474347
// column instance. We also check if we should abandon the rewrite
4348-
// in case we find counterproductive
4348+
// in case we find it counterproductive
43494349
boolean abandonRewrite = false;
43504350
for (int i = 0, n = bottomUpColumns.size(); i < n; i++) {
4351-
QueryColumn qc = bottomUpColumns.getQuick(i);
4352-
ExpressionNode ast = qc.getAst();
4353-
CharSequence alias = qc.getAlias();
4351+
final QueryColumn qc = bottomUpColumns.getQuick(i);
4352+
final ExpressionNode ast = qc.getAst();
4353+
final CharSequence alias = qc.getAlias();
43544354
if (qc.isWindowColumn() || (ast.type == FUNCTION && functionParser.getFunctionFactoryCache().isGroupBy(ast.token))) {
43554355
abandonRewrite = true;
43564356
break;
@@ -4370,12 +4370,12 @@ private QueryModel rewriteDistinct(QueryModel model) throws SqlException {
43704370
// simple group by with an extra column, that wasn't requested. But,
43714371
// it is a good start.
43724372

4373-
ExpressionNode countAst = expressionNodePool.next();
4373+
final ExpressionNode countAst = expressionNodePool.next();
43744374
countAst.token = "count";
43754375
countAst.paramCount = 0;
43764376
countAst.type = FUNCTION;
43774377

4378-
QueryColumn countColumn = queryColumnPool.next();
4378+
final QueryColumn countColumn = queryColumnPool.next();
43794379
countColumn.of(
43804380
createColumnAlias("count", model),
43814381
countAst,
@@ -6721,7 +6721,7 @@ private CharSequence validateColumnAndGetAlias(QueryModel model, CharSequence co
67216721
/**
67226722
* Validates column reference alias (e.g. that the literal is a reference to an existing column or alias). When
67236723
* the reference is valid, the method returns index of the table where reference is pointing. That is index of
6724-
* table in the join clause. If the literal refences the current projection, the table index is -1.
6724+
* table in the join clause. If the literal references the current projection, the table index is -1.
67256725
* <p>
67266726
* The validation order: the innerVirtualModel represent the current projection. If the referenced column is found
67276727
* there first, this is where the search stops. The innerVirtualModel is allowed to be null, in which case the
@@ -6739,7 +6739,7 @@ private CharSequence validateColumnAndGetAlias(QueryModel model, CharSequence co
67396739
* reference to the base model.
67406740
* @param position literal position in the SQL text to aid SQL error reporting
67416741
* @param groupByCall flag indicating that the return value is required, which is in turn asking to ignore projection lookup.
6742-
* This lookup will be unhandled when the return value is needed to disambigute the column name.
6742+
* This lookup will be unhandled when the return value is needed to disambiguate the column name.
67436743
* @return -1 if literal was matched to the projection, otherwise 0-based index of the join model where it was matched.
67446744
* @throws SqlException exception is throw when literal could not be matched, or it matches several models at the same time (ambiguous).
67456745
*/
@@ -6751,11 +6751,11 @@ private int validateColumnAndGetModelIndex(
67516751
int position,
67526752
boolean groupByCall
67536753
) throws SqlException {
6754-
ObjList<QueryModel> joinModels = baseModel.getJoinModels();
6754+
final ObjList<QueryModel> joinModels = baseModel.getJoinModels();
67556755
int index = -1;
67566756
if (dot == -1) {
67576757
if (innerVirtualModel != null && innerVirtualModel.getAliasToColumnMap().contains(literal) && !groupByCall) {
6758-
// Ror now, most places ignore the return values, except one - adding missing table prefixes in group-by
6758+
// For now, most places ignore the return values, except one - adding missing table prefixes in group-by
67596759
// cases. We do not yet support projection reference in group-by. When we do, we will need to deal with
67606760
// -1 there.
67616761
return -1;

‎core/src/main/java/io/questdb/griffin/SqlUtil.java‎

Lines changed: 34 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@
3636
import io.questdb.griffin.model.IntervalUtils;
3737
import io.questdb.griffin.model.QueryColumn;
3838
import io.questdb.griffin.model.QueryModel;
39-
import io.questdb.std.CharSequenceHashSet;
4039
import io.questdb.std.Chars;
4140
import io.questdb.std.GenericLexer;
4241
import io.questdb.std.Long256;
4342
import io.questdb.std.Long256Acceptor;
4443
import io.questdb.std.Long256FromCharSequenceDecoder;
4544
import io.questdb.std.Long256Impl;
45+
import io.questdb.std.LowerCaseCharSequenceHashSet;
4646
import io.questdb.std.LowerCaseCharSequenceObjHashMap;
4747
import io.questdb.std.Numbers;
4848
import io.questdb.std.NumericException;
@@ -65,7 +65,7 @@
6565

6666
public class SqlUtil {
6767

68-
static final CharSequenceHashSet disallowedAliases = new CharSequenceHashSet();
68+
static final LowerCaseCharSequenceHashSet disallowedAliases = new LowerCaseCharSequenceHashSet();
6969
private static final DateFormat[] DATE_FORMATS_FOR_TIMESTAMP;
7070
private static final int DATE_FORMATS_FOR_TIMESTAMP_SIZE;
7171
private static final ThreadLocal<Long256ConstantFactory> LONG256_FACTORY = new ThreadLocal<>(Long256ConstantFactory::new);
@@ -96,23 +96,28 @@ public static CharSequence createExprColumnAlias(
9696
boolean nonLiteral
9797
) {
9898
// We need to wrap disallowed aliases with double quotes to avoid later conflicts.
99-
final boolean quote = nonLiteral && !Chars.isDoubleQuoted(base) && (
100-
Chars.indexOf(base, '.') > -1 || disallowedAliases.contains(base)
101-
);
99+
final int baseLen = base.length();
100+
final int indexOfDot = Chars.indexOfLastUnquoted(base, '.');
101+
final boolean prefixedLiteral = !nonLiteral && indexOfDot > -1 && indexOfDot < baseLen - 1;
102+
boolean quote = nonLiteral
103+
? !Chars.isDoubleQuoted(base) && (indexOfDot > -1 || disallowedAliases.contains(base))
104+
: indexOfDot > -1 && disallowedAliases.contains(base, indexOfDot + 1, base.length());
102105

103-
int len = base.length();
104106
// early exit for simple cases
105-
if (!quote && aliasToColumnMap.excludes(base) && len > 0 && len <= maxLength && base.charAt(len - 1) != ' ') {
107+
if (!prefixedLiteral && !quote && aliasToColumnMap.excludes(base)
108+
&& baseLen > 0 && baseLen <= maxLength && base.charAt(baseLen - 1) != ' ') {
106109
return base;
107110
}
108111

112+
final int start = prefixedLiteral ? indexOfDot + 1 : 0;
113+
int len = baseLen - start;
109114
final CharacterStoreEntry entry = store.newEntry();
110115
final int entryLen = entry.length();
111116
if (quote) {
112117
entry.put('"');
113118
len += 2;
114119
}
115-
entry.put(base);
120+
entry.put(base, start, baseLen);
116121

117122
int sequence = 1;
118123
int seqSize = 0;
@@ -123,8 +128,8 @@ public static CharSequence createExprColumnAlias(
123128
len = Math.min(len, maxLength - seqSize - (quote ? 1 : 0));
124129

125130
// We don't want the alias to finish with a space.
126-
if (!quote && len > 0 && base.charAt(len - 1) == ' ') {
127-
final int lastSpace = Chars.lastIndexOfDifferent(base, 0, len, ' ');
131+
if (!quote && len > 0 && base.charAt(start + len - 1) == ' ') {
132+
final int lastSpace = Chars.lastIndexOfDifferent(base, start, start + len, ' ') - start;
128133
if (lastSpace > 0) {
129134
len = lastSpace + 1;
130135
}
@@ -153,22 +158,8 @@ public static long dateToTimestamp(long millis) {
153158
}
154159

155160
public static long expectMicros(CharSequence tok, int position) throws SqlException {
156-
int k = -1;
157-
158161
final int len = tok.length();
159-
160-
// look for end of digits
161-
for (int i = 0; i < len; i++) {
162-
char c = tok.charAt(i);
163-
if (c < '0' || c > '9') {
164-
k = i;
165-
break;
166-
}
167-
}
168-
169-
if (k == -1) {
170-
throw SqlException.$(position + len, "expected interval qualifier in ").put(tok);
171-
}
162+
final int k = findEndOfDigitsPos(tok, len, position);
172163

173164
try {
174165
long interval = Numbers.parseLong(tok, 0, k);
@@ -223,22 +214,8 @@ public static long expectMicros(CharSequence tok, int position) throws SqlExcept
223214
}
224215

225216
public static long expectSeconds(CharSequence tok, int position) throws SqlException {
226-
int k = -1;
227-
228217
final int len = tok.length();
229-
230-
// look for end of digits
231-
for (int i = 0; i < len; i++) {
232-
char c = tok.charAt(i);
233-
if (c < '0' || c > '9') {
234-
k = i;
235-
break;
236-
}
237-
}
238-
239-
if (k == -1) {
240-
throw SqlException.$(position + len, "expected interval qualifier in ").put(tok);
241-
}
218+
final int k = findEndOfDigitsPos(tok, len, position);
242219

243220
try {
244221
long interval = Numbers.parseLong(tok, 0, k);
@@ -1075,6 +1052,23 @@ public static short toPersistedTypeTag(@NotNull CharSequence tok, int tokPositio
10751052
throw SqlException.$(tokPosition, "non-persisted type: ").put(tok);
10761053
}
10771054

1055+
private static int findEndOfDigitsPos(CharSequence tok, int tokLen, int tokPosition) throws SqlException {
1056+
int k = -1;
1057+
// look for end of digits
1058+
for (int i = 0; i < tokLen; i++) {
1059+
char c = tok.charAt(i);
1060+
if (c < '0' || c > '9') {
1061+
k = i;
1062+
break;
1063+
}
1064+
}
1065+
1066+
if (k == -1) {
1067+
throw SqlException.$(tokPosition + tokLen, "expected interval qualifier in ").put(tok);
1068+
}
1069+
return k;
1070+
}
1071+
10781072
private static long implicitCastStrVarcharAsDate0(CharSequence value, int columnType) {
10791073
assert columnType == ColumnType.VARCHAR || columnType == ColumnType.STRING;
10801074
try {

‎core/src/main/java/io/questdb/std/LowerCaseCharSequenceHashSet.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ public boolean contains(CharSequence key) {
6767
return keyIndex(key) < 0;
6868
}
6969

70+
public boolean contains(CharSequence key, int lo, int hi) {
71+
return keyIndex(key, lo, hi) < 0;
72+
}
73+
7074
@Override
7175
public boolean equals(Object o) {
7276
if (this == o) return true;

0 commit comments

Comments
 (0)