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 David Van Couvering <Da...@Sun.COM> on 2006/08/02 01:30:34 UTC
Unexplained diff in procedureInTrigger.sql for JDK 1.6
Hi, all. In evaluating diffs currently happening in JDK 1.6 derbyall, I
ran across this strange diff:
398d397
< ERROR 38000: The exception 'java.sql.SQLException: The external
routine is not allowed to execute SQL statements.' was thrown while
evaluating an expression.
Basically what is happening is in JDK 1.6 Derby is not wrapping 38001
inside 38000. The reason? Well, in JDK 1.6, Derby public methods
throws SQLExceptions that are *not* a subclass of EmbedSQLException.
This results in different logic in the method
StandardException.unexpectedUserException():
public static StandardException unexpectedUserException(Throwable t)
{
/*
** If we have a SQLException that isn't a Util
** (i.e. it didn't come from cloudscape), then we check
** to see if it is a valid user defined exception range
** (38001-38XXX). If so, then we convert it into a
** StandardException without further ado.
*/
if ((t instanceof SQLException) &&
!(t instanceof EmbedSQLException))
{
SQLException sqlex = (SQLException)t;
String state = sqlex.getSQLState();
if ((state != null) &&
(state.length() == 5) &&
state.startsWith("38") &&
!state.equals("38000"))
{
StandardException se = new StandardException(state, sqlex.getMessage());
if (sqlex.getNextException() != null)
{
se.setNestedException(sqlex.getNextException());
}
return se;
}
}
So, my question is, should I codify this by creating a jdk16 master file
for procedureInTrigger.sql? Or is this something we need to try and
address?
What also is curious is that this should have shown up elsewhere. I'm
noticing a lot of other JDK 1.6-specific master files -- maybe this is
part of it?
Thanks,
David
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by David Van Couvering <Da...@Sun.COM>.
The harm is that each developer has to sift through a diff for a known
bug. If we all did this then we would have a stack of diffs and we'd
have to do pattern recognition to know if our change caused a regression
or not. That has me concerned... That's why I thought I'd log the bug,
since it's not a major issue, and note that a canon file is masking the
bug for now.
I think there are a whole class of master file outputs that are there
because of bugs. I think I even fixed a bug for 10.1.3 that you
reported which had its output encoded in a master file.
David
Daniel John Debrunner wrote:
> David Van Couvering wrote:
>
>> Now that I think of it further, I suspect it is not good practice to
>> hold a test hostage waiting for this bug to be fixed, and I should
>> probably add a jdk16-specific master file for procedureInTrigger.sql.
>>
>> I can point the reader of the bug to this master file as a guide to
>> reproducing the problem...
>>
>> Any thoughts? Otherwise I'll go ahead and do that...
>
> Little nervous. Especially as your comment says "so at least derbyall
> can pass". With derbyall "passing" a release could go ahead, having
> derbyall failing might hold up a release, but I can't see anyone making
> DERBY-1629 blocker or critical. Maybe the fact it is marked as a
> regression is enough.
>
> Is there any harm in having this test continue fail due to a real bug?
>
>
> Dan.
>
>
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by David Van Couvering <Da...@Sun.COM>.
Thanks for the tips, Mike. I think I'd like to go for adjusting the
output rather than comment out the statement or pull the test, since it
is specific to JDK 1.6.
David
Mike Matrigali wrote:
>
>
> Daniel John Debrunner wrote:
>> David Van Couvering wrote:
>>
>>
>>> Now that I think of it further, I suspect it is not good practice to
>>> hold a test hostage waiting for this bug to be fixed, and I should
>>> probably add a jdk16-specific master file for procedureInTrigger.sql.
>>>
>>> I can point the reader of the bug to this master file as a guide to
>>> reproducing the problem...
>>>
>>> Any thoughts? Otherwise I'll go ahead and do that...
>>
>>
>> Little nervous. Especially as your comment says "so at least derbyall
>> can pass". With derbyall "passing" a release could go ahead, having
>> derbyall failing might hold up a release, but I can't see anyone making
>> DERBY-1629 blocker or critical. Maybe the fact it is marked as a
>> regression is enough.
>>
>> Is there any harm in having this test continue fail due to a real bug?
>
> In general I think it is a bad idea to leave "expected" diffs in the
> regression suite. I think it causes unnecessary work for all the
> developers trying to figure expected vs. "real" diffs. I think that
> filing a bug and marking it a regression should be enough.
>
> I am not as clear on the right way to "make it pass". I am
> uncomfortable with just creating new master. In the past we have
> sometimes just removed the test from the suite until the bug is
> fixed - which means less testing. Sometimes we have just removed
> the offending statement until the bug is fixed. And other times
> with the canon based tests we have added comments to the test
> output itself saying something like "the next few lines are wrong
> due to the DERBY-xxx" and then updated that master - that has the
> benefit of making it very clear when the bug is fixed and a new
> diff appears why it is happening.
>
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by Mike Matrigali <mi...@sbcglobal.net>.
Daniel John Debrunner wrote:
> David Van Couvering wrote:
>
>
>>Now that I think of it further, I suspect it is not good practice to
>>hold a test hostage waiting for this bug to be fixed, and I should
>>probably add a jdk16-specific master file for procedureInTrigger.sql.
>>
>>I can point the reader of the bug to this master file as a guide to
>>reproducing the problem...
>>
>>Any thoughts? Otherwise I'll go ahead and do that...
>
>
> Little nervous. Especially as your comment says "so at least derbyall
> can pass". With derbyall "passing" a release could go ahead, having
> derbyall failing might hold up a release, but I can't see anyone making
> DERBY-1629 blocker or critical. Maybe the fact it is marked as a
> regression is enough.
>
> Is there any harm in having this test continue fail due to a real bug?
In general I think it is a bad idea to leave "expected" diffs in the
regression suite. I think it causes unnecessary work for all the
developers trying to figure expected vs. "real" diffs. I think that
filing a bug and marking it a regression should be enough.
I am not as clear on the right way to "make it pass". I am
uncomfortable with just creating new master. In the past we have
sometimes just removed the test from the suite until the bug is
fixed - which means less testing. Sometimes we have just removed
the offending statement until the bug is fixed. And other times
with the canon based tests we have added comments to the test
output itself saying something like "the next few lines are wrong
due to the DERBY-xxx" and then updated that master - that has the
benefit of making it very clear when the bug is fixed and a new
diff appears why it is happening.
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by Daniel John Debrunner <dj...@apache.org>.
David Van Couvering wrote:
> Now that I think of it further, I suspect it is not good practice to
> hold a test hostage waiting for this bug to be fixed, and I should
> probably add a jdk16-specific master file for procedureInTrigger.sql.
>
> I can point the reader of the bug to this master file as a guide to
> reproducing the problem...
>
> Any thoughts? Otherwise I'll go ahead and do that...
Little nervous. Especially as your comment says "so at least derbyall
can pass". With derbyall "passing" a release could go ahead, having
derbyall failing might hold up a release, but I can't see anyone making
DERBY-1629 blocker or critical. Maybe the fact it is marked as a
regression is enough.
Is there any harm in having this test continue fail due to a real bug?
Dan.
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by David Van Couvering <Da...@Sun.COM>.
Now that I think of it further, I suspect it is not good practice to
hold a test hostage waiting for this bug to be fixed, and I should
probably add a jdk16-specific master file for procedureInTrigger.sql.
I can point the reader of the bug to this master file as a guide to
reproducing the problem...
Any thoughts? Otherwise I'll go ahead and do that...
David
David Van Couvering wrote:
> Thanks, Dan, I concur, and thanks for your quick response. I am going
> to log it as a bug and not create a jdk16-specific master file for
> procedureInTrigger.
>
> I suspect when this bug is fixed a lot of jdk16-specific master files
> will be out-of-date. Personally, I am tired of fixing master files, as
> the SQLException itch is turning into a bit of a rash :), so you may not
> see my jump on fixing this bug...
>
> David
>
> Daniel John Debrunner wrote:
>> David Van Couvering wrote:
>>> Hi, all. In evaluating diffs currently happening in JDK 1.6 derbyall, I
>>> ran across this strange diff:
>>>
>>> 398d397
>>> < ERROR 38000: The exception 'java.sql.SQLException: The external
>>> routine is not allowed to execute SQL statements.' was thrown while
>>> evaluating an expression.
>>>
>>> Basically what is happening is in JDK 1.6 Derby is not wrapping 38001
>>> inside 38000. The reason? Well, in JDK 1.6, Derby public methods
>>> throws SQLExceptions that are *not* a subclass of EmbedSQLException.
>>>
>>> This results in different logic in the method
>>> StandardException.unexpectedUserException():
>>>
>>> public static StandardException unexpectedUserException(Throwable t)
>>> {
>>> /*
>>> ** If we have a SQLException that isn't a Util
>>> ** (i.e. it didn't come from cloudscape), then we check
>>> ** to see if it is a valid user defined exception range
>>> ** (38001-38XXX). If so, then we convert it into a
>>> ** StandardException without further ado.
>>> */
>>> if ((t instanceof SQLException) &&
>>> !(t instanceof EmbedSQLException))
>>> {
>>> SQLException sqlex = (SQLException)t;
>>> String state = sqlex.getSQLState();
>>> if ((state != null) &&
>>> (state.length() == 5) &&
>>> state.startsWith("38") &&
>>> !state.equals("38000"))
>>> {
>>> StandardException se = new StandardException(state,
>>> sqlex.getMessage());
>>> if (sqlex.getNextException() != null)
>>> {
>>> se.setNestedException(sqlex.getNextException());
>>> }
>>> return se;
>>> }
>>> }
>>>
>>> So, my question is, should I codify this by creating a jdk16 master file
>>> for procedureInTrigger.sql? Or is this something we need to try and
>>> address?
>>
>> I think that's a bug. The comment indicates the code is determining if
>> the error was raised by Derby (nee Cloudscape), if so handle it
>> according to the SQL standard. The check for EmbedSQLException is no
>> longer sufficient with the JDBC 4 changes.
>>
>>
>> Dan.
>>
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by David Van Couvering <Da...@Sun.COM>.
Thanks, Dan, I concur, and thanks for your quick response. I am going
to log it as a bug and not create a jdk16-specific master file for
procedureInTrigger.
I suspect when this bug is fixed a lot of jdk16-specific master files
will be out-of-date. Personally, I am tired of fixing master files, as
the SQLException itch is turning into a bit of a rash :), so you may not
see my jump on fixing this bug...
David
Daniel John Debrunner wrote:
> David Van Couvering wrote:
>> Hi, all. In evaluating diffs currently happening in JDK 1.6 derbyall, I
>> ran across this strange diff:
>>
>> 398d397
>> < ERROR 38000: The exception 'java.sql.SQLException: The external
>> routine is not allowed to execute SQL statements.' was thrown while
>> evaluating an expression.
>>
>> Basically what is happening is in JDK 1.6 Derby is not wrapping 38001
>> inside 38000. The reason? Well, in JDK 1.6, Derby public methods
>> throws SQLExceptions that are *not* a subclass of EmbedSQLException.
>>
>> This results in different logic in the method
>> StandardException.unexpectedUserException():
>>
>> public static StandardException unexpectedUserException(Throwable t)
>> {
>> /*
>> ** If we have a SQLException that isn't a Util
>> ** (i.e. it didn't come from cloudscape), then we check
>> ** to see if it is a valid user defined exception range
>> ** (38001-38XXX). If so, then we convert it into a
>> ** StandardException without further ado.
>> */
>> if ((t instanceof SQLException) &&
>> !(t instanceof EmbedSQLException))
>> {
>> SQLException sqlex = (SQLException)t;
>> String state = sqlex.getSQLState();
>> if ((state != null) &&
>> (state.length() == 5) &&
>> state.startsWith("38") &&
>> !state.equals("38000"))
>> {
>> StandardException se = new StandardException(state,
>> sqlex.getMessage());
>> if (sqlex.getNextException() != null)
>> {
>> se.setNestedException(sqlex.getNextException());
>> }
>> return se;
>> }
>> }
>>
>> So, my question is, should I codify this by creating a jdk16 master file
>> for procedureInTrigger.sql? Or is this something we need to try and
>> address?
>
> I think that's a bug. The comment indicates the code is determining if
> the error was raised by Derby (nee Cloudscape), if so handle it
> according to the SQL standard. The check for EmbedSQLException is no
> longer sufficient with the JDBC 4 changes.
>
>
> Dan.
>
Re: Unexplained diff in procedureInTrigger.sql for JDK 1.6
Posted by Daniel John Debrunner <dj...@apache.org>.
David Van Couvering wrote:
> Hi, all. In evaluating diffs currently happening in JDK 1.6 derbyall, I
> ran across this strange diff:
>
> 398d397
> < ERROR 38000: The exception 'java.sql.SQLException: The external
> routine is not allowed to execute SQL statements.' was thrown while
> evaluating an expression.
>
> Basically what is happening is in JDK 1.6 Derby is not wrapping 38001
> inside 38000. The reason? Well, in JDK 1.6, Derby public methods
> throws SQLExceptions that are *not* a subclass of EmbedSQLException.
>
> This results in different logic in the method
> StandardException.unexpectedUserException():
>
> public static StandardException unexpectedUserException(Throwable t)
> {
> /*
> ** If we have a SQLException that isn't a Util
> ** (i.e. it didn't come from cloudscape), then we check
> ** to see if it is a valid user defined exception range
> ** (38001-38XXX). If so, then we convert it into a
> ** StandardException without further ado.
> */
> if ((t instanceof SQLException) &&
> !(t instanceof EmbedSQLException))
> {
> SQLException sqlex = (SQLException)t;
> String state = sqlex.getSQLState();
> if ((state != null) &&
> (state.length() == 5) &&
> state.startsWith("38") &&
> !state.equals("38000"))
> {
> StandardException se = new StandardException(state,
> sqlex.getMessage());
> if (sqlex.getNextException() != null)
> {
> se.setNestedException(sqlex.getNextException());
> }
> return se;
> }
> }
>
> So, my question is, should I codify this by creating a jdk16 master file
> for procedureInTrigger.sql? Or is this something we need to try and
> address?
I think that's a bug. The comment indicates the code is determining if
the error was raised by Derby (nee Cloudscape), if so handle it
according to the SQL standard. The check for EmbedSQLException is no
longer sufficient with the JDBC 4 changes.
Dan.