You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Bryan Pendleton (JIRA)" <ji...@apache.org> on 2007/10/03 04:31:50 UTC
[jira] Created: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
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.
Re: [jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by Bryan Pendleton <bp...@amberpoint.com>.
Myrna van Lunteren wrote:
>
> I found this:
Thanks very much, Myrna! This is quite interesting, and
I will have a deeper look to see what I can learn about DERBY-3097.
thanks,
bryan
Re: [jira] Commented: (DERBY-3097) Unnecessary if statement can be removed from BaseActivation.getColumnFromNow
Posted by Myrna van Lunteren <m....@gmail.com>.
On 10/7/07, Bryan Pendleton (JIRA) <ji...@apache.org> wrote:
> Does anyone have access to the old Cloudscape test script for "Beetle 4880"?
> I'd like to see if this test sheds any light on the BaseActivation.getColumnFromRecord code,
> and possibly add the test case to our current regression suite.
Hi,
I found is this:
------------
NEST_THIS definition:
public static int NEST_THIS( int returnValue,
String statement1 )
throws SQLException, StandardException
{
Connection conn = getCurrentConnection();
PreparedStatement ps = conn.prepareStatement( statement1 );
ps.execute();
return returnValue;
}
ij> create table L_1
(
pk int primary key,
s1 int,
b int,
s2 int,
s3 int
);
0 rows inserted/updated/deleted
ij> create unique btree index L_1_UNIQUE on L_1( s1, s2, s3 );
0 rows inserted/updated/deleted
ij> insert into L_1 ( pk, s1 ) values
( -16, NEST_THIS( 103, 'delete from L_1 properties index=null
where s2 = 103 and s1 =
NEST_THIS( 103, ''insert into L_1 ( pk, s1, s2, s3 ) values
( -18, 3, 1, 1 ), ( -19, 403, 403, 403 )'' )' ) );
-------------------------------
The info further indicates that at one time, this code caused a deadlock.
The notes say: "Consider a query like: select * from t1, t2 where
t2.c1 = MyClass::myMethod(t1.c1). Since the method call's arguments
have no local reference to t2, and the predicate is pushed down as a
qualifier, we should be able to deem it as scan-invariant and evaluate
it once per outer row, so all together row_count(t1) number of times.
And we should evaluate it outside the store, before a heap page is
latched."
I've not tried this code out; not sure the syntax even works in Derby.
I hope it helps.
I'll leave it to you to add this to the bug comments or not...
Myrna
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12532980 ]
Knut Anders Hatlen commented on DERBY-3097:
-------------------------------------------
You might be seeing the intermittent failure reported in DERBY-1902.
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12601421#action_12601421 ]
Knut Anders Hatlen commented on DERBY-3097:
-------------------------------------------
Not all that useful, but in case someone finds it interesting...
I tried to modify the test case so that it would run on Derby. To get the correct syntax, I did
a) replace getCurrentConnection() with DriverManager.getConnection("jdbc:default:connection") in the Java method
b) add a CREATE FUNCTION statement to create the NEST_THIS function
c) change "create unique btree index" to "create unique index"
d) change "properties index=null" to "--DERBY-PROPERTIES index=null"
The first problem that I ran into, was that the optimizer override caused a syntax error. Apparently, optimizer overrides are not supported for DELETE statements. Not sure if this is a bug or not. The documentation says that they work on FROM clauses, and I couldn't find anything about their being limited to SELECT FROM. Anyway, it was pretty easy to enable it in the parser (can't guarantee that it works perfectly, though). Just change a flag from false to true in sqlgrammar.deleteBody().
Next problem was that functions don't allow SQL statements that modify data. Since the test case indicates that it was supported in Cloudscape, it might be that only a simple change in the parser is required, so that it accepts "MODIFIES SQL" in CREATE FUNCTION statements, but I didn't get that far before I ran out of daylight.
> 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.1.3
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
> Attachments: checkNumOpens.diff
>
>
> 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.
[jira] Issue Comment Edited: (DERBY-3097) Unnecessary if statement
can be removed from BaseActivation.getColumnFromNow
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12549190 ]
army edited comment on DERBY-3097 at 12/6/07 12:58 PM:
------------------------------------------------------
Turns out that this same "if" statement is also at play in DERBY-3253. The repro there fails with an NPE whether or not checkNumOpens.diff is applied in my codeline...
was (Author: army):
Turns out that this same "if" statement is also at play in DERBY-3253. I applied the checkNumOpens.diff patch attached here and that didn't help--still fails with an NPE...
> 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
> Attachments: checkNumOpens.diff
>
>
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Dag H. Wanvik (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908130#action_12908130 ]
Dag H. Wanvik commented on DERBY-3097:
--------------------------------------
It appears that the removed code in BaseActivation resulted in a regression, cf DERBY-4798.
> 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.1.3
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
> Fix For: 10.5.1.1
>
> Attachments: checkNumOpens.diff, updatedJune2008.diff
>
>
> 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.
[jira] Resolved: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Pendleton resolved DERBY-3097.
------------------------------------
Resolution: Fixed
Fix Version/s: 10.5.0.0
Committed the updated patch to the trunk as revision 662947
and am marking this issue as resolved.
Thanks all for your help!
> 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.1.3
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
> Fix For: 10.5.0.0
>
> Attachments: checkNumOpens.diff, updatedJune2008.diff
>
>
> 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.
[jira] Issue Comment Edited: (DERBY-3097) Unnecessary if statement
can be removed from BaseActivation.getColumnFromNow
Posted by "Thomas Nielsen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12544385 ]
thomanie edited comment on DERBY-3097 at 11/21/07 12:53 AM:
------------------------------------------------------------------
The defensive if() statement is triggered in DERBY-3094, but in 3094 results in an NPE. Applying this patch and thus removing the defensiveness, cause another NPE from the following return statement in 3094.
Marking these two reports as related.
Is the attached patch only fixing the symptom, rather than the underlying root cause that makes the if() statement necessary?
was (Author: thomanie):
The defensive if() statement is triggered in DERBY-3094, but in 3094 results in an NPE. Applying this patch and thus removing the defensiveness, cause another NPE from the following return statement in 3094.
Is the attached patch only fixing the symptom, rather than the underlying root cause that makes the if() statement necessary?
> 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
> Attachments: checkNumOpens.diff
>
>
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "A B (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12549190 ]
A B commented on DERBY-3097:
----------------------------
Turns out that this same "if" statement is also at play in DERBY-3253. I applied the checkNumOpens.diff patch attached here and that didn't help--still fails with an NPE...
> 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
> Attachments: checkNumOpens.diff
>
>
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12532979 ]
Bryan Pendleton commented on DERBY-3097:
----------------------------------------
Removing the "if" statement causes a test failure in lang/predicatePushdown.sql.
A different query plan appears to be chosen.
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12597770#action_12597770 ]
Bryan Pendleton commented on DERBY-3097:
----------------------------------------
Regarding my 7-Oct-2007 request for more information on Beetle 4880, Myrna van Lunteren
posted the following to the derby-dev mailing list:
------------
NEST_THIS definition:
public static int NEST_THIS( int returnValue,
String statement1 )
throws SQLException, StandardException
{
Connection conn = getCurrentConnection();
PreparedStatement ps = conn.prepareStatement( statement1 );
ps.execute();
return returnValue;
}
ij> create table L_1
(
pk int primary key,
s1 int,
b int,
s2 int,
s3 int
);
0 rows inserted/updated/deleted
ij> create unique btree index L_1_UNIQUE on L_1( s1, s2, s3 );
0 rows inserted/updated/deleted
ij> insert into L_1 ( pk, s1 ) values
( -16, NEST_THIS( 103, 'delete from L_1 properties index=null
where s2 = 103 and s1 =
NEST_THIS( 103, ''insert into L_1 ( pk, s1, s2, s3 ) values
( -18, 3, 1, 1 ), ( -19, 403, 403, 403 )'' )' ) );
-------------------------------
The info further indicates that at one time, this code caused a deadlock.
The notes say: "Consider a query like: select * from t1, t2 where
t2.c1 = MyClass::myMethod(t1.c1). Since the method call's arguments
have no local reference to t2, and the predicate is pushed down as a
qualifier, we should be able to deem it as scan-invariant and evaluate
it once per outer row, so all together row_count(t1) number of times.
And we should evaluate it outside the store, before a heap page is
latched."
I've not tried this code out; not sure the syntax even works in Derby.
I hope it helps.
> 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.1.3
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
> Attachments: checkNumOpens.diff
>
>
> 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.
[jira] Updated: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Pendleton updated DERBY-3097:
-----------------------------------
Attachment: checkNumOpens.diff
Attached is checkNumOpens.diff, a patch proposal.
This patch:
- removes the defensive "if" statement from getColumnFromRow
- removes the code which caught and recovered from the NPE
in TableScanResultSet.printPosition, and replaces it with code
which checks numOpens == 0 and uses that to decide if the
positioning information should be printed or not.
- adds the script from Beetle 4736 as a new regression case in
subqueryFlattening.sql
- updates predicatePushdown.out to reflect the new positioning output.
I think this patch is worthy of review; any feedback would be welcome.
> 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
> Attachments: checkNumOpens.diff
>
>
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12533017 ]
Bryan Pendleton commented on DERBY-3097:
----------------------------------------
Does anyone have access to the old Cloudscape test script for "Beetle 4880"?
I'd like to see if this test sheds any light on the BaseActivation.getColumnFromRecord code,
and possibly add the test case to our current regression suite.
> 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.
[jira] Updated: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Pendleton updated DERBY-3097:
-----------------------------------
Attachment: updatedJune2008.diff
Attached 'updatedJune2008.diff' patch proposal has been
updated to the head of trunk, and reflects that predicatePushdown.out
no longer needs to be modified by this patch.
The patch passes the regression tests, so I intend to move ahead
with commit.
Knut, thanks very much for the work on trying to convert the
old Cloudscape test code to Derby syntax.
> 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.1.3
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
> Attachments: checkNumOpens.diff, updatedJune2008.diff
>
>
> 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.
[jira] Commented: (DERBY-3097) Unnecessary if statement can be
removed from BaseActivation.getColumnFromNow
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12601400#action_12601400 ]
Bryan Pendleton commented on DERBY-3097:
----------------------------------------
I'm intending to commit this change to the trunk, as I still think this "if" statement
is unnecessary and can sometimes hide other problems elsewhere in the code.
I'll need to bring the patch up to date and re-verify that the tests pass, then if all
goes well I'll commit it to the trunk.
> 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.1.3
> Reporter: Bryan Pendleton
> Assignee: Bryan Pendleton
> Priority: Minor
> Attachments: checkNumOpens.diff
>
>
> 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.