QueryException truncates chained SQLExceptions #1007

Closed
cowwoc opened this Issue Oct 14, 2014 · 27 comments

Comments

Projects
None yet
3 participants
@cowwoc
Contributor

cowwoc commented Oct 14, 2014

In the case of batch queries, SQLException may reference one or more exceptions behind SQLException.getNextException() or SQLException.iterator(). Because SQLException.getCause() is not set, QueryException.printStackTrace() truncates this information. See https://groups.google.com/d/msg/h2-database/OrCkGSpe2BQ/bTGCaAbGixgJ for an example of what the output looks like.

I think we should try converting SQLException.getNextException() to SQLException.addSuppressed(). I suspect (though this needs to be tested) that QueryException.printStackTrace() would then work as expected.

I propose the following:

  1. If QueryException's constructor is passed a SQLException, then iterate through the chained exceptions and addSuppressed() on any exception that isn't already returned by getSuppressed().
  2. Create a testcase to ensure that QueryException.printStackTrace() prints these chained stack-traces in full.
@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

Suppressed Exceptions are something entirely different.
http://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#addSuppressed(java.lang.Throwable) (updated the correct link)
They were introduced in Java 7 and it's try-with-resource syntax.

An exception chain combines exceptions in a causal relationship.
A suppressed exception is still part of the exception 'chain', it is still exceptional behaviour, but it doesn't have a causal relationship to the root cause.

A causal chain should be enough for regular (logging, printStackStrace(), propagation) purposes.
getSuppressed() is only for programmatic access (typically after a try-with-resource statement) to the suppressed exceptions.

I'll look into enriching the causal chain.

Member

Shredder121 commented Oct 14, 2014

Suppressed Exceptions are something entirely different.
http://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#addSuppressed(java.lang.Throwable) (updated the correct link)
They were introduced in Java 7 and it's try-with-resource syntax.

An exception chain combines exceptions in a causal relationship.
A suppressed exception is still part of the exception 'chain', it is still exceptional behaviour, but it doesn't have a causal relationship to the root cause.

A causal chain should be enough for regular (logging, printStackStrace(), propagation) purposes.
getSuppressed() is only for programmatic access (typically after a try-with-resource statement) to the suppressed exceptions.

I'll look into enriching the causal chain.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

The only thing I would like to ask, is if you could provide a concrete test case that reflects your issue, so batching, and generating an SQLException that doesn't provide its causal chain correctly.

Member

Shredder121 commented Oct 14, 2014

The only thing I would like to ask, is if you could provide a concrete test case that reflects your issue, so batching, and generating an SQLException that doesn't provide its causal chain correctly.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 14, 2014

Contributor

@Shredder121 addSuppressed() works for multiple parallel exceptions. getCause() will only work for a single exception. If a batch update fails, you might have multiple/different exceptions.

Contributor

cowwoc commented Oct 14, 2014

@Shredder121 addSuppressed() works for multiple parallel exceptions. getCause() will only work for a single exception. If a batch update fails, you might have multiple/different exceptions.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 14, 2014

Contributor

@Shredder121 I don't have a testcase skeleton handy. If you do, please let me know and I'll edit it to demonstrate this issue. You should be able to reproduce the problem by doing the following:

  1. Using H2, create a table and insert a few rows into it.
  2. Register a BEFORE UPDATE trigger against the table.
  3. Fire a batch update against the table, where the trigger throws an exception trying to cast Short to an Integer (uppercase types). This will throw a ClassCastException which will propagate up to the batch update in the form of a chained exception instead of a casual link.
Contributor

cowwoc commented Oct 14, 2014

@Shredder121 I don't have a testcase skeleton handy. If you do, please let me know and I'll edit it to demonstrate this issue. You should be able to reproduce the problem by doing the following:

  1. Using H2, create a table and insert a few rows into it.
  2. Register a BEFORE UPDATE trigger against the table.
  3. Fire a batch update against the table, where the trigger throws an exception trying to cast Short to an Integer (uppercase types). This will throw a ClassCastException which will propagate up to the batch update in the form of a chained exception instead of a casual link.
@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Oct 14, 2014

Member

The DefaultSQLExceptionTranslator might be a better place to do this transformation. I am though not yet completely sure if it is semantically correct to transform the SQLException chain into suppressed exceptions. I can confirm though that suppressed exceptions are included in the stacktrace.

Member

timowest commented Oct 14, 2014

The DefaultSQLExceptionTranslator might be a better place to do this transformation. I am though not yet completely sure if it is semantically correct to transform the SQLException chain into suppressed exceptions. I can confirm though that suppressed exceptions are included in the stacktrace.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 14, 2014

