Skip to content

Commit

Permalink
support eq on arrays
Browse files Browse the repository at this point in the history
  • Loading branch information
mfussenegger committed Feb 6, 2015
1 parent d3b2a05 commit 0a24539
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 20 deletions.
4 changes: 1 addition & 3 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ Changes for Crate
Unreleased
==========

- Comparing arrays now works correctly for literals and comparing an array
field to an array literal will result in a correct
UnsupportedOperationException.
- Added support for array comparisons using the equals ``=`` operator.

- Support `INSERT ... ON DUPLICATE KEY UPDATE` statement

Expand Down
61 changes: 55 additions & 6 deletions sql/src/main/java/io/crate/lucene/LuceneQueryBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,12 @@
import io.crate.operation.scalar.geo.WithinFunction;
import io.crate.planner.symbol.*;
import io.crate.types.CollectionType;
import io.crate.types.DataType;
import io.crate.types.DataTypes;
import org.apache.lucene.index.AtomicReader;
import org.apache.lucene.index.AtomicReaderContext;
import org.apache.lucene.index.Term;
import org.apache.lucene.queries.BooleanFilter;
import org.apache.lucene.sandbox.queries.regex.JavaUtilRegexCapabilities;
import org.apache.lucene.sandbox.queries.regex.RegexQuery;
import org.apache.lucene.search.*;
Expand Down Expand Up @@ -344,13 +346,57 @@ public Query apply(Function input, Context context) {
return null;
}

if (DataTypes.isCollectionType(tuple.v1().valueType()) && DataTypes.isCollectionType(tuple.v2().valueType())) {
throw new UnsupportedFeatureException("Cannot compare two arrays");
Reference reference = tuple.v1();
Literal literal = tuple.v2();
String columnName = reference.info().ident().columnIdent().fqn();
if (DataTypes.isCollectionType(reference.valueType()) && DataTypes.isCollectionType(literal.valueType())) {
// create terms query to utilize lucene index to pre-filter the result..
BooleanQuery booleanQuery = new BooleanQuery();
DataType type = literal.valueType();
while (DataTypes.isCollectionType(type)) {
type = ((CollectionType) type).innerType();
}
QueryBuilderHelper builder = QueryBuilderHelper.forType(type);
Object value = literal.value();
buildTermsQuery(booleanQuery, value, columnName, builder);

if (booleanQuery.clauses().isEmpty()) {
// all values are null...
return genericFunctionQuery(input);
}
// genericFunctionFilter will do the exact match, operating on the _DOC
BooleanFilter filterClauses = new BooleanFilter();
filterClauses.add(new QueryWrapperFilter(booleanQuery), BooleanClause.Occur.MUST);
filterClauses.add(genericFunctionFilter(input), BooleanClause.Occur.MUST);
return new FilteredQuery(Queries.newMatchAllQuery(), filterClauses);
}
String columnName = tuple.v1().info().ident().columnIdent().fqn();
QueryBuilderHelper builder = QueryBuilderHelper.forType(tuple.v1().valueType());
return builder.eq(columnName, tuple.v2().value());
}

private boolean buildTermsQuery(BooleanQuery booleanQuery,
Object value,
String columnName,
QueryBuilderHelper builder) {
if (value == null) {
return true;
}
if (value.getClass().isArray()) {
Object[] array = (Object[]) value;
for (Object o : array) {
if (!buildTermsQuery(booleanQuery, o, columnName, builder)) {
return false;
}
}
} else {
try {
booleanQuery.add(builder.eq(columnName, value), BooleanClause.Occur.MUST);
} catch (BooleanQuery.TooManyClauses e) {
return false;
}
}
return true;
}
}

class AndQuery implements FunctionToQuery {
Expand Down Expand Up @@ -835,7 +881,7 @@ private String validateNoUnsupportedFields(Function function, Context context){
return null;
}

private Query genericFunctionQuery(Function function) {
private Filter genericFunctionFilter(Function function) {
if (function.valueType() != DataTypes.BOOLEAN) {
raiseUnsupported(function);
}
Expand Down Expand Up @@ -876,8 +922,11 @@ public DocIdSet getDocIdSet(AtomicReaderContext context, Bits acceptDocs) throws
acceptDocs);
}
};
Filter cachedFilter = indexCache.filter().cache(filter);
return new FilteredQuery(Queries.newMatchAllQuery(), cachedFilter);
return indexCache.filter().cache(filter);
}

private Query genericFunctionQuery(Function function) {
return new FilteredQuery(Queries.newMatchAllQuery(), genericFunctionFilter(function));
}

static class FunctionDocSet extends MatchDocIdSet {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,27 @@ public void testWhereFunctionWithAnalyzedColumnArgument() throws Exception {
assertThat(response.rowCount(), is(1L));
}

@Test
public void testEqualsQueryOnArrayType() throws Exception {
execute("create table t (a array(integer)) with (number_of_replicas = 0)");
ensureYellow();
execute("insert into t (a) values (?)", new Object[][]{
new Object[]{new Object[] {10, 10, 20}},
new Object[]{new Object[] {40, 50, 60}},
new Object[]{new Object[] {null, null}}
});
execute("refresh table t");

execute("select * from t where a = [10, 10, 20]");
assertThat(response.rowCount(), is(1L));

execute("select * from t where a = [10, 20]");
assertThat(response.rowCount(), is(0L));

execute("select * from t where a = [null, null]");
assertThat(response.rowCount(), is(1L));
}

@Test
public void testWhereFunctionWithIndexOffColumn() throws Exception {
execute("create table t (text string index off) " +
Expand Down
37 changes: 26 additions & 11 deletions sql/src/test/java/io/crate/lucene/LuceneQueryBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.carrotsearch.randomizedtesting.RandomizedTest;
import com.google.common.collect.Sets;
import io.crate.analyze.WhereClause;
import io.crate.exceptions.UnsupportedFeatureException;
import io.crate.metadata.FunctionIdent;
import io.crate.metadata.FunctionInfo;
import io.crate.metadata.Functions;
Expand All @@ -45,9 +44,7 @@
import org.elasticsearch.index.cache.IndexCache;
import org.elasticsearch.search.internal.SearchContext;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Answers;

import java.util.Arrays;
Expand All @@ -58,13 +55,10 @@
import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;

public class LuceneQueryBuilderTest extends RandomizedTest{
public class LuceneQueryBuilderTest extends RandomizedTest {

private LuceneQueryBuilder builder;

@Rule
public ExpectedException expectedException = ExpectedException.none();

@Before
public void setUp() throws Exception {
Functions functions = new ModulesBuilder()
Expand Down Expand Up @@ -99,13 +93,34 @@ public void testLteQuery() throws Exception {

@Test
public void testEqOnTwoArraysBecomesGenericFunctionQuery() throws Exception {
expectedException.expect(UnsupportedFeatureException.class);
expectedException.expectMessage("Cannot compare two arrays");
DataType longArray = new ArrayType(DataTypes.LONG);
convert(new WhereClause(createFunction(EqOperator.NAME,
Query query = convert(new WhereClause(createFunction(EqOperator.NAME,
DataTypes.BOOLEAN,
createReference("x", longArray),
Literal.newLiteral(longArray, new Object[] { 10L, null, 20L }))));
assertThat(query, instanceOf(FilteredQuery.class));
}

@Test
public void testEqOnTwoArraysBecomesGenericFunctionQueryAllValuesNull() throws Exception {
DataType longArray = new ArrayType(DataTypes.LONG);
Query query = convert(new WhereClause(createFunction(EqOperator.NAME,
DataTypes.BOOLEAN,
createReference("x", longArray),
Literal.newLiteral(longArray, new Object[] { 10L, 20L }))));
Literal.newLiteral(longArray, new Object[] { null, null, null }))));
assertThat(query, instanceOf(FilteredQuery.class));
}

@Test
public void testEqOnArrayWithTooManyClauses() throws Exception {
Object[] values = new Object[2000]; // should trigger the TooManyClauses exception
Arrays.fill(values, 10L);
DataType longArray = new ArrayType(DataTypes.LONG);
Query query = convert(new WhereClause(createFunction(EqOperator.NAME,
DataTypes.BOOLEAN,
createReference("x", longArray),
Literal.newLiteral(longArray, values))));
assertThat(query, instanceOf(FilteredQuery.class));
}

@Test
Expand Down

0 comments on commit 0a24539

Please sign in to comment.