Fix NPE in pushdown to NULL expression

[alexey-fin]

toImmutableList does not accept nulls, so pushdown to a null expression fails:

Caused by: java.lang.NullPointerException
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:877)
at com.google.common.collect.ImmutableList$Builder.add(ImmutableList.java:788)
at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
at java.base/java.util.Collections$2.tryAdvance(Collections.java:4747)
at java.base/java.util.Collections$2.forEachRemaining(Collections.java:4755)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
at io.prestosql.sql.planner.ExpressionInterpreter$Visitor.visitBindExpression(ExpressionInterpreter.java:974)
....

[tooptoop4]

@alexey-fin what is a sample query affected?

[martint]

Thanks! Can you add a test that reproduces the issue? It should go in TestExpressionInterpreter.

[findepi]

static import + comment

Suggested change

	.collect(Collectors.toList());
	.collect(toList()); // values are nullable

[alexey-fin]

@martint
i think the test should be in the planner,
is it a good enough place?

[alexey-fin]

@tooptoop4 this fails
WITH
u(k, v) AS (SELECT null, CAST(name AS varchar(4)) FROM nation)
SELECT 1
FROM u
WHERE try(cast(u.k as varchar)) = 'x'

[martint]

This failure is unrelated to predicate pushdown. It's due to the null being inlined in the argument to the internal bind function that's used to model try when the InlineProjections optimizer rule executes. The query can be simplified to the following and still reproduce the issue:

SELECT try(k)
FROM (SELECT null) t(k)

Let's add a test using the simplified query in io.prestosql.sql.query.TestExpressions instead. Also add a comment in the test linking to the github issue so that we can trace back the reason for the test in the future.

[martint]

There's a checkstyle error:

[ERROR] src/test/java/io/prestosql/sql/query/TestExpressions.java:[53,38] (blocks) LeftCurly: '{' at column 38 should be on a new line.

Also, can you squash the commits? Once the build passes, I'll merge this. Thanks for finding and fixing this issue!

[alexey-fin]

@martint
sorry for the mess

[martint]

Merged, thanks!