Contributor

@timowest I agree that it's not clear whether it is semantically correct, but practically speaking I can think of no other way to prevent chained exceptions from being truncated.

The following works for me:

/**
 * Converts an exception chain to {@code getCause()} (for one exception) or {@code getSuppress()} for multiple
 * exceptions.
 * <p>
 * @param e the exception to process
 * @return the resulting exception
 */
private static SQLException convertExceptionChain(SQLException e)
{
    SQLException next = e.getNextException();
    if (next == null)
        return e;
    List<SQLException> chainedExceptions = new ArrayList<>(1);
    do
    {
        chainedExceptions.add(next);
        next = next.getNextException();
    }
    while (next != null);

    // Use getCause() for one exception and addSuppressed() for multiple exceptions (e.g. batch updates).
    if (chainedExceptions.size() == 1 && e.getCause() == null)
        return (SQLException) e.initCause(convertExceptionChain(chainedExceptions.get(0)));
    Set<Throwable> existingExceptions = new HashSet<>(Arrays.asList(e.getSuppressed()));
    chainedExceptions.removeAll(existingExceptions);
    for (Throwable chained: chainedExceptions)
        e.addSuppressed(chained);
    return e;
}
Contributor

cowwoc commented Oct 14, 2014

@timowest I agree that it's not clear whether it is semantically correct, but practically speaking I can think of no other way to prevent chained exceptions from being truncated.

The following works for me:

/**
 * Converts an exception chain to {@code getCause()} (for one exception) or {@code getSuppress()} for multiple
 * exceptions.
 * <p>
 * @param e the exception to process
 * @return the resulting exception
 */
private static SQLException convertExceptionChain(SQLException e)
{
    SQLException next = e.getNextException();
    if (next == null)
        return e;
    List<SQLException> chainedExceptions = new ArrayList<>(1);
    do
    {
        chainedExceptions.add(next);
        next = next.getNextException();
    }
    while (next != null);

    // Use getCause() for one exception and addSuppressed() for multiple exceptions (e.g. batch updates).
    if (chainedExceptions.size() == 1 && e.getCause() == null)
        return (SQLException) e.initCause(convertExceptionChain(chainedExceptions.get(0)));
    Set<Throwable> existingExceptions = new HashSet<>(Arrays.asList(e.getSuppressed()));
    chainedExceptions.removeAll(existingExceptions);
    for (Throwable chained: chainedExceptions)
        e.addSuppressed(chained);
    return e;
}
@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Oct 14, 2014

Member

I'd transform the full SQLException chain into suppressed exceptions, since they are independent and not in a causal relationship.

Member

timowest commented Oct 14, 2014

I'd transform the full SQLException chain into suppressed exceptions, since they are independent and not in a causal relationship.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

@timowest Yes, indeed, I thought of the DefaultSQLExceptionTranslator as well, although there are some notes.

The problem with suppressed exceptions is, that it's introduced in Java 7, this means that support for Java 6 must be dropped for the SQL module.

What I was thinking was inserting another initCause at the rootcause with the 'info level exception', with all the SQLException's exceptions.

That way it will be printed the same as the suppressed exceptions, but Java 6 compatible.

But if we want to leave Java 6 behind and use suppressed exceptions, that's your call.

Member

Shredder121 commented Oct 14, 2014

@timowest Yes, indeed, I thought of the DefaultSQLExceptionTranslator as well, although there are some notes.

The problem with suppressed exceptions is, that it's introduced in Java 7, this means that support for Java 6 must be dropped for the SQL module.

What I was thinking was inserting another initCause at the rootcause with the 'info level exception', with all the SQLException's exceptions.

That way it will be printed the same as the suppressed exceptions, but Java 6 compatible.

But if we want to leave Java 6 behind and use suppressed exceptions, that's your call.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Oct 14, 2014

Member

We don't build with JDK6 anymore, but users are relying on JDK6 support, and with a minor release JDK6 support shouldn't be dropped.

@Shredder121 Could you elaborate your idea with the additional root causes?

Member

timowest commented Oct 14, 2014

We don't build with JDK6 anymore, but users are relying on JDK6 support, and with a minor release JDK6 support shouldn't be dropped.

@Shredder121 Could you elaborate your idea with the additional root causes?

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

I was thinking of something like this:

    @Override
    public RuntimeException translate(String sql, List<Object> bindings, SQLException e) {
        if (!Iterables.isEmpty(e)) {
            Throwable rootCause = Throwables.getRootCause(e);
            rootCause.initCause(new WrappedSQLCauseException(e));
        }
        return new QueryException("Caught " + e.getClass().getSimpleName() + " for " + sql, e);
    }

