Skip to content

Commit

Permalink
handle Nullables especially for orderby and groupby in queryspec
Browse files Browse the repository at this point in the history
  • Loading branch information
dobe committed Jan 23, 2015
1 parent 85cf289 commit c951efc
Show file tree
Hide file tree
Showing 11 changed files with 223 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,14 @@ public void onSuccess(@Nullable List<TaskResult> result) {
sendResponse(listener, buildSQLActionException(e));
return;
}
assert (jobId != null);
statsTables.jobFinished(jobId, null);
sendResponse(listener, response);
}

@Override
public void onFailure(@Nonnull Throwable t) {
logger.debug("Error processing SQLRequest", t);
if (jobId != null) {
statsTables.jobFinished(jobId, Exceptions.messageOf(t));
}
statsTables.jobFinished(jobId, Exceptions.messageOf(t));
sendResponse(listener, buildSQLActionException(t));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public AnalyzedStatement visitInsertFromSubquery(InsertFromSubquery node, Void c
new InsertFromSubQueryAnalyzedStatement(selectAnalyzedStatement, tableInfo);

// We forbid using limit/offset or order by until we've implemented ES paging support (aka 'scroll')
if (selectAnalyzedStatement.querySpec().isLimited() || selectAnalyzedStatement.querySpec().orderBy().isSorted()) {
if (selectAnalyzedStatement.querySpec().isLimited() || selectAnalyzedStatement.querySpec().orderBy() != null) {
throw new UnsupportedFeatureException("Using limit, offset or order by is not" +
"supported on insert using a sub-query");
}
Expand All @@ -84,7 +84,7 @@ private void validateMatchingColumns(InsertFromSubQueryAnalyzedStatement context
if (insertColumns.size() != sourceSymbols.size()) {
throw new IllegalArgumentException("Number of columns in insert statement and subquery differ");
}

for (int i = 0; i < sourceSymbols.size(); i++) {
Reference insertColumn = insertColumns.get(i);
DataType targetType = insertColumn.valueType();
Expand All @@ -96,10 +96,11 @@ private void validateMatchingColumns(InsertFromSubQueryAnalyzedStatement context
Function castFunction = new Function(
CastFunctionResolver.functionInfo(sourceType, targetType),
Arrays.asList(sourceColumn));
if (selectAnalyzedStatement.querySpec().groupBy()!=null) {
if (selectAnalyzedStatement.querySpec().groupBy() != null) {
replaceIfPresent(selectAnalyzedStatement.querySpec().groupBy(), sourceColumn, castFunction);
}
if (selectAnalyzedStatement.querySpec().orderBy().isSorted()) {
if (selectAnalyzedStatement.querySpec().orderBy() != null) {
//noinspection ConstantConditions
replaceIfPresent(selectAnalyzedStatement.querySpec().orderBy().orderBySymbols(), sourceColumn, castFunction);
}
sourceSymbols.set(i, castFunction);
Expand Down
22 changes: 17 additions & 5 deletions sql/src/main/java/io/crate/analyze/QuerySpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ public OrderBy orderBy() {
}

public QuerySpec orderBy(@Nullable OrderBy orderBy) {
this.orderBy = orderBy;
if (orderBy== null || !orderBy.isSorted()){
this.orderBy = null;
} else{
this.orderBy = orderBy;
}
return this;
}

Expand All @@ -123,10 +127,18 @@ public boolean isLimited() {
}

public void normalize(EvaluatingNormalizer normalizer) {
normalizer.normalizeInplace(groupBy);
orderBy.normalize(normalizer);
normalizer.normalizeInplace(outputs);
where = where.normalize(normalizer);
if (groupBy != null){
normalizer.normalizeInplace(groupBy);
}
if (orderBy != null){
orderBy.normalize(normalizer);
}
if (outputs != null){
normalizer.normalizeInplace(outputs);
}
if (where != null){
where = where.normalize(normalizer);
}
}

public boolean hasNoResult() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,12 @@ public Void visitAggregation(Aggregation symbol, AggregationSearcherContext cont
}
}

@Nullable
private OrderBy analyzeOrderBy(SelectAnalyzer.SelectAnalysis selectAnalysis, List<SortItem> orderBy) {
int size = orderBy.size();
if (size==0){
return null;
}
List<Symbol> symbols = new ArrayList<>(size);
boolean[] reverseFlags = new boolean[size];
Boolean[] nullsFirst = new Boolean[size];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.common.collect.Lists;
import io.crate.Constants;
import io.crate.analyze.AnalysisMetaData;
import io.crate.analyze.OrderBy;
import io.crate.analyze.SelectAnalyzedStatement;
import io.crate.analyze.WhereClause;
import io.crate.analyze.relations.AnalyzedRelation;
Expand Down Expand Up @@ -115,10 +116,14 @@ public AnalyzedRelation visitSelectAnalyzedStatement(SelectAnalyzedStatement sta

GroupByConsumer.validateGroupBySymbols(tableRelation, statement.querySpec().groupBy());
PlannerContextBuilder contextBuilder = new PlannerContextBuilder(2, tableRelation.resolve(statement.querySpec().groupBy()))
.output(tableRelation.resolve(statement.querySpec().outputs()))
.orderBy(tableRelation.resolveAndValidateOrderBy(statement.querySpec().orderBy())
.output(tableRelation.resolve(statement.querySpec().outputs())
);

OrderBy orderBy = statement.querySpec().orderBy();
if (orderBy != null){
contextBuilder.orderBy(tableRelation.resolveAndValidateOrderBy(orderBy));
}

Symbol havingClause = null;
if(statement.querySpec().having() != null){
havingClause = tableRelation.resolveHaving(statement.querySpec().having());
Expand Down Expand Up @@ -179,22 +184,26 @@ public AnalyzedRelation visitSelectAnalyzedStatement(SelectAnalyzedStatement sta
contextBuilder.getAndClearProjections());

List<Symbol> outputs;
List<Symbol> orderBy;
List<Symbol> orderBySymbols;
if (topNForReducer == null) {
orderBy = contextBuilder.orderBy();
orderBySymbols = contextBuilder.orderBy();
outputs = contextBuilder.outputs();
} else {
orderBy = contextBuilder.passThroughOrderBy();
orderBySymbols = contextBuilder.passThroughOrderBy();
outputs = contextBuilder.passThroughOutputs();
}
// mergeNode handler
TopNProjection topN = new TopNProjection(
firstNonNull(statement.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT),
statement.querySpec().offset(),
orderBy,
statement.querySpec().orderBy().reverseFlags(),
statement.querySpec().orderBy().nullsFirst()
);
int limit = firstNonNull(statement.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT);
TopNProjection topN;
if (orderBy == null){
topN = new TopNProjection(limit, statement.querySpec().offset());
} else {
topN = new TopNProjection(limit, statement.querySpec().offset(),
orderBySymbols,
orderBy.reverseFlags(),
orderBy.nullsFirst()
);
}
topN.outputs(outputs);
MergeNode localMergeNode = PlanNodeBuilder.localMerge(ImmutableList.<Projection>of(topN), mergeNode);

Expand Down Expand Up @@ -224,13 +233,17 @@ private TopNProjection getTopNForReducer(SelectAnalyzedStatement analysis,
PlannerContextBuilder contextBuilder,
List<Symbol> outputs) {
if (requireLimitOnReducer(analysis, contextBuilder.aggregationsWrappedInScalar)) {
TopNProjection topN = new TopNProjection(
firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT) + analysis.querySpec().offset(),
0,
contextBuilder.orderBy(),
analysis.querySpec().orderBy().reverseFlags(),
analysis.querySpec().orderBy().nullsFirst()
);
OrderBy orderBy = analysis.querySpec().orderBy();
int limit = firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT) + analysis.querySpec().offset();
TopNProjection topN;
if (orderBy==null){
topN = new TopNProjection(limit, 0);
} else {
topN = new TopNProjection(limit, 0,
contextBuilder.orderBy(),
orderBy.reverseFlags(),
orderBy.nullsFirst());
}
topN.outputs(outputs);
return topN;
}
Expand Down
39 changes: 27 additions & 12 deletions sql/src/main/java/io/crate/planner/consumer/ESGetConsumer.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
package io.crate.planner.consumer;

import io.crate.analyze.AnalysisMetaData;
import io.crate.analyze.OrderBy;
import io.crate.analyze.SelectAnalyzedStatement;
import io.crate.analyze.relations.AnalyzedRelation;
import io.crate.analyze.relations.AnalyzedRelationVisitor;
Expand Down Expand Up @@ -103,18 +104,32 @@ public PlannedAnalyzedRelation visitSelectAnalyzedStatement(SelectAnalyzedStatem
} else {
indexName = tableInfo.ident().name();
}
return new ESGetNode(
indexName,
tableRelation.resolve(statement.querySpec().outputs()),
whereClauseContext.ids(),
whereClauseContext.routingValues(),
tableRelation.resolveAndValidateOrderBy(statement.querySpec().orderBy()),
statement.querySpec().orderBy().reverseFlags(),
statement.querySpec().orderBy().nullsFirst(),
statement.querySpec().limit(),
statement.querySpec().offset(),
tableInfo.partitionedByColumns()
);
OrderBy orderBy = statement.querySpec().orderBy();
if (orderBy == null){
return new ESGetNode(
indexName,
tableRelation.resolve(statement.querySpec().outputs()),
whereClauseContext.ids(),
whereClauseContext.routingValues(),
null, null, null,
statement.querySpec().limit(),
statement.querySpec().offset(),
tableInfo.partitionedByColumns()
);
} else {
return new ESGetNode(
indexName,
tableRelation.resolve(statement.querySpec().outputs()),
whereClauseContext.ids(),
whereClauseContext.routingValues(),
tableRelation.resolveAndValidateOrderBy(statement.querySpec().orderBy()),
orderBy.reverseFlags(),
orderBy.nullsFirst(),
statement.querySpec().limit(),
statement.querySpec().offset(),
tableInfo.partitionedByColumns()
);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.crate.Constants;
import io.crate.analyze.AnalysisMetaData;
import io.crate.analyze.InsertFromSubQueryAnalyzedStatement;
import io.crate.analyze.OrderBy;
import io.crate.analyze.SelectAnalyzedStatement;
import io.crate.analyze.relations.AnalyzedRelation;
import io.crate.analyze.relations.AnalyzedRelationVisitor;
Expand Down Expand Up @@ -229,15 +230,20 @@ private static AnalyzedRelation distributedWriterGroupBy(SelectAnalyzedStatement
}

boolean topNDone = false;
OrderBy orderBy = analysis.querySpec().orderBy();
if (analysis.querySpec().isLimited()) {
topNDone = true;
TopNProjection topN = new TopNProjection(
firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT) + analysis.querySpec().offset(),
0,
tableRelation.resolveAndValidateOrderBy(analysis.querySpec().orderBy()),
analysis.querySpec().orderBy().reverseFlags(),
analysis.querySpec().orderBy().nullsFirst()
);
TopNProjection topN;
int limit = firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT) + analysis.querySpec().offset();
if (orderBy == null) {
topN = new TopNProjection(limit, 0);
} else {
topN = new TopNProjection(limit, 0,
tableRelation.resolveAndValidateOrderBy(analysis.querySpec().orderBy()),
orderBy.reverseFlags(),
orderBy.nullsFirst()
);
}
topN.outputs(contextBuilder.outputs());
contextBuilder.addProjection((topN));
} else {
Expand All @@ -249,22 +255,26 @@ private static AnalyzedRelation distributedWriterGroupBy(SelectAnalyzedStatement
// local merge on handler
if (analysis.querySpec().isLimited()) {
List<Symbol> outputs;
List<Symbol> orderBy;
List<Symbol> orderBySymbols;
if (topNDone) {
orderBy = contextBuilder.passThroughOrderBy();
orderBySymbols = contextBuilder.passThroughOrderBy();
outputs = contextBuilder.passThroughOutputs();
} else {
orderBy = contextBuilder.orderBy();
orderBySymbols = contextBuilder.orderBy();
outputs = contextBuilder.outputs();
}
// mergeNode handler
TopNProjection topN = new TopNProjection(
firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT),
analysis.querySpec().offset(),
orderBy,
analysis.querySpec().orderBy().reverseFlags(),
analysis.querySpec().orderBy().nullsFirst()
);
TopNProjection topN;
int limit = firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT);
if (orderBy == null) {
topN = new TopNProjection(limit, analysis.querySpec().offset());
} else {
topN = new TopNProjection(limit, analysis.querySpec().offset(),
orderBySymbols,
orderBy.reverseFlags(),
orderBy.nullsFirst()
);
}
topN.outputs(outputs);
contextBuilder.addProjection(topN);
contextBuilder.addProjection(writerProjection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import io.crate.Constants;
import io.crate.analyze.AnalysisMetaData;
import io.crate.analyze.OrderBy;
import io.crate.analyze.SelectAnalyzedStatement;
import io.crate.analyze.WhereClause;
import io.crate.analyze.relations.AnalyzedRelation;
Expand Down Expand Up @@ -179,13 +180,18 @@ public static AnalyzedRelation nonDistributedGroupBy(SelectAnalyzedStatement ana
contextBuilder.addProjection(fp);
}
if (!ignoreSorting) {
TopNProjection topN = new TopNProjection(
firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT),
analysis.querySpec().offset(),
contextBuilder.orderBy(),
analysis.querySpec().orderBy().reverseFlags(),
analysis.querySpec().orderBy().nullsFirst()
);
OrderBy orderBy = analysis.querySpec().orderBy();
int limit = firstNonNull(analysis.querySpec().limit(), Constants.DEFAULT_SELECT_LIMIT);
TopNProjection topN;
if (orderBy == null){
topN = new TopNProjection(limit, analysis.querySpec().offset());

} else {
topN = new TopNProjection(limit, analysis.querySpec().offset(),
contextBuilder.orderBy(),
orderBy.reverseFlags(),
orderBy.nullsFirst());
}
topN.outputs(contextBuilder.outputs());
contextBuilder.addProjection(topN);
}
Expand Down
Loading

0 comments on commit c951efc

Please sign in to comment.