EXISTS may return the wrong value for sub-queries involving set operations

• Key: DERBY-2370
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: A B
• Reporter: Dyre Tjeldvoll
• Component/s: SQL
• Affects Version/s: 10.2.2.0
• Fix Version/s: 10.3.1.4

Description

It seems like EXISTS on a SELECT returning zero rows returns false (as
expected), but EXISTS on INTERSECT of two disjunct sets returns true,
e.g EXISTS (values 1 intersect values 2).

Yip Ng wrote on derby-dev:
I believe its probably got to do with the EXISTS subquery transforming
the original RCL to
a TRUE boolean value for the INTERSECT. So during row comparison at
execution time
for INTERSECT processing since true == true(thus intersects), so it
will always return 'BAD'. Likewise,

select * from ( values 'OK' ) as T where exists (values 1 except values 2);

This supposedly should return 'OK' but because of the boolean
transformation mentioned
above for EXISTS subquery, it will return no rows for EXCEPT
processing.

Comments

[Dyre Tjeldvoll]

ij script to reproduce the bug.

[A B]

Yip was exactly right in his diagnosis of the problem. I'm attaching a short writeup of the problem and a proposed solution as d2370_writeup_v1.patch.

I'm also attaching a patch, d2370_engine_v1.patch, that implements the change by doing the following:

  1. Adds the ability to mark a FromList as "transparent", and updates FromList.bindExpressions() to
     pass the outer FROM list down (instead of "this") if the FromList is transparent.
  2. Updates FromList.expandAll(...) to account for the fact that outer FROM tables might now appear
     in a nested FromList (as a result of "transparent" FromLists; see code comments for details).
  3. Modifies the "setResultToBooleanTrue()" signature to return a ResultSetNode (it was "void" previously).
  4. Modifies ResultSetNode.setResultToBooleanTrue() to always return "this".
  5. Modifies SetOperatorNode.setResultToBooleanTrue() so that it now creates an internal "SELECT *" query
     whose FROM list contains just the SetOperatorNode. Then we transform the "*" for the new SELECT into
     "TRUE" and leave the SetOperatorNode's result columns UN-transformed. Finally, we mark the new
     SelectNode's FromList as "transparent" and return the new SelectNode.

I've included a corresponding patch, d2370_tests_v1.patch, that contains slight modifications to two tests: lang/union.sql and lang/ResultSetsFromPreparedStatementTest. The latter changes are expected based on comments in the test; the former (lang/union.sql) has a couple of queries that now fail when they used to succeed. However, I think the failures are correct--i.e. that Derby should have been failing prior to these changes and was not--so I've updated lang/union.sql accordingly. I will send an email about this to derby-dev to see if I can get any feedback/suggestions around this. And finally, d2370_tests_v1.patch creates a new JUnit test, lang/ExistsWithSetOpsTest, which captures and builds on the repro.sql script attached to this issue.

I ran derbyall and suites.All with a single failure:

   jdbcapi/parameterMetaDataJdbc30.java

This failure only occurred for the client framework and the diff showed a failure to connect--which doesn't seem related to my changes. When I ran the test independently it passed as expected. So I think it was just a fluke.

Reviews or other feedback would be greatly appreciated, as always.

[Dyre Tjeldvoll]

I have looked at the writeup and and also looked at the
patch. While I am certainly no expert in this area I think the
explanation and solution in the writeup looks very sound. The
patch seem to implement what is suggested, it is clear, well
documented and tested. +1 from me.

[A B]

Thank you for the review, Dyre. I synced with the latest codeline and re-ran derbyall and suites.All as a sanity check, then committed the patch with svn #532509:

  URL: http://svn.apache.org/viewvc?view=rev&rev=532509

As I haven't heard any comments regarding the new behavior for certain queries (namely, throwing an error where Derby used to succeed in some cases), I'm working under the assumption that this is okay and am marking the issue RESOLVED.

Many thanks for your time!

[A B]

Reopening to attach release note.

[A B]

Attaching first draft of the release note.

[A B]

Marking issue resolved (again). I'll wait a few days before closing it in case there are comments on the release note...

[Rick Hillegas]

Scrub the html in the release note so that it can be digested by the SAX parser in the release note generator.