That way, the last exception in the chain contains the additional information of the state that actually caused the exception, and since SQLException is Iterable, we could call toString() on the elements(which are probably SQLExceptions) and then we have a reasonable Exception message.

I think that because the exception is still of a causal nature, the semantics aren't broken.

Member

Shredder121 commented Oct 14, 2014

I was thinking of something like this:

    @Override
    public RuntimeException translate(String sql, List<Object> bindings, SQLException e) {
        if (!Iterables.isEmpty(e)) {
            Throwable rootCause = Throwables.getRootCause(e);
            rootCause.initCause(new WrappedSQLCauseException(e));
        }
        return new QueryException("Caught " + e.getClass().getSimpleName() + " for " + sql, e);
    }

That way, the last exception in the chain contains the additional information of the state that actually caused the exception, and since SQLException is Iterable, we could call toString() on the elements(which are probably SQLExceptions) and then we have a reasonable Exception message.

I think that because the exception is still of a causal nature, the semantics aren't broken.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

The processing in our exception wrapper can be something like this: https://db.apache.org/derby/docs/10.3/devguide/rdevprocsqle.html

Except for the sysout statements and printStackTrace, but the toString() of our exception can do something that looks like this.
Think of a Collection of Exception messages like these.

Member

Shredder121 commented Oct 14, 2014

The processing in our exception wrapper can be something like this: https://db.apache.org/derby/docs/10.3/devguide/rdevprocsqle.html

Except for the sysout statements and printStackTrace, but the toString() of our exception can do something that looks like this.
Think of a Collection of Exception messages like these.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 14, 2014

Contributor

@Shredder121 https://db.apache.org/derby/docs/10.3/devguide/rdevprocsqle.html won't work for you. Overriding printStackTrace() is one thing. Making toString() work properly is a different matter.

My understanding is that SQLException.toString() truncates chained exceptions, which is why we're transforming them into suppressed exceptions in the first place.

The only way to really see what I'm talking about is running a testcase against some real exceptions. The code sniplet I attached has been well tested for my specific use-cases. If you want to convert all exceptions to getSuppressed() then simply remove:

if (chainedExceptions.size() == 1 && e.getCause() == null)
        return (SQLException) e.initCause(convertExceptionChain(chainedExceptions.get(0)));

from the code sniplet.

If you want to make this work for JDK6, I think your only option is overriding toString() but it's going to be hard work. Usually toString() of the current exception delegates to toString() of nested exceptions, but you can't do that for SQLException because it does the wrong thing. The easiest approach is to drop JDK6 support but obviously that is questionable for such a small feature request.

Contributor

cowwoc commented Oct 14, 2014

@Shredder121 https://db.apache.org/derby/docs/10.3/devguide/rdevprocsqle.html won't work for you. Overriding printStackTrace() is one thing. Making toString() work properly is a different matter.

My understanding is that SQLException.toString() truncates chained exceptions, which is why we're transforming them into suppressed exceptions in the first place.

The only way to really see what I'm talking about is running a testcase against some real exceptions. The code sniplet I attached has been well tested for my specific use-cases. If you want to convert all exceptions to getSuppressed() then simply remove:

if (chainedExceptions.size() == 1 && e.getCause() == null)
        return (SQLException) e.initCause(convertExceptionChain(chainedExceptions.get(0)));

from the code sniplet.

If you want to make this work for JDK6, I think your only option is overriding toString() but it's going to be hard work. Usually toString() of the current exception delegates to toString() of nested exceptions, but you can't do that for SQLException because it does the wrong thing. The easiest approach is to drop JDK6 support but obviously that is questionable for such a small feature request.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 14, 2014

Contributor

@Shredder121 A word of warning: avoid SQLException.iterator() if you can. If you take a look at the implementation you'll notice that Iterables.isEmpty(e) will always return false. That method is very confusing so I recommend sticking to SQLException.getNextException().

Contributor

cowwoc commented Oct 14, 2014

@Shredder121 A word of warning: avoid SQLException.iterator() if you can. If you take a look at the implementation you'll notice that Iterables.isEmpty(e) will always return false. That method is very confusing so I recommend sticking to SQLException.getNextException().

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

@cowwoc why would it always return false?

edit:
I think I see what you mean.
That emptycheck was only the sensible thing to do at the time, since an empty Exception says nothing.

So the empty check can be omitted, nice catch!

Member

Shredder121 commented Oct 14, 2014

@cowwoc why would it always return false?

edit:
I think I see what you mean.
That emptycheck was only the sensible thing to do at the time, since an empty Exception says nothing.

