[
https://issues.apache.org/jira/browse/DERBY-2218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12466173
]
A B commented on DERBY-2218:
----------------------------
I looked at the patch and it seems straightforward--thank you for tracking this
down, Yip!
The test cases all look good--but I did notice that they are limited to IN and
EXISTS predicates. Since the only file you changed was SuqbueryNode.java that
made me wonder if the problem didn't apply elsewhere, as well. And sure
enough, it looks like "values null" will cause a NPE if it appears in any part
of the query except the FROM list--without your patch.
For example, all of the following queries fail with NPEs without your patch but
throw the correct error with it:
-- where clause
select * from t1 where (values null);
-- order by clause
select * from t1 order by (values null);
-- result column
select (values null) from t1;
-- group by clause
select * from t1 group by (values null);
-- having clause
select * from t1 group by i having (values null);
So your patch actually fixes more than just IN-list; I wonder if the
description for this issue should be changed to reflect that? In any event, do
you think it would be useful to add these queries to the test, as well? If so,
that can of course come as a follow-up patch (by you or someone else with that
fish to fry).
My only other comment is that with your change we will end up calling:
resultSet.bindResultColumns(fromList);
*twice* for an EXISTS query. Given that derbyall and suites.All all passed I
guess this isn't really a problem--but it does kind of look weird in the code
and it sort of makes me worry (a little). How certain are we that
double-binding is safe thing to do here (I admit, I didn't actually look into
the relevant code)? Would it be worth it to add some minimal logic to avoid
the double bind? Also, with the patch we call "bindExpressions()" for EXISTS
queries when we didn't use to. Again, this is apparently pretty harmless, but
I thought I'd mention it...
If you think the double-binding is safe and that it's not worth adding logic to
avoid it, then just let me know and I'll go ahead with the commit. I don't
consider anything I've mentioned here to be a blocker for the patch.
Thank you for picking this issue up and resolving it so quickly!
> Null Pointer Exception when an IN list contains an untyped NULL subquery
> ("values null").
> -----------------------------------------------------------------------------------------
>
> Key: DERBY-2218
> URL: https://issues.apache.org/jira/browse/DERBY-2218
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.1.3.2, 10.2.2.1, 10.2.3.0, 10.3.0.0
> Reporter: A B
> Assigned To: Yip Ng
> Priority: Minor
> Attachments: derby2218-trunk-diff01.txt, derby2218-trunk-diff02.txt,
> derby2218-trunk-stat01.txt, derby2218-trunk-stat02.txt
>
>
> If a query specifies an IN list that contains a subquery which returns an
> untyped NULL value, Derby will throw an NPE at bind time.
> ij> create table t1 (i int);
> 0 rows inserted/updated/deleted
> ij> select * from t1 where i in (1, 2, (values null));
> ERROR XJ001: Java exception: ': java.lang.NullPointerException'.
> I verified the error against the latest 10.2 and 10.3 trunks; it could very
> well exist in earlier versions, too, but I haven't checked.
> Stack trace (from 10.2.2) is:
> java.lang.NullPointerException
> at
> org.apache.derby.impl.sql.compile.SubqueryNode.setDataTypeServices(SubqueryNode.java:2289)
> at
> org.apache.derby.impl.sql.compile.SubqueryNode.bindExpression(SubqueryNode.java:529)
> at
> org.apache.derby.impl.sql.compile.ValueNodeList.bindExpression(ValueNodeList.java:130)
> at
> org.apache.derby.impl.sql.compile.BinaryListOperatorNode.bindExpression(BinaryListOperatorNode.java:161)
> at
> org.apache.derby.impl.sql.compile.SelectNode.bindExpressions(SelectNode.java:540)
> at
> org.apache.derby.impl.sql.compile.DMLStatementNode.bindExpressions(DMLStatementNode.java:249)
> at
> org.apache.derby.impl.sql.compile.DMLStatementNode.bind(DMLStatementNode.java:162)
> at
> org.apache.derby.impl.sql.compile.CursorNode.bind(CursorNode.java:253)
> at
> org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:345)
> at
> org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:119)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
