[
https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12532989
]
Bryan Pendleton commented on DERBY-3097:
----------------------------------------
Hi Knut Anders, thanks for the pointer to DERBY-1902,
but I think this problem is different.
The diff is quite interesting. I misspoke in my previous comment: Derby
does not choose a different query plan. The difference is more subtle.
The reference file has a section of query plan output which looks like this:
Index Scan ResultSet for T4 using index T4_IX2 at read
committed isolation level using share row locking chosen by the optimizer
Number of opens = 0
Rows seen = 0
Rows filtered = 0
Fetch Size = 1
constructor time (milliseconds) = 0
open time (milliseconds) = 0
next time (milliseconds) = 0
close time (milliseconds) = 0
scan information:
start position:
>= on first 1 column(s).
Ordered null semantics on the following columns:
stop position:
> on first 1 column(s).
Ordered null semantics on the following columns:
The actual output that I get with the "if" statement removed is this:
Index Scan ResultSet for T4 using index T4_IX2 at read
committed isolation level using share row locking chosen by the optimizer
Number of opens = 0
Rows seen = 0
Rows filtered = 0
Fetch Size = 1
constructor time (milliseconds) = 0
open time (milliseconds) = 0
next time (milliseconds) = 0
close time (milliseconds) = 0
scan information:
start position:
Positioning information not available because this ResultSet was never
opened.
stop position:
Positioning information not available because this ResultSet was never
opened.
That is, instead of printing the start and stop positions for the scan, the new
output instead prints a message saying that this ResultSet was never
positioned, and hence has no start and stop positions to print.
The actual output seems pretty reasonable, as in fact the T4 index scan
result set *was* never opened ("Number of opens = 0").
But why does removing the "if" statement in BaseActivation.getColumnFromRow
cause this change?
The answer lies in a routine called TableScanResultSet.printPosition,
where we see the following code:
if (positioner == null)
{
try
{
positioner =
(ExecIndexRow)positionGetter.invoke(activation);
}
catch (StandardException e)
{
// the positionGetter will fail with a
NullPointerException
// if the outer table is empty
// (this isn't a problem since we won't call it
on the inner
// table if there are no rows on the outer
table)
if (e.getSQLState() ==
SQLState.LANG_UNEXPECTED_USER_EXCEPTION )
return "\t" +
MessageService.getTextMessage(
SQLState.LANG_POSITION_NOT_AVAIL);
return "\t" + MessageService.getTextMessage(
SQLState.LANG_UNEXPECTED_EXC_GETTING_POSITIONER,
e.toString());
}
}
With the current trunk code, containing the defensive "if" statement
in BaseActivation.getColumnFromRow(), the positionGetter method quietly
returns a template index row filled with NULL values, and
TableScanResult.printPosition quietly formats and prints the index row.
But with the "if" statement removed, a NullPointerException is raised in
BaseActivation.getColumnFromRow() because we are trying to fetch a column
from a non-existent row. This NPE is then wrapped by the Java reflection
libraries into an InvocationTargetException, which is then caught by the
following code in ReflectMethod.invoke and turned into a Derby
StandardException:
try {
return realMethod.invoke(ref, null);
} catch (IllegalAccessException iae) {
t = iae;
} catch (IllegalArgumentException iae2) {
t = iae2;
} catch (InvocationTargetException ite) {
t = ite.getTargetException();
if (t instanceof StandardException)
throw (StandardException) t;
}
throw StandardException.unexpectedUserException(t);
This StandardException is then caught by TableScanResultSet.printPosition
and results in the "Positioning information not available" message.
So I think that the new output is reasonable, and I'm not entirely
opposed to simply updating the master file as part of this change.
However, it does *not* seem desirable to arrive at this output by
catching and recovering from a NullPointerException.
My next thought is that perhaps TableResultScan.printPosition can
detect that the result set has not been opened, and avoid calling the
positionGetter in this case, and instead proceed directly to formatting
and returning the LANG_POSITION_NOT_AVAIL message.
I'll investigate this possibility.
> Unnecessary if statement can be removed from BaseActivation.getColumnFromNow
> ----------------------------------------------------------------------------
>
> Key: DERBY-3097
> URL: https://issues.apache.org/jira/browse/DERBY-3097
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.4.0.0
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
>
> In BaseActivation.java there is the following code:
> protected final DataValueDescriptor getColumnFromRow(int rsNumber, int colId)
> throws StandardException {
> if( row[rsNumber] == null)
> {
> /* This actually happens. NoPutResultSetImpl.clearOrderableCache
> attempts to prefetch invariant values
> * into a cache. This fails in some deeply nested joins. See
> Beetle 4736 and 4880.
> */
> return null;
> }
> return row[rsNumber].getColumn(colId);
> }
> During the investigation of DERBY-3033, I came to the conclusion that this
> "if" statement is no longer necessary, and in fact is counter-productive, for
> it makes diagnosing other problems harder by delaying the point at which data
> structure problems are exposed as errors in the code.
> This JIRA issue requests that this code be evaluated, to determine whether or
> not it truly is necessary, and, if it is not necessary, suggests that it
> should be removed, to result in simpler, clearer code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