So the empty check can be omitted, nice catch!

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 14, 2014

Member

We are not overriding printStackTrace(), we are not overriding toString() either.
Both of those are too much work to get right, while the default implementation is good enough to work for us.
Exceptions can pass their detail message to the superclass, which is what I thought would be good.

What I have right now, is this:

    @Override
    public RuntimeException translate(String sql, List<Object> bindings, SQLException e) {
        if (!Iterables.isEmpty(e)) {
            wrap(e);
        }
        return new QueryException("Caught " + e.getClass().getSimpleName() + " for " + sql, e);
    }

    private void wrap(SQLException exception) {
        Throwable rootCause = Throwables.getRootCause(exception);
        rootCause.initCause(new WrappedSQLCauseException(exception));
    }

    private static class WrappedSQLCauseException extends Exception {

        private static final long serialVersionUID = 1L;

        public WrappedSQLCauseException(Iterable<Throwable> nextExceptions) {
            super("SQLException details:\n"
                    + Iterables.transform(nextExceptions, exceptionMessageFunction));
        }

    }
    private static final Function<Throwable, String> exceptionMessageFunction = new Function<Throwable, String>() {

        @Override
        public String apply(Throwable input) {
            if (input instanceof SQLException) {
                SQLException sqle = (SQLException) input;
                return "SQLState: " + sqle.getSQLState()
                        + "Severity: " + sqle.getErrorCode()
                        + "Message: " + sqle.getMessage();
            }
            return input.toString();
        }
    };

This should link a last exception cause with the state returned from all exceptions that are contained in the SQLException.

It is still a work in progress, but personally I don't want to drop Java 6 support, so I just wanted to show what I had to avoid further confusion.

Member

Shredder121 commented Oct 14, 2014

We are not overriding printStackTrace(), we are not overriding toString() either.
Both of those are too much work to get right, while the default implementation is good enough to work for us.
Exceptions can pass their detail message to the superclass, which is what I thought would be good.

What I have right now, is this:

    @Override
    public RuntimeException translate(String sql, List<Object> bindings, SQLException e) {
        if (!Iterables.isEmpty(e)) {
            wrap(e);
        }
        return new QueryException("Caught " + e.getClass().getSimpleName() + " for " + sql, e);
    }

    private void wrap(SQLException exception) {
        Throwable rootCause = Throwables.getRootCause(exception);
        rootCause.initCause(new WrappedSQLCauseException(exception));
    }

    private static class WrappedSQLCauseException extends Exception {

        private static final long serialVersionUID = 1L;

        public WrappedSQLCauseException(Iterable<Throwable> nextExceptions) {
            super("SQLException details:\n"
                    + Iterables.transform(nextExceptions, exceptionMessageFunction));
        }

    }
    private static final Function<Throwable, String> exceptionMessageFunction = new Function<Throwable, String>() {

        @Override
        public String apply(Throwable input) {
            if (input instanceof SQLException) {
                SQLException sqle = (SQLException) input;
                return "SQLState: " + sqle.getSQLState()
                        + "Severity: " + sqle.getErrorCode()
                        + "Message: " + sqle.getMessage();
            }
            return input.toString();
        }
    };

This should link a last exception cause with the state returned from all exceptions that are contained in the SQLException.

It is still a work in progress, but personally I don't want to drop Java 6 support, so I just wanted to show what I had to avoid further confusion.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 14, 2014

Contributor

@Shredder121 SQLException.iterator() contains the following elements:

[this, firstException, firstException.getCause(), nextException, nextException.getCause(), etc..]

This is problematic for two reasons:

  1. It always contains this.
  2. If you rely on this list for exceptions you will end up printing the stack-trace of firstException.getCause() twice (once when you firstException.printStackTrace(), once when you firstException.getCause().printStackTrace()).

Regarding your proposal, I see two problems:

  1. It only supports a single exception (this is a non-starter for batch operations).
  2. It fails to provide a stack-trace per exception (which is important because we could be getting different exceptions occurring on different file/line numbers per statement).

Is it possible to conditionally provide improved functionality for end-users of JDK7 and up? Meaning, JDK6 users would keep the old behavior, while JDK7 users would get the getSuppressed() behavior.

Contributor

cowwoc commented Oct 14, 2014

@Shredder121 SQLException.iterator() contains the following elements:

[this, firstException, firstException.getCause(), nextException, nextException.getCause(), etc..]

This is problematic for two reasons:

  1. It always contains this.
  2. If you rely on this list for exceptions you will end up printing the stack-trace of firstException.getCause() twice (once when you firstException.printStackTrace(), once when you firstException.getCause().printStackTrace()).

Regarding your proposal, I see two problems:

  1. It only supports a single exception (this is a non-starter for batch operations).
  2. It fails to provide a stack-trace per exception (which is important because we could be getting different exceptions occurring on different file/line numbers per statement).

Is it possible to conditionally provide improved functionality for end-users of JDK7 and up? Meaning, JDK6 users would keep the old behavior, while JDK7 users would get the getSuppressed() behavior.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 15, 2014

Member

I'd like to point out that the iterator only contains 'this' once.
So the correct sequence would be:
[firstException(a.k.a. this), firstException.getCause(), nextException, nextException.getCause(), ...]
This means we could skip the first two elements, since they are already present in the original stacktrace(after that the list could hold no more elements, and be empty).

As for your problems, you are right, it supports only a single exception.

It only supports a single exception (this is a non-starter for batch operations).

http://docs.oracle.com/javase/7/docs/api/java/sql/BatchUpdateException.html

Drivers are free to continue operation, so in a batch of 100, if 10 fail, that would mean about 3 exceptions in the chain, and 18 others in our custom exception(at the end of the chain), or 18 exceptions as suppressed exceptions.
Making this readable is not an easy feat, printStackTrace() wouldn't do the job.
Maybe the stacktrace is necessary, maybe only the message and error code.
I think that a test case would be best to see what works out, before jumping to conclusions.

It fails to provide a stack-trace per exception (which is important because we could be getting different exceptions occurring on different file/line numbers per statement).

In my opinion, when you are at the database level, and getting vendor specific error codes, you are far out of the JVM to care for stacktraces(they will probably not give you any useful information).
But the same goes for this point, I don't want to make early conclusions from speculation and assumptions.

I'll see how I can prepare your test case, I am currently very busy with other things.

Member

Shredder121 commented Oct 15, 2014

I'd like to point out that the iterator only contains 'this' once.
So the correct sequence would be:
[firstException(a.k.a. this), firstException.getCause(), nextException, nextException.getCause(), ...]
This means we could skip the first two elements, since they are already present in the original stacktrace(after that the list could hold no more elements, and be empty).

As for your problems, you are right, it supports only a single exception.

It only supports a single exception (this is a non-starter for batch operations).

http://docs.oracle.com/javase/7/docs/api/java/sql/BatchUpdateException.html

Drivers are free to continue operation, so in a batch of 100, if 10 fail, that would mean about 3 exceptions in the chain, and 18 others in our custom exception(at the end of the chain), or 18 exceptions as suppressed exceptions.
Making this readable is not an easy feat, printStackTrace() wouldn't do the job.
Maybe the stacktrace is necessary, maybe only the message and error code.
I think that a test case would be best to see what works out, before jumping to conclusions.

It fails to provide a stack-trace per exception (which is important because we could be getting different exceptions occurring on different file/line numbers per statement).

In my opinion, when you are at the database level, and getting vendor specific error codes, you are far out of the JVM to care for stacktraces(they will probably not give you any useful information).
But the same goes for this point, I don't want to make early conclusions from speculation and assumptions.

I'll see how I can prepare your test case, I am currently very busy with other things.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 15, 2014

Member

Yes, we can conditionally provide improved functionality.
(At least, I think it would work, looking at the spec. as long as the bytecode verifier doesn't kick in)

Nested classes (static inner class) aren't loaded until needed, so we could provide a holder class with logic for jdk6 with the root link exception, and a holder class with logic for jdk7 and up with suppressed exceptions.

Member

Shredder121 commented Oct 15, 2014

Yes, we can conditionally provide improved functionality.
(At least, I think it would work, looking at the spec. as long as the bytecode verifier doesn't kick in)

Nested classes (static inner class) aren't loaded until needed, so we could provide a holder class with logic for jdk6 with the root link exception, and a holder class with logic for jdk7 and up with suppressed exceptions.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 15, 2014

Contributor

@Shredder121 You are right about the sequence returned by iterator() but I still don't think we want to process nextException.getCause() since it's already included in the stack-trace of nextException.

In my opinion, when you are at the database level, and getting vendor specific error codes, you are far out of the JVM to care for stacktraces(they will probably not give you any useful information).
But the same goes for this point, I don't want to make early conclusions from speculation and assumptions.

Agreed (on not jumping to conclusions).

In the case of H2, I'd really want to get those stack-traces. Because H2 and user code (stored procedures) are written in Java, stack-traces help me track problems in the database itself and user code. I've never used Oracle or PostgresSQL's Java binding but it's my understanding that they support Java triggers as well (so getting stack-traces might be useful there too).

Whenever you get around to putting together a testcase, I recommend examining the stack-trace you get for a batch update before/after using my code sniplet. You should see a very noticeable benefit with H2, and I'm hoping the same is true for other databases.

Regarding providing separate implementations for JDK6/7, let's hope that works. One way that would work for sure (but is worse from a deployment point of view) is shipping a separate JDK6-specific "compatibility" module and having users include it if they need such support.

Contributor

cowwoc commented Oct 15, 2014

@Shredder121 You are right about the sequence returned by iterator() but I still don't think we want to process nextException.getCause() since it's already included in the stack-trace of nextException.

In my opinion, when you are at the database level, and getting vendor specific error codes, you are far out of the JVM to care for stacktraces(they will probably not give you any useful information).
But the same goes for this point, I don't want to make early conclusions from speculation and assumptions.

Agreed (on not jumping to conclusions).

In the case of H2, I'd really want to get those stack-traces. Because H2 and user code (stored procedures) are written in Java, stack-traces help me track problems in the database itself and user code. I've never used Oracle or PostgresSQL's Java binding but it's my understanding that they support Java triggers as well (so getting stack-traces might be useful there too).

Whenever you get around to putting together a testcase, I recommend examining the stack-trace you get for a batch update before/after using my code sniplet. You should see a very noticeable benefit with H2, and I'm hoping the same is true for other databases.

Regarding providing separate implementations for JDK6/7, let's hope that works. One way that would work for sure (but is worse from a deployment point of view) is shipping a separate JDK6-specific "compatibility" module and having users include it if they need such support.

Shredder121 added a commit that referenced this issue Oct 16, 2014

Add additional SQLExceptions to the QueryException output
Fixes #1007

When JRE6 is used, it adds an extra Exception with additional information.
When JRE7 is used, it adds the extra SQLExceptions as suppressed exceptions.
@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 16, 2014

Member

After some research, I found out that SQLException's cause is already set, so an initcause at the root is not possible.
I now added a single exception above the SQLException with detailed information when Java 6 is used.

Note that Querydsl doesn't compile anymore (on JDK 6) without removing the Java 7 exceptionwrapper. We should probably include this in the documentation, or a wiki page.

Member

Shredder121 commented Oct 16, 2014

After some research, I found out that SQLException's cause is already set, so an initcause at the root is not possible.
I now added a single exception above the SQLException with detailed information when Java 6 is used.

Note that Querydsl doesn't compile anymore (on JDK 6) without removing the Java 7 exceptionwrapper. We should probably include this in the documentation, or a wiki page.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 16, 2014

Contributor

d3ffd3e looks good to me by visual inspection.

Contributor

cowwoc commented Oct 16, 2014

d3ffd3e looks good to me by visual inspection.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 17, 2014

Contributor

@Shredder121 At second glance, I think JavaSE6SQLExceptionWrapper fails to consider multiple chained exceptions. It appends information about each exception using exceptionMessageFunction but it fails to insert a newline between each invocation of the function.

Contributor

cowwoc commented Oct 17, 2014

@Shredder121 At second glance, I think JavaSE6SQLExceptionWrapper fails to consider multiple chained exceptions. It appends information about each exception using exceptionMessageFunction but it fails to insert a newline between each invocation of the function.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 17, 2014

Member

Hmm, yes that could be improved.

Also, maybe now you can add your test case to the test suite?

Member

Shredder121 commented Oct 17, 2014

Hmm, yes that could be improved.

Also, maybe now you can add your test case to the test suite?

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 17, 2014

Member

Thanks for your feedback.

For reference, for JDK6 it now looks like this:

com.mysema.query.QueryException: Caught JdbcBatchUpdateException for update SURVEY
set ID = ?
    at com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper.wrap(JavaSE6SQLExceptionWrapper.java:47)
    at com.mysema.query.sql.DefaultSQLExceptionTranslator.translate(DefaultSQLExceptionTranslator.java:48)
    at com.mysema.query.sql.Configuration.translate(Configuration.java:524)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:196)
    at com.mysema.query.AbstractBaseTest.execute(AbstractBaseTest.java:169)
    at com.mysema.query.suites.H2ExceptionSuiteTest.UpdateBatchFailed(H2ExceptionSuiteTest.java:33)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:24)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:136)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.createRequestAndRun(JUnitCoreWrapper.java:141)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.executeEager(JUnitCoreWrapper.java:114)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:86)
    at org.apache.maven.surefire.junitcore.JUnitCoreProvider.invoke(JUnitCoreProvider.java:134)
    at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
Caused by: com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper$WrappedSQLCauseException: Detailed SQLException information:
SQLState: 23505
ErrorCode: 23505
Message: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (1, 3)"; SQL statement:
update SURVEY
set ID = ? [23505-178]

    ... 40 more
Caused by: org.h2.jdbc.JdbcBatchUpdateException: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (1, 3)"; SQL statement:
update SURVEY
set ID = ? [23505-178]
    at org.h2.jdbc.JdbcPreparedStatement.executeBatch(JdbcPreparedStatement.java:1199)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:161)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:171)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:190)
    ... 36 more

The individual exceptions (which there are not that many of in this example) are separated by a blank line.

Any more suggestions/improvements?
There is of course a limit to how readable it can get, since it is actually not supported, and we are only emulating something similar.
But as long as it's as good as it gets, I'm happy.

Member

Shredder121 commented Oct 17, 2014

Thanks for your feedback.

For reference, for JDK6 it now looks like this:

com.mysema.query.QueryException: Caught JdbcBatchUpdateException for update SURVEY
set ID = ?
    at com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper.wrap(JavaSE6SQLExceptionWrapper.java:47)
    at com.mysema.query.sql.DefaultSQLExceptionTranslator.translate(DefaultSQLExceptionTranslator.java:48)
    at com.mysema.query.sql.Configuration.translate(Configuration.java:524)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:196)
    at com.mysema.query.AbstractBaseTest.execute(AbstractBaseTest.java:169)
    at com.mysema.query.suites.H2ExceptionSuiteTest.UpdateBatchFailed(H2ExceptionSuiteTest.java:33)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:24)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:136)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.createRequestAndRun(JUnitCoreWrapper.java:141)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.executeEager(JUnitCoreWrapper.java:114)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:86)
    at org.apache.maven.surefire.junitcore.JUnitCoreProvider.invoke(JUnitCoreProvider.java:134)
    at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
Caused by: com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper$WrappedSQLCauseException: Detailed SQLException information:
SQLState: 23505
ErrorCode: 23505
Message: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (1, 3)"; SQL statement:
update SURVEY
set ID = ? [23505-178]

    ... 40 more
Caused by: org.h2.jdbc.JdbcBatchUpdateException: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (1, 3)"; SQL statement:
update SURVEY
set ID = ? [23505-178]
    at org.h2.jdbc.JdbcPreparedStatement.executeBatch(JdbcPreparedStatement.java:1199)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:161)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:171)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:190)
    ... 36 more

The individual exceptions (which there are not that many of in this example) are separated by a blank line.

Any more suggestions/improvements?
There is of course a limit to how readable it can get, since it is actually not supported, and we are only emulating something similar.
But as long as it's as good as it gets, I'm happy.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 17, 2014

Contributor

Maybe I missed something, but looking at the previous comment I have two questions:

  1. The chained exception being wrapped has the same message as exception.getCause().getMessage()? I was expecting them to be two separate exceptions.
  2. It seems you only have one chained exception in the above example. Can you include an example with two chained exceptions?
Contributor

cowwoc commented Oct 17, 2014

Maybe I missed something, but looking at the previous comment I have two questions:

  1. The chained exception being wrapped has the same message as exception.getCause().getMessage()? I was expecting them to be two separate exceptions.
  2. It seems you only have one chained exception in the above example. Can you include an example with two chained exceptions?
@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Oct 17, 2014

Member

Yes, this exception's first linked exception is the same as the cause.

This is because the batch update's exception cause was the uniqueconstraint, but also the BATCH's linked exception was the contents of the batch, being only that particular exception.

So yeah, I actually want a better test case as well, could you maybe add a trigger in the @BeforeClass of H2ExceptionSuiteTest that reflects your use case?

I added a second batch, with output:

com.mysema.query.QueryException: Caught JdbcBatchUpdateException for update SURVEY
set ID = 2
    at com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper.wrap(JavaSE6SQLExceptionWrapper.java:47)
    at com.mysema.query.sql.DefaultSQLExceptionTranslator.translate(DefaultSQLExceptionTranslator.java:48)
    at com.mysema.query.sql.Configuration.translate(Configuration.java:524)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:196)
    at com.mysema.query.AbstractBaseTest.execute(AbstractBaseTest.java:169)
    at com.mysema.query.suites.H2ExceptionSuiteTest.UpdateBatchFailed2(H2ExceptionSuiteTest.java:48)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:24)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:136)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.createRequestAndRun(JUnitCoreWrapper.java:141)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.executeEager(JUnitCoreWrapper.java:114)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:86)
    at org.apache.maven.surefire.junitcore.JUnitCoreProvider.invoke(JUnitCoreProvider.java:134)
    at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
Caused by: com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper$WrappedSQLCauseException: Detailed SQLException information:
SQLState: 23505
ErrorCode: 23505
Message: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (3, 7)"; SQL statement:
update SURVEY
set ID = 3 [23505-178]

SQLState: 23505
ErrorCode: 23505
Message: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (3, 5)"; SQL statement:
update SURVEY
set ID = 3 [23505-178]

    ... 40 more
Caused by: org.h2.jdbc.JdbcBatchUpdateException: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (3, 7)"; SQL statement:
update SURVEY
set ID = 3 [23505-178]
    at org.h2.jdbc.JdbcPreparedStatement.executeBatch(JdbcPreparedStatement.java:1199)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:161)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:171)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:190)
    ... 36 more

Here the content of the batch is a bit more clear, is this okay?

Member

Shredder121 commented Oct 17, 2014

Yes, this exception's first linked exception is the same as the cause.

This is because the batch update's exception cause was the uniqueconstraint, but also the BATCH's linked exception was the contents of the batch, being only that particular exception.

So yeah, I actually want a better test case as well, could you maybe add a trigger in the @BeforeClass of H2ExceptionSuiteTest that reflects your use case?

I added a second batch, with output:

com.mysema.query.QueryException: Caught JdbcBatchUpdateException for update SURVEY
set ID = 2
    at com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper.wrap(JavaSE6SQLExceptionWrapper.java:47)
    at com.mysema.query.sql.DefaultSQLExceptionTranslator.translate(DefaultSQLExceptionTranslator.java:48)
    at com.mysema.query.sql.Configuration.translate(Configuration.java:524)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:196)
    at com.mysema.query.AbstractBaseTest.execute(AbstractBaseTest.java:169)
    at com.mysema.query.suites.H2ExceptionSuiteTest.UpdateBatchFailed2(H2ExceptionSuiteTest.java:48)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:622)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:24)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:136)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.createRequestAndRun(JUnitCoreWrapper.java:141)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.executeEager(JUnitCoreWrapper.java:114)
    at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:86)
    at org.apache.maven.surefire.junitcore.JUnitCoreProvider.invoke(JUnitCoreProvider.java:134)
    at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
Caused by: com.mysema.query.sql.support.JavaSE6SQLExceptionWrapper$WrappedSQLCauseException: Detailed SQLException information:
SQLState: 23505
ErrorCode: 23505
Message: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (3, 7)"; SQL statement:
update SURVEY
set ID = 3 [23505-178]

SQLState: 23505
ErrorCode: 23505
Message: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (3, 5)"; SQL statement:
update SURVEY
set ID = 3 [23505-178]

    ... 40 more
Caused by: org.h2.jdbc.JdbcBatchUpdateException: Unique index or primary key violation: "UNIQUE_ID_INDEX_9 ON PUBLIC.SURVEY(ID) VALUES (3, 7)"; SQL statement:
update SURVEY
set ID = 3 [23505-178]
    at org.h2.jdbc.JdbcPreparedStatement.executeBatch(JdbcPreparedStatement.java:1199)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:161)
    at com.mysema.query.sql.dml.AbstractSQLClause.executeBatch(AbstractSQLClause.java:171)
    at com.mysema.query.sql.dml.SQLUpdateClause.execute(SQLUpdateClause.java:190)
    ... 36 more

Here the content of the batch is a bit more clear, is this okay?

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Oct 17, 2014

Contributor

Yes, the new output looks okay. As for the testcase, I provided one approach in #1008 (comment)

Another approach (closer to the original) is:

  1. Register a user-function (see http://www.h2database.com/html/features.html#user_defined_functions) which throws ClassCastException depending on the input value. Make sure it throws different exceptions for different inputs.
  2. Feed in two different inputs, using batch mode.
  3. Ensure the returned exception contains both (different exceptions).

I think the first approach suggested in #1008 (comment) is better because it isn't database-specific and is easier to implement.

Contributor

cowwoc commented Oct 17, 2014

Yes, the new output looks okay. As for the testcase, I provided one approach in #1008 (comment)

Another approach (closer to the original) is:

  1. Register a user-function (see http://www.h2database.com/html/features.html#user_defined_functions) which throws ClassCastException depending on the input value. Make sure it throws different exceptions for different inputs.
  2. Feed in two different inputs, using batch mode.
  3. Ensure the returned exception contains both (different exceptions).

I think the first approach suggested in #1008 (comment) is better because it isn't database-specific and is easier to implement.

@timowest timowest added this to the 3.5.1 milestone Oct 19, 2014

@timowest timowest closed this in #1008 Oct 20, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment