Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Support for ORDER BY IS NULL/NOT NULL for non-aggregation queries (#206)
Browse files Browse the repository at this point in the history
* Enhance order by is null/not nulll for non-aggregation queries
  • Loading branch information
abbashus authored Oct 7, 2019
1 parent 5034571 commit d09e83a
Show file tree
Hide file tree
Showing 9 changed files with 313 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private Field makeFieldImpl(SQLExpr expr, String alias, String tableAlias) throw
if (expr instanceof SQLIdentifierExpr || expr instanceof SQLPropertyExpr || expr instanceof SQLVariantRefExpr) {
return handleIdentifier(expr, alias, tableAlias);
} else if (expr instanceof SQLQueryExpr) {
throw new SqlParseException("unknow field name : " + expr);
throw new SqlParseException("unknown field name : " + expr);
} else if (expr instanceof SQLBinaryOpExpr) {
//make a SCRIPT method field;
return makeFieldImpl(makeBinaryMethodField((SQLBinaryOpExpr) expr, alias, true), alias, tableAlias);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
import com.alibaba.druid.sql.ast.SQLExpr;
import com.alibaba.druid.sql.ast.SQLOrderBy;
import com.alibaba.druid.sql.ast.SQLOrderingSpecification;
import com.alibaba.druid.sql.ast.expr.SQLBinaryOpExpr;
import com.alibaba.druid.sql.ast.expr.SQLBinaryOperator;
import com.alibaba.druid.sql.ast.expr.SQLIdentifierExpr;
import com.alibaba.druid.sql.ast.expr.SQLListExpr;
import com.alibaba.druid.sql.ast.expr.SQLMethodInvokeExpr;
import com.alibaba.druid.sql.ast.expr.SQLNullExpr;
import com.alibaba.druid.sql.ast.expr.SQLPropertyExpr;
import com.alibaba.druid.sql.ast.expr.SQLQueryExpr;
import com.alibaba.druid.sql.ast.statement.SQLDeleteStatement;
Expand Down Expand Up @@ -243,7 +246,7 @@ private void addOrderByToSelect(Select select, MySqlSelectQueryBlock queryBlock,
sqlSelectOrderByItem.setType(SQLOrderingSpecification.ASC);
}
String type = sqlSelectOrderByItem.getType().toString();
SQLExpr expr = sqlSelectOrderByItem.getExpr();
SQLExpr expr = extractExprFromOrderExpr(sqlSelectOrderByItem);

if (expr instanceof SQLIdentifierExpr) {
if (queryBlock.getGroupBy() == null || queryBlock.getGroupBy().getItems().isEmpty()) {
Expand All @@ -255,6 +258,13 @@ private void addOrderByToSelect(Select select, MySqlSelectQueryBlock queryBlock,

Field field = fieldMaker.makeField(expr, null, null);

SQLExpr sqlExpr = sqlSelectOrderByItem.getExpr();
if (sqlExpr instanceof SQLBinaryOpExpr && hasNullOrderInBinaryOrderExpr(sqlExpr)) {
// override Field.expression to SQLBinaryOpExpr,
// which was set by FieldMaker.makeField() to SQLIdentifierExpr above
field.setExpression(sqlExpr);
}

String orderByName;
if (field.isScriptField()) {
MethodField methodField = (MethodField) field;
Expand All @@ -275,6 +285,44 @@ private void addOrderByToSelect(Select select, MySqlSelectQueryBlock queryBlock,
}
}

private SQLExpr extractExprFromOrderExpr(SQLSelectOrderByItem sqlSelectOrderByItem) {
SQLExpr expr = sqlSelectOrderByItem.getExpr();

// extract SQLIdentifier from Order IS NULL/NOT NULL expression to generate Field
// else passing SQLBinaryOpExpr to FieldMaker.makeFieldImpl tries to convert to SQLMethodInvokeExpr
// and throws SQLParserException
if (hasNullOrderInBinaryOrderExpr(expr)) {
return ((SQLBinaryOpExpr) expr).getLeft();
}
return expr;
}

private boolean hasNullOrderInBinaryOrderExpr(SQLExpr expr) {
/**
* Valid AST that meets ORDER BY IS NULL/NOT NULL condition (true)
*
* SQLSelectOrderByItem
* |
* SQLBinaryOpExpr (Is || IsNot)
* / \
* SQLIdentifierExpr SQLNullExpr
*/
if (!(expr instanceof SQLBinaryOpExpr)) {
return false;
}

// check "shape of expression": <identifier> <operator> <NULL>
SQLBinaryOpExpr binaryExpr = (SQLBinaryOpExpr) expr;
if (!(binaryExpr.getLeft() instanceof SQLIdentifierExpr)|| !(binaryExpr.getRight() instanceof SQLNullExpr)) {
return false;
}

// check that operator IS or IS NOT
SQLBinaryOperator operator = binaryExpr.getOperator();
return operator == SQLBinaryOperator.Is || operator == SQLBinaryOperator.IsNot;

}

private void findLimit(MySqlSelectQueryBlock.Limit limit, Select select) {

if (limit == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

package com.amazon.opendistroforelasticsearch.sql.query;

import com.alibaba.druid.sql.ast.SQLExpr;
import com.alibaba.druid.sql.ast.expr.SQLBinaryOpExpr;
import com.alibaba.druid.sql.ast.expr.SQLBinaryOperator;
import com.amazon.opendistroforelasticsearch.sql.domain.Field;
import com.amazon.opendistroforelasticsearch.sql.domain.KVValue;
import com.amazon.opendistroforelasticsearch.sql.domain.MethodField;
Expand All @@ -41,14 +44,17 @@
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.search.sort.FieldSortBuilder;
import org.elasticsearch.search.sort.NestedSortBuilder;
import org.elasticsearch.search.sort.ScoreSortBuilder;
import org.elasticsearch.search.sort.ScriptSortBuilder.ScriptSortType;
import org.elasticsearch.search.sort.SortBuilders;
import org.elasticsearch.search.sort.SortOrder;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
Expand Down Expand Up @@ -210,28 +216,53 @@ private void setWhere(Where where) throws SqlParseException {
* @param orderBys list of Order object
*/
private void setSorts(List<Order> orderBys) {
Map<String, FieldSortBuilder> sortBuilderMap = new HashMap<>();

for (Order order : orderBys) {
String orderByName = order.getName();
SortOrder sortOrder = SortOrder.valueOf(order.getType());

if (order.getNestedPath() != null) {
request.addSort(
SortBuilders.fieldSort(order.getName())
.order(SortOrder.valueOf(order.getType()))
SortBuilders.fieldSort(orderByName)
.order(sortOrder)
.setNestedSort(new NestedSortBuilder(order.getNestedPath())));
} else if (order.isScript()) {
// TODO: Investigate how to find the type of expression (string or number)
// As of now this shouldn't be a problem, because the support is for date_format function
request.addSort(
SortBuilders
.scriptSort(new Script(orderByName), getScriptSortType(order))
.order(sortOrder));
} else if (orderByName.equals(ScoreSortBuilder.NAME)) {
request.addSort(orderByName, sortOrder);
} else {
if (order.isScript()) {
// TODO: Investigate how to find the type of expression (string or number)
// As of now this shouldn't be a problem, because the support is for date_format function

request.addSort(
SortBuilders
.scriptSort(new Script(order.getName()), getScriptSortType(order))
.order(SortOrder.valueOf(order.getType())));
} else {
request.addSort(order.getName(), SortOrder.valueOf(order.getType()));
}
FieldSortBuilder fieldSortBuilder = sortBuilderMap.computeIfAbsent(orderByName, key -> {
FieldSortBuilder fs = SortBuilders.fieldSort(key);
request.addSort(fs);
return fs;
});
setSortParams(fieldSortBuilder, order);
}
}
}


private void setSortParams(FieldSortBuilder fieldSortBuilder, Order order) {
fieldSortBuilder.order(SortOrder.valueOf(order.getType()));

SQLExpr expr = order.getSortField().getExpression();
if (expr instanceof SQLBinaryOpExpr) {
// we set SQLBinaryOpExpr in Field.setExpression() to support ORDER by IS NULL/IS NOT NULL
fieldSortBuilder.missing(getNullOrderString((SQLBinaryOpExpr) expr));
}
}

private String getNullOrderString(SQLBinaryOpExpr expr) {
SQLBinaryOperator operator = expr.getOperator();
return operator == SQLBinaryOperator.IsNot ? "_first" : "_last";
}

private ScriptSortType getScriptSortType(Order order) {
ScriptSortType scriptSortType;
ScriptMethodField smf = (ScriptMethodField) order.getSortField();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Visitor to rewrite AST (abstract syntax tree) for supporting term_query in WHERE and IN condition
Expand Down Expand Up @@ -218,28 +219,34 @@ public boolean isValidIdentifierForTerm(SQLIdentifierExpr expr) {
*
* NOTE: Does not impact fields on ON condition clause in JOIN as we skip visiting SQLJoinTableSource
*/
return
!expr.getName().startsWith("_")
&& (isValidIdentifier(expr) || checkIfNestedIdentifier(expr));
return !expr.getName().startsWith("_") && (isValidIdentifier(expr) || checkIfNestedIdentifier(expr));
}

private boolean checkIfNestedIdentifier(SQLIdentifierExpr expr) {
return
expr.getParent() instanceof SQLMethodInvokeExpr
&& ((SQLMethodInvokeExpr) expr.getParent()).getMethodName().equals("nested")
&& isValidIdentifier(expr.getParent());
expr.getParent() instanceof SQLMethodInvokeExpr
&& ((SQLMethodInvokeExpr) expr.getParent()).getMethodName().equals("nested")
&& isValidIdentifier(expr.getParent());
}

private boolean isValidIdentifier(SQLObject expr) {
SQLObject parent = expr.getParent();
return
(parent instanceof SQLBinaryOpExpr
&& ((SQLBinaryOpExpr) parent).getOperator() == SQLBinaryOperator.Equality
)
|| parent instanceof SQLInListExpr
|| parent instanceof SQLInSubQueryExpr
|| parent instanceof SQLSelectOrderByItem
|| parent instanceof MySqlSelectGroupByExpr;
return isBinaryExprWithValidOperators(parent)
|| parent instanceof SQLInListExpr
|| parent instanceof SQLInSubQueryExpr
|| parent instanceof SQLSelectOrderByItem
|| parent instanceof MySqlSelectGroupByExpr;
}

private boolean isBinaryExprWithValidOperators(SQLObject expr) {
if (!(expr instanceof SQLBinaryOpExpr)) {
return false;
}
return Stream.of(
SQLBinaryOperator.Equality,
SQLBinaryOperator.Is,
SQLBinaryOperator.IsNot
).anyMatch(operator -> operator == ((SQLBinaryOpExpr) expr).getOperator());
}

private void checkMappingCompatibility(TermFieldScope scope, Map<String, String> indexToType) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/*
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file 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 com.amazon.opendistroforelasticsearch.sql.esintgtest;

import org.json.JSONArray;
import org.json.JSONObject;
import org.junit.Test;

import java.io.IOException;

import static org.hamcrest.Matchers.equalTo;

public class OrderIT extends SQLIntegTestCase {

@Override
protected void init() throws Exception {
loadIndex(Index.ORDER);
}

@Test
public void simpleOrder() throws IOException {
String query = "SELECT id, name FROM elasticsearch-sql_test_index_order ORDER BY id";
JSONArray result = getSortExplain(query);
assertThat(result.length(), equalTo(1));
JSONObject jsonObject = getSortByField(result, "id");
assertThat(query(result, "/0/id/order"), equalTo("asc"));
assertFalse(jsonObject.getJSONObject("id").has("missing"));
}

@Test
public void orderByScore() throws IOException {
String query = "SELECT * FROM elasticsearch-sql_test_index_order ORDER BY _score";
JSONArray result = getSortExplain(query);
assertThat(result.length(), equalTo(1));
JSONObject jsonObject = getSortByField(result, "_score");
assertThat(query(result, "/0/_score/order"), equalTo("asc"));
assertFalse(jsonObject.getJSONObject("_score").has("missing"));

JSONObject response = executeQuery(query);
JSONArray hits = getHits(response);
assertThat(hits.length(), equalTo(7));
}

@Test
public void simpleOrderMultipleFields() throws IOException {
String query = "SELECT id, name FROM elasticsearch-sql_test_index_order ORDER BY id, name";
JSONArray result = getSortExplain(query);
assertThat(result.length(), equalTo(2));
assertTrue(result.getJSONObject(0).has("id"));
assertTrue(result.getJSONObject(1).has("name.keyword"));
}

@Test
public void explicitOrderType() throws IOException {
String query = "SELECT id, name FROM elasticsearch-sql_test_index_order ORDER BY id ASC, name DESC";
JSONArray result = getSortExplain(query);
assertThat(result.length(), equalTo(2));
assertThat(query(result, "/0/id/order"), equalTo("asc"));
assertThat(query(result, "/1/name.keyword/order"), equalTo("desc"));
}

@Test
public void orderByIsNull() throws IOException {
String query = "SELECT * FROM elasticsearch-sql_test_index_order ORDER BY id IS NULL, id DESC";
JSONArray result = getSortExplain(query);
assertThat(result.length(), equalTo(1));
assertThat(query(result, "/0/id/order"), equalTo("desc"));
assertThat(query(result, "/0/id/missing"), equalTo("_last"));

JSONObject response = executeQuery(query);
JSONArray hits = getHits(response);
assertThat(query(hits, "/0/_source/id"), equalTo("5"));

// Another equivalent syntax
assertThat(explainQuery("SELECT * FROM elasticsearch-sql_test_index_order " +
"ORDER BY id IS NULL, id DESC"),
equalTo(explainQuery("SELECT * FROM elasticsearch-sql_test_index_order " +
"ORDER BY id IS NULL DESC"))
);
}

@Test
public void orderByIsNotNull() throws IOException {
String query = "SELECT id, name FROM elasticsearch-sql_test_index_order ORDER BY name IS NOT NULL";
JSONArray result = getSortExplain(query);
assertThat(1, equalTo(result.length()));
assertThat(query(result, "/0/name.keyword/order"), equalTo("asc"));
assertThat(query(result, "/0/name.keyword/missing"), equalTo("_first"));

JSONObject response = executeQuery(query);
JSONArray hits = getHits(response);
assertFalse(hits.getJSONObject(0).getJSONObject("_source").has("name"));
assertThat(hits.getJSONObject(hits.length()-1).query("/_source/name").toString(), equalTo("f"));

// Another equivalent syntax
assertThat(explainQuery("SELECT id, name FROM elasticsearch-sql_test_index_order " +
"ORDER BY name IS NOT NULL"),
equalTo(explainQuery("SELECT id, name FROM elasticsearch-sql_test_index_order " +
"ORDER BY name IS NOT NULL ASC"))
);
}

@Test
public void multipleOrderByWithNulls() throws IOException {
String query = "SELECT id, name FROM elasticsearch-sql_test_index_order ORDER BY id IS NULL, name IS NOT NULL";
JSONArray result = getSortExplain(query);
assertThat(result.length(), equalTo(2));
assertThat(query(result, "/0/id/missing"), equalTo("_last"));
assertThat(query(result, "/1/name.keyword/missing"), equalTo("_first"));
}

@Test
public void testOrderByMergeForSameField() throws IOException {
String query = "SELECT * FROM elasticsearch-sql_test_index_order " +
"ORDER BY id IS NULL, name DESC, id DESC, id IS NOT NULL, name IS NULL";
JSONArray result = getSortExplain(query);
assertThat(2, equalTo(result.length()));
assertThat(query(result, "/0/id/order"), equalTo("asc"));
assertThat(query(result, "/0/id/missing"), equalTo("_first"));
assertThat(query(result, "/1/name.keyword/order"), equalTo("asc"));
assertThat(query(result, "/1/name.keyword/missing"), equalTo("_last"));
}

private JSONArray getSortExplain(String sqlQuery) throws IOException {
String result = explainQuery(sqlQuery);
JSONObject jsonObject = new JSONObject(result);
return jsonObject.getJSONArray("sort");
}

private JSONObject getSortByField(JSONArray sortArray, String orderByName) {
JSONObject jsonObject;
for(int i = 0; i < sortArray.length(); i++) {
jsonObject = (JSONObject) sortArray.get(i);
if (jsonObject.has(orderByName)) {
return jsonObject;
}
}
return null;
}

private String query(JSONArray jsonArray, String jsonPath) {
return jsonArray.query(jsonPath).toString();
}
}
Loading

0 comments on commit d09e83a

Please sign in to comment.