Skip to content

Commit

Permalink
fix LuceneQueryBuilder for "x = null" where x is numeric or boolean
Browse files Browse the repository at this point in the history
This actually isn't a problem in 0.48 because the "= null"
case is handled during normalization or analysis. But it is
a problem in 0.47 for "where x in (null)" where this will
result in a Range query from null to null which matches
everything
  • Loading branch information
mfussenegger committed Mar 17, 2015
1 parent f639351 commit fc7103d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
11 changes: 11 additions & 0 deletions sql/src/main/java/io/crate/lucene/QueryBuilderHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import org.apache.lucene.search.*;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.lucene.search.MatchNoDocsFilter;
import org.elasticsearch.common.lucene.search.Queries;

public abstract class QueryBuilderHelper {

Expand Down Expand Up @@ -53,10 +55,16 @@ public static QueryBuilderHelper forType(DataType dataType) {
public abstract Query rangeQuery(String columnName, Object from, Object to, boolean includeLower, boolean includeUpper);

public Filter eqFilter(String columnName, Object value) {
if (value == null) {
return new MatchNoDocsFilter();
}
return rangeFilter(columnName, value, value, true, true);
}

public Query eq(String columnName, Object value) {
if (value == null) {
return Queries.newMatchNoDocsQuery();
}
return rangeQuery(columnName, value, value, true, true);
}

Expand All @@ -77,6 +85,9 @@ public Query rangeQuery(String columnName, Object from, Object to, boolean inclu

@Override
public Query eq(String columnName, Object value) {
if (value == null) {
return Queries.newMatchNoDocsQuery();
}
return new TermQuery(new Term(columnName, value == true ? "T" : "F"));
}
}
Expand Down
18 changes: 18 additions & 0 deletions sql/src/test/java/io/crate/lucene/LuceneQueryBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,24 @@ public void testNoMatchWhereClause() throws Exception {
assertThat(query, instanceOf(MatchNoDocsQuery.class));
}

@Test
public void testWhereRefEqNullInteger() throws Exception {
Reference foo = createReference("foo", DataTypes.INTEGER);
Query query = convert(whereClause(EqOperator.NAME, foo, Literal.newLiteral(DataTypes.INTEGER, null)));

// shouldn't become a range query because from null to null is basically match all
assertThat(query, instanceOf(MatchNoDocsQuery.class));
}

@Test
public void testWhereRefEqNullBoolean() throws Exception {
Reference foo = createReference("foo", DataTypes.BOOLEAN);
Query query = convert(whereClause(EqOperator.NAME, foo, Literal.newLiteral(DataTypes.BOOLEAN, null)));

// shouldn't become a TermQuery because otherwise where b = null would match columns where b = false
assertThat(query, instanceOf(MatchNoDocsQuery.class));
}

@Test
public void testWhereRefEqRef() throws Exception {
Reference foo = createReference("foo", DataTypes.STRING);
Expand Down

0 comments on commit fc7103d

Please sign in to comment.