The current code looks wrong and the suggested fix looks right.

None of our regression tests seem to exercise this code path, though. It would be good if someone could come up with a statement that caused this code to be executed so that we can verify that it behaves as expected after the proposed changes.

There was some discussion on the list about tracking down the statement that goes through BaseColumnNode.isEquivalient() , by adding a thread dump in the method. I wanted to suggest an alternate method that might make things easier to track down. I may be missing the mark as I have not been folllowing the thread on the list carefully.

I think if you set the system property derby.language.logStatementText=true with -Dderby.language.logStatementText=true and then in the method call org.apache.derby.iapi.services.monitor.Monitor.getMonitor().logThrowable(new Exception("Trace in isEquivalent")
you should get the stack trace right after the statement that goes there in derby.log

The getmonitor() method returns a modulefactory interface which doesnt have a logThrowable method in it..

Modified the BasecolumnNode class's IsEquivalent method to add the monitor method... But still when executing test.lang.__Suite, the log doesn't show any trace of the newly added trace , i am attaching the patch and the __Suite output

Perhaps the code is simply never run, or perhaps the traces are getting swallowed
up. The tests sometimes redirect the output, so it can be hard to figure out where the
traces might go.

Change the method to perform System.exit(1);

Then see if any of the tests in the test suite fail.

After adding the system.exit(1) to the BaseColumnNode.IsEquivalent method and the running java -Dderby.tests.trace=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.lang._Suite > runoutputJune14.out 2>&1
is causing the log output to stop at the "aggregate" test.
Attaching the output log and the diff for reference.

If it's definitely the 'aggregate' test which is touching the code, then you should be able
to confirm this by doing something like:

java org.apache.derbyTesting.functionTests.harness.RunTest lang/aggregate.sql

with and without your "System.exit(1)", and see the test pass without your change, and
exit early with your change.

Alternatively, since the test will probably fail with some spurious diffs when run in the old test harness now even when it's unmodified, aggregate.sql can be run as a standalone JUnit test by invoking this command:

java org.apache.derbyTesting.functionTests.tests.lang.LangScripts aggregate

Thanks for continuing the digging, Jayaram!

Modified the the basecolumnNode class isequivalent method to do a Thread.dumpStack(); and then ran the aggregate test alone by doing
$ java org.apache.derbyTesting.functionTests.tests.lang.LangScripts aggregate
>june15.out 2>&1

Verified that executing aggregate resulted in the dump getting produced . But the dump didnt indicate the sql statement which resulted in calling the isquivalent method.. Attaching the june15.out file for reference

Now that you know that the aggregate test exercises this code, can you try Kathey's suggestion from
https://issues.apache.org/jira/browse/DERBY-5010?focusedCommentId=13021634&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13021634

to see if you can further isolate the particular statement that is being executed?

As mentioned added the Monitor.getMonitor().logThrowable(new Exception("Trace in isEquivalent") and then executed java org.apache.derbyTesting.functionTests.tests.lang.LangScripts aggregate >> June19.out ..

in the derby log file got
Sun Jun 19 14:08:06 CDT 2011 Thread[main,5,main] (XID = 624), (SESSIONID = 3), (DATABASE = wombat), (DRDAID = null), Failed Statement is: null

the failed statement is coming as null.. Attaching the log file for reference... The log file shows the "isequivalent" method being invoked but the statement could not be traced out..

Looking at the derby.log, it seems to me that the derby.language.logStatementText=true property is not getting picked up.
If you look at the top of the log, there is no statement listed before the first instance of the stack trace printing,

Database Class Loader started - derby.database.classpath=''
java.lang.Exception: Trace in isEquivalent
at org.apache.derby.impl.sql.compile.BaseColumnNode.isEquivalent(BaseColumnNode.java:183)
at org.apache.derby.impl.sql.compile.SubstituteExpressionVisitor.visit(SubstituteExpressionVisitor.java:62)
at org.apache.derby.impl.sql.compile.QueryTreeNode.accept(QueryTreeNode.java:718)
at org.apache.derby.impl.sql.compile.ResultColumn.acceptChildren(ResultColumn.java:1550)

In your previous comment, you mentioned you ran it as:
java org.apache.derbyTesting.functionTests.tests.lang.LangScripts aggregate

try
java -Dderby.language.logStatementText=true org.apache.derbyTesting.functionTests.tests.lang.LangScripts aggregate

If that still doesn't show the statement before the exception prints, try running the java/testing/org/apache/derbyTesting/functionTests/tests/lang/aggregate.sql ust with ij, outside of the harness, making sure you start ij with the property

java -Dderby.language.logStatementText=true org.apache.derby.tools.ij

As suggested ran the aggregate test with logstatementtext=true option and was able to get the sql statement causing the isequivalent method to be called

select c1 from t1
group by c1
having (max(c2) in (select c1 from t2)) OR
(max(c1) in (select c2-999 from t2)) OR
(count > 0)

Thank you Jayaram for tracking this down!
Can you post the full stack trace from isEquivalent when this statement is executed?

Now that we know how to get into isEquivalent(), the next step will be to try to find a variation of this statement that fails without Dave's patch and passes with it. First, to get yourself setup to debug the situation, I would suggest you break out an SQL script with just the first part of aggregate.sql to setup these two tables call it create.sql or some such. Then you can use that to create a database with ij and then you can just connect with ij to the database and run this one sql statement and variations to understand what is happening in isEquivalent, either by printing the values for tableName and other.tableName or using the debugger.

adding the full stack trace of isequivalent when the statement got executed

When executing the sql statement select c1 from t1 group by c1 having (max(c2) in (select c1 from t2)) OR (max(c1) in (select c2-999 from t2)) OR (count > 0); in ij observed the following

1) the control goes to isequivalent method but if (isSameNodeType(o)) is returning false making the flow not to go in to the modified section of code
2) the reason why if (isSameNodeType(o)) is returning false is
2.1) in valuenode class nodetype has a value 62
2.2) in querytreenode class nodetype has a value 94

I am not able to infer what these values mean or how they got assigned... but if we are able to get to make them equal then control would go to the modified section of code...

Hi Jayaram,

I think these constants come from C_NodeTypes:

> static final int COLUMN_REFERENCE = 62;

and

> static final int BASE_COLUMN_NODE = 94;

So it looks like we're calling isEquivalent() to check whether a ColumnReference instance is equivalent to a BaseColumnNode in this query.

I think the ColumnReference here is the reference to C1 in the GROUP BY clause, and to exercise the code using a variant of this query, we'd have to get the GROUP BY clause to contain a BaseColumnNode somehow. I'm afraid I don't have any good ideas on how to do that (I'm not even sure if it's possible).

Hi Knut,
I tried all the following variations but nothing seem to break the basecolumn to columnreference comparison.. So shall i jump on to the next task
select c1 from t1 group by c1 having (max(c2) in (select c1 from t2)) OR (max(c1) in (select c2-999 from t2)) OR (count > 0);
select c1 from t1 group by c1 having (max(c1) in (select c1 from t2)) OR (max(c1) in (select c2-999 from t2)) OR (count > 0);
select c1 from t1 group by c1 having (max(c1) in (select c1 from t2)) OR (max(c1) in (select c1-999 from t2)) OR (count > 0);
select c1,count(c1) from t1 group by c1 having (max(c1) in (select c1 from t2)) OR (max(c1) in (select c1-999 from t2)) OR (count > 0);

Thanks for the continued investigation, Jayaram.

The more I look at this code, the more convinced I get that the broken code cannot currently be exercised, and your experiments seem to support that.

In any case, the patch looks correct, so I've committed it to trunk (revision 1143002). Thanks, Dave, for the fix.

It seems the code may not be reachable so no need to backport.

[bulk update] Close all resolved issues that haven't been updated for more than one year.