Regression (wrong results): Join predicate can be ignored for left-most child in a chain of nested unions.

• Key: DERBY-1681
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: A B
• Reporter: A B
• Component/s: SQL
• Affects Version/s: 10.1.3.1, 10.2.1.6
• Fix Version/s: 10.1.4.0, 10.2.1.6, 10.3.1.4

Description

If Derby chooses to do a join between two FromTables and the right table is a chain of UNIONs, then the optimizer may choose to push the join predicate (if provided) down into the UNION and to both children of every UNION in the chain. But if the predicate cannot be pushed to the children of any of the UNIONs (except the top-level one) the predicate can end up being ignored altogether with respect to that UNION's children. The result is that query execution can return rows that do not satisfy the predicate.

This is a regression introduced in 10.1.2.4 and thus it affects 10.1.3 and 10.2. I came across this while tracing through code for DERBY-1633.

Comments

[A B]

Attaching three things to this issue:

  1. A patch for the problem: d1681_v1.patch
  2. A patch for a test to verify the fix: d1681_test.patch
  3. A (short) description of the issue and the fix.

The reason I've separated the fix from the test is that the test case relies on database objects that are included in the patch for DERBY-1633, and thus the test patch cannot be applied until the patch for DERBY-1633 has been applied.

The actual fix to the problem, though--i.e. d1681_v1.patch--can be applied and committed independently of DERBY-1633, and thus I've separated it out.

I ran derbyall on Red Hat Linux with ibm142 against SANE jars and there were no new failures.

Both patches are ready for review and commit (with the caveat just noted about the test case).

[Yip Ng]

I have reviewed the patch and the logic looks reasonable to me. I think its ready to be committed. +1

[Mike Matrigali]

Reviewed and ran tests on this patch. Committed to trunk:
m1_142:151>svn commit

Sending java\engine\org\apache\derby\impl\sql\compile\SetOperatorNode.java
Transmitting file data .
Committed revision 432367.

[Mike Matrigali]

changing fix in version as I believe this fix should be back ported to 10.1.

[Mike Matrigali]

committed backport of change from trunk to 10.1:

------------------------------------------------------------------------
r432414 | mikem | 2006-08-17 15:33:59 -0700 (Thu, 17 Aug 2006) | 20 lines

DERBY-1681, submitted by Army Brown

Backporting svn r432367 from trunk into 10.1.

Fixes the following issue:
If Derby chooses to do a join between two FromTables and the right table is a
chain of UNIONs, then the optimizer may choose to push the join predicate (if
provided) down into the UNION and to both children of every UNION in the chain.
But if the predicate cannot be pushed to the children of any of the UNIONs
(except the top-level one) the predicate can end up being ignored altogether
with respect to that UNION's children. The result is that query execution can
return rows that do not satisfy the predicate.

This is a regression introduced in 10.1.2.4 and thus it affects 10.1.3 and 10.2.
I came across this while tracing through code for DERBY-1633.

For more detailed explanation of problem and solution see html doc attached to
DERBY-1681.

[A B]

Attaching a second version of the new test case for this issue. The test changed as part of the DERBY-1633 checkins and thus the patch had to be re-synced.

Note that the test cases which were added for DERBY-1633 actually already demonstrate the problem reported (and now fixed) in this issue. However, I don't think it hurts to have a dedicated test case, so I still think it would be good to commit d1681_tests_v2.patch to the trunk and 10.2 codelines (those are the codelines into which the fix for this issue has already been comitted).

I'm also working to port DERBY-1633 to 10.1; when that's done, the test patch for this issue should be ported to 10.1, as well.

This is test-only patch: d1681_test_v2.patch is ready for commit.

[Mike Matrigali]

I will look at committing these test changes to trunk and 10.2

[Mike Matrigali]

committed d1681_test_v2.patch to trunk:

m1_142:27>svn commit

Sending java\testing\org\apache\derbyTesting\functionTests\master\predicatePushdown.out
Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\predicatePushdown.sql
Transmitting file data ..
Committed revision 437380.

[Mike Matrigali]

backported and committed patch d1681_test_v2.patch from trunk to 10.2 branch:

r437487 | mikem | 2006-08-27 17:34:07 -0700 (Sun, 27 Aug 2006) | 14 lines

DERBY-1681
merging change 437380 from trunk to 10.2 branch:

DERBY-1691
contributed by Army Brown

Adding extra tests for this issue, fix already committed.

Note that the test cases which were added for DERBY-1633 actually already
demonstrate the problem reported (and now fixed) in this issue. This checkin
is a dedicated test case for this issue.

[A B]

Thanks for commiting the new test case to trunk and 10.2, Mike. Since the changes for DERBY-1633 have now been ported to 10.1 (thank you for that, too, Mike!), can the test case for this issue go to 10.1, as well? It should be a simple merge:

svn merge -r 437379:437380 http://svn.apache.org/repos/asf/db/derby/code/trunk

Also, one small note: it looks there's a typo in the Jira issue for the svn commit logs with the d1681_test_v2.patch: there are a couple of references to "DERBY-1691" instead of "DERBY-1681". Perhaps these can be corrected?

[Mike Matrigali]

merged test case from trunk to 10.1 branch:

437380

m101_142:9>svn commit

Sending java\testing\org\apache\derbyTesting\functionTests\master\predicatePushdown.out
Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\predicatePushdown.sql
Transmitting file data ..
Committed revision 438138.

[A B]

Reopening to update Fixin to include trunk (10.3)

[A B]

Fix and tests for this issue have not been commited to 10.1, 10.2, and 10.3 branches (thanks Mike and Rick). Un-checking the "patch available" flag and resolving the issue.

[A B]

oops, typo:

"have not been committed to..."

should be

"have now been committed to..."

[Mike Matrigali]

I fixed the typo's that I could see, if there are any others let me know. Not a svn expert, you may have to svn update to see changes - not sure. Easiest if you give the svn rev number of the problem log, it is very easy to edit them: ie. svn propedit svn:log --revprop -r 437487, pops you into an edit window like svn commit.

[A B]

Looks like I was making changes the same time Mike was, and our changes clashed. So I'm reopening (again) to correct fix-in and uncheck patch available...