Skip to content

Commit

Permalink
Revert "[CALCITE-2845] Avoid duplication of exception messages"
Browse files Browse the repository at this point in the history
This reverts commit 80ccd20
  • Loading branch information
danny0405 authored and F21 committed May 9, 2019
1 parent 75dd37d commit d52c203
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 188 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -552,11 +552,9 @@ protected ResultSet executeQueryInternal(AvaticaStatement statement,
statement.updateCount = metaResultSet.updateCount;
signature2 = executeResult.resultSets.get(0).signature;
}
} catch (SQLException e) {
// We don't add meaningful info yet, so just rethrow the original exception
throw e;
} catch (Exception e) {
throw HELPER.createException("Error while executing a prepared statement", e);
e.printStackTrace();
throw HELPER.createException(e.getMessage(), e);
}

final TimeZone timeZone = getTimeZone();
Expand All @@ -575,12 +573,9 @@ protected ResultSet executeQueryInternal(AvaticaStatement statement,
statement.openResultSet.execute();
isUpdateCapable(statement);
}
} catch (SQLException e) {
// We don't add meaningful info yet, so just rethrow the original exception
throw e;
} catch (Exception e) {
throw HELPER.createException(
"Error while executing a resultset", e);
"exception while executing query: " + e.getMessage(), e);
}
return statement.openResultSet;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,7 @@ public String getRemoteServer() {
}

void printServerStackTrace(PrintStreamOrWriter streamOrWriter) {
List<String> traces = this.stackTraces;
for (int i = 0; i < traces.size(); i++) {
String serverStackTrace = traces.get(i);
streamOrWriter.println("Remote driver error #" + i + ":");
for (String serverStackTrace : this.stackTraces) {
streamOrWriter.println(serverStackTrace);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ protected void executeInternal(String sql) throws SQLException {
}
}
} catch (RuntimeException e) {
throw AvaticaConnection.HELPER.createException(
"Error while executing SQL \"" + sql + "\"", e);
throw AvaticaConnection.HELPER.createException("Error while executing SQL \"" + sql + "\": "
+ e.getMessage(), e);
}

throw new RuntimeException("Failed to successfully execute query after "
Expand Down Expand Up @@ -231,8 +231,8 @@ public ResultSet executeQuery(String sql) throws SQLException {
}
return openResultSet;
} catch (RuntimeException e) {
throw AvaticaConnection.HELPER.createException(
"Error while executing SQL \"" + sql + "\"", e);
throw AvaticaConnection.HELPER.createException("Error while executing SQL \"" + sql + "\": "
+ e.getMessage(), e);
}
}

Expand Down
5 changes: 1 addition & 4 deletions core/src/main/java/org/apache/calcite/avatica/Helper.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ public SQLException createException(String message, String sql, Exception e) {
if (null != rte.getRpcMetadata()) {
serverAddress = rte.getRpcMetadata().serverAddress;
}
// Note: we don't add "e" as a cause, so we add its message to the message of
// the newly created exception
return new AvaticaSqlException(message + ". " + e.getMessage(),
rte.getSqlState(), rte.getErrorCode(),
return new AvaticaSqlException(message, rte.getSqlState(), rte.getErrorCode(),
rte.getServerExceptions(), serverAddress);
}
return new SQLException(message, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ private HandlerResponse<T> createErrorResponse(Exception e, int statusCode) {
* @param e The Exception to summarize.
* @return A summary message for the Exception.
*/
private String getCausalChain(Throwable e) {
private String getCausalChain(Exception e) {
StringBuilder sb = new StringBuilder(16);
Throwable curr = e;
// Could use Guava, but that would increase dependency set unnecessarily.
Expand All @@ -156,17 +156,6 @@ private String getCausalChain(Throwable e) {
String message = curr.getMessage();
sb.append(curr.getClass().getSimpleName()).append(": ");
sb.append(null == message ? NULL_EXCEPTION_MESSAGE : message);
Throwable[] suppressed = curr.getSuppressed();
if (suppressed.length > 0) {
sb.append(", suppressed: [");
String sep = "";
for (Throwable throwable : suppressed) {
sb.append(sep);
sb.append(getCausalChain(throwable));
sep = ", ";
}
sb.append("]");
}
curr = curr.getCause();
}
if (sb.length() == 0) {
Expand Down
43 changes: 24 additions & 19 deletions server/src/main/java/org/apache/calcite/avatica/jdbc/JdbcMeta.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ public Map<DatabaseProperty, Object> getDatabaseProperties(ConnectionHandle ch)
}
return map;
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -315,7 +315,7 @@ private static Object addProperty(Map<DatabaseProperty, Object> map,
try {
propertyValue = p.method.invoke(metaData);
} catch (IllegalAccessException | InvocationTargetException e) {
throw propagate(e);
throw new RuntimeException(e);
}
} else {
propertyValue = p.defaultValue;
Expand All @@ -333,7 +333,7 @@ public MetaResultSet getTables(ConnectionHandle ch, String catalog, Pat schemaPa
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -359,7 +359,7 @@ public MetaResultSet getColumns(ConnectionHandle ch, String catalog, Pat schemaP
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -370,7 +370,7 @@ public MetaResultSet getSchemas(ConnectionHandle ch, String catalog, Pat schemaP
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -380,7 +380,7 @@ public MetaResultSet getCatalogs(ConnectionHandle ch) {
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -390,7 +390,7 @@ public MetaResultSet getTableTypes(ConnectionHandle ch) {
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -403,7 +403,7 @@ public MetaResultSet getProcedures(ConnectionHandle ch, String catalog, Pat sche
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -416,7 +416,7 @@ public MetaResultSet getProcedureColumns(ConnectionHandle ch, String catalog, Pa
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -429,7 +429,7 @@ public MetaResultSet getColumnPrivileges(ConnectionHandle ch, String catalog, St
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -442,7 +442,7 @@ public MetaResultSet getTablePrivileges(ConnectionHandle ch, String catalog, Pat
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -457,7 +457,7 @@ public MetaResultSet getBestRowIdentifier(ConnectionHandle ch, String catalog, S
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -470,7 +470,7 @@ public MetaResultSet getVersionColumns(ConnectionHandle ch, String catalog, Stri
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -483,7 +483,7 @@ public MetaResultSet getPrimaryKeys(ConnectionHandle ch, String catalog, String
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -509,7 +509,7 @@ public MetaResultSet getTypeInfo(ConnectionHandle ch) {
int stmtId = registerMetaStatement(rs);
return JdbcResultSet.create(ch.id, stmtId, rs);
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand Down Expand Up @@ -630,7 +630,7 @@ public StatementHandle createStatement(ConnectionHandle ch) {
throw new RuntimeException("Connection already exists: " + ch.id);
}
} catch (SQLException e) {
throw propagate(e);
throw new RuntimeException(e);
}
}

Expand Down Expand Up @@ -691,9 +691,14 @@ protected void apply(Connection conn, ConnectionProperties connProps)
}
}

static <E extends Throwable> RuntimeException propagate(Throwable e) throws E {
// We have nothing to add, so just throw the original exception
throw (E) e;
RuntimeException propagate(Throwable e) {
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
} else if (e instanceof Error) {
throw (Error) e;
} else {
throw new RuntimeException(e);
}
}

public StatementHandle prepare(ConnectionHandle ch, String sql,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,11 @@
import java.util.UUID;
import java.util.concurrent.Callable;

import static org.apache.calcite.avatica.test.StringContainsOnce.containsStringOnce;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.core.StringContains.containsString;
import static org.hamcrest.core.StringStartsWith.startsWith;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -915,9 +914,8 @@ private void checkPrepareBindExecuteFetch(Connection connection)
fail("expected error, got " + resultSet);
} catch (SQLException e) {
LOG.info("Caught expected error", e);
String message = ExceptionUtils.toString(e);
assertThat(message,
containsStringOnce("unbound parameter"));
assertThat(e.getMessage(),
containsString("exception while executing query: unbound parameter"));
}

final ParameterMetaData parameterMetaData = ps.getParameterMetaData();
Expand Down Expand Up @@ -1844,9 +1842,8 @@ public LoggingLocalProtobufService(Service service, ProtobufTranslation translat

@Override public Response _apply(Request request) {
final RequestLogger logger = THREAD_LOG.get();
String jsonRequest = null;
try {
jsonRequest = JsonService.MAPPER.writeValueAsString(request);
String jsonRequest = JsonService.MAPPER.writeValueAsString(request);
logger.requestStart(jsonRequest);

Response response = super._apply(request);
Expand All @@ -1856,7 +1853,7 @@ public LoggingLocalProtobufService(Service service, ProtobufTranslation translat

return response;
} catch (Exception e) {
throw new RuntimeException("Request " + jsonRequest + " failed", e);
throw new RuntimeException(e);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ public class JdbcMetaTest {
@Test public void testExceptionPropagation() throws SQLException {
JdbcMeta meta = new JdbcMeta("url");
final Throwable e = new Exception();
final RuntimeException rte;
try {
meta.propagate(e);
fail("Expected an exception to be thrown");
} catch (Throwable caughtException) {
assertThat(caughtException, is(e));
} catch (RuntimeException caughtException) {
rte = caughtException;
assertThat(rte.getCause(), is(e));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,10 @@ private static String longString(String fragment, int length) {
DriverManager.getConnection(url, "john", "doe");
fail("expected exception");
} catch (RuntimeException e) {
assertEquals("Remote driver error: "
+ "SQLInvalidAuthorizationSpecException: invalid authorization specification - "
assertEquals("Remote driver error: RuntimeException: "
+ "java.sql.SQLInvalidAuthorizationSpecException: invalid authorization specification"
+ " - not found: john"
+ " -> SQLInvalidAuthorizationSpecException: invalid authorization specification - "
+ "not found: john"
+ " -> HsqlException: invalid authorization specification - not found: john",
e.getMessage());
Expand All @@ -355,8 +357,9 @@ private static String longString(String fragment, int length) {
stmt2.executeQuery("select * from buffer");
fail("expected exception");
} catch (Exception e) {
assertEquals("Error -1 (00000) : Error while executing SQL \"select * from buffer\". "
+ "Remote driver error: "
assertEquals("Error -1 (00000) : Error while executing SQL \"select * from buffer\": "
+ "Remote driver error: RuntimeException: java.sql.SQLSyntaxErrorException: "
+ "user lacks privilege or object not found: BUFFER -> "
+ "SQLSyntaxErrorException: user lacks privilege or object not found: BUFFER -> "
+ "HsqlException: user lacks privilege or object not found: BUFFER",
e.getMessage());
Expand Down
Loading

0 comments on commit d52c203

Please sign in to comment.