Skip to content

Commit

Permalink
Add more tests and fixup nullable
Browse files Browse the repository at this point in the history
  • Loading branch information
nik9000 committed Aug 17, 2023
1 parent 888e3d1 commit 1307dab
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,29 @@ emp_no:integer | first_name:keyword
10010 | X
;

coalesceEndsInNull
# ending in null is sill because it'll noop but it shouldn't break things.
FROM employees
| EVAL first_name = COALESCE(first_name, last_name, null)
| SORT first_name DESC, emp_no ASC
| KEEP emp_no, first_name
| limit 3;

emp_no:integer | first_name:keyword
10047 | Zvonko
10081 | Zhongwei
10026 | Yongqiao
;

coalesceFolding
FROM employees
| EVAL foo=COALESCE(true, false, null)
| SORT emp_no ASC
| KEEP emp_no, first_name, foo
| limit 3;

emp_no:integer | first_name:keyword | foo:boolean
10001 | Georgi | true
10002 | Bezalel | true
10003 | Parto | true
;
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,18 @@ protected TypeResolution resolveType() {

@Override
public Nullability nullable() {
return children().get(children().size() - 1).nullable();
// If any of the children aren't nullable then this isn't.
for (Expression c : children()) {
if (c.nullable() == Nullability.FALSE) {
return Nullability.FALSE;
}
}
/*
* Otherwise let's call this one "unknown". If we returned TRUE here
* an optimizer rule would replace this with null if any of our children
* fold to null. We don't want that at all.
*/
return Nullability.UNKNOWN;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import org.elasticsearch.xpack.esql.planner.Layout;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.FieldAttribute;
import org.elasticsearch.xpack.ql.expression.Literal;
import org.elasticsearch.xpack.ql.expression.Nullability;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;
Expand Down Expand Up @@ -151,4 +153,24 @@ public void testCoalesceIsLazy() {
return EvalMapper.toEvaluator(child, layout);
}).get().eval(row(testCase.getDataValues())), 0), testCase.getMatcher());
}

public void testCoalesceNullabilityIsUnknown() {
assertThat(buildFieldExpression(testCase).nullable(), equalTo(Nullability.UNKNOWN));
}

public void testCoalesceKnownNullable() {
List<Expression> sub = new ArrayList<>(testCase.getDataAsFields());
sub.add(between(0, sub.size()), new Literal(Source.EMPTY, null, sub.get(0).dataType()));
Coalesce exp = build(Source.EMPTY, sub);
// Still UNKNOWN - if it were TRUE then an optimizer would replace it with null
assertThat(exp.nullable(), equalTo(Nullability.UNKNOWN));
}

public void testCoalesceNotNullable() {
List<Expression> sub = new ArrayList<>(testCase.getDataAsFields());
sub.add(between(0, sub.size()), randomLiteral(sub.get(0).dataType()));
Coalesce exp = build(Source.EMPTY, sub);
// Known not to be nullable because it contains a non-null literal
assertThat(exp.nullable(), equalTo(Nullability.FALSE));
}
}

0 comments on commit 1307dab

Please sign in to comment.