Skip to content

Commit

Permalink
DRILL-2769: Fix most non-SQLException not-supported-yet exceptions.
Browse files Browse the repository at this point in the history
Core:

Added (auto-scanning) unit test. [Drill2769UnsupportedReportsUseSqlExceptionTest]

Added translation of lots of UnsupportedOperationExceptions (and some
RuntimeExceptions) from Avatica code to SQLFeatureNotSupportedExceptions (tons
of method overrides).

Also:

Added explicit bounds checks in ResultSetMetaData methods and checking of
last-accessed column in DrillAccessorList.wasNull() (to fix other
RuntimeExceptions to SQLExceptions).

Added resetting of last-accessed column to fix latent bug in DrillAccessorList.

Hygiene:
- Renamed some zero-based index/ordinal-position parameters to "...Offset".
- Renamed some one-based index/ordinal-position parameters to "...Number".
- Renamed DrillAccessorList lastColumn to rowLastColumnOffset; declared
  explicit logical null value for rowLastColumnOffset.
  • Loading branch information
dsbos authored and hnfgns committed Nov 9, 2015
1 parent daf816c commit e4f257b
Show file tree
Hide file tree
Showing 9 changed files with 1,712 additions and 249 deletions.
Expand Up @@ -26,15 +26,34 @@
import org.apache.drill.exec.vector.ValueVector; import org.apache.drill.exec.vector.ValueVector;
import org.apache.drill.exec.vector.accessor.BoundCheckingAccessor; import org.apache.drill.exec.vector.accessor.BoundCheckingAccessor;
import org.apache.drill.exec.vector.accessor.SqlAccessor; import org.apache.drill.exec.vector.accessor.SqlAccessor;
import org.apache.drill.jdbc.JdbcApiSqlException;




class DrillAccessorList extends BasicList<Accessor>{ class DrillAccessorList extends BasicList<Accessor> {
static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillAccessorList.class);
@SuppressWarnings("unused")
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillAccessorList.class);
/** "None" value for rowLastColumnOffset. */
// (Not -1, since -1 can result from 0 (bad 1-based index) minus 1 (offset
// from 1-based to 0-based indexing.)
private static final int NULL_LAST_COLUMN_INDEX = -2;


private AvaticaDrillSqlAccessor[] accessors = new AvaticaDrillSqlAccessor[0]; private AvaticaDrillSqlAccessor[] accessors = new AvaticaDrillSqlAccessor[0];
// TODO Rename to lastColumnAccessed and/or document.
// TODO Why 1, rather than, say, -1? /** Zero-based offset of last column referenced in current row.
private int lastColumn = 1; * For {@link #wasNull()}. */
private int rowLastColumnOffset = NULL_LAST_COLUMN_INDEX;


/**
* Resets last-column-referenced information for {@link #wasNull}.
* Must be called whenever row is advanced (when {@link ResultSet#next()}
* is called).
*/
void clearLastColumnIndexedInRow() {
rowLastColumnOffset = NULL_LAST_COLUMN_INDEX;
}


void generateAccessors(DrillCursor cursor, RecordBatchLoader currentBatch) { void generateAccessors(DrillCursor cursor, RecordBatchLoader currentBatch) {
int cnt = currentBatch.getSchema().getFieldCount(); int cnt = currentBatch.getSchema().getFieldCount();
Expand All @@ -47,16 +66,29 @@ void generateAccessors(DrillCursor cursor, RecordBatchLoader currentBatch) {
); );
accessors[i] = new AvaticaDrillSqlAccessor(acc, cursor); accessors[i] = new AvaticaDrillSqlAccessor(acc, cursor);
} }
clearLastColumnIndexedInRow();
} }


/**
* @param accessorOffset 0-based index of accessor array (not 1-based SQL
* column index/ordinal value)
*/
@Override @Override
public AvaticaDrillSqlAccessor get(int index) { public AvaticaDrillSqlAccessor get(final int accessorOffset) {
lastColumn = index; final AvaticaDrillSqlAccessor accessor = accessors[accessorOffset];
return accessors[index]; // Update lastColumnIndexedInRow after indexing accessors to not touch
// lastColumnIndexedInRow in case of out-of-bounds exception.
rowLastColumnOffset = accessorOffset;
return accessor;
} }


boolean wasNull() throws SQLException{ boolean wasNull() throws SQLException{
return accessors[lastColumn].wasNull(); if (NULL_LAST_COLUMN_INDEX == rowLastColumnOffset) {
throw new JdbcApiSqlException(
"ResultSet.wasNull() called without a preceding call to a column"
+ " getter method since the last call to ResultSet.next()");
}
return accessors[rowLastColumnOffset].wasNull();
} }


@Override @Override
Expand Down
Expand Up @@ -406,13 +406,23 @@ public PreparedStatement prepareStatement(String sql) throws SQLException {
@Override @Override
public CallableStatement prepareCall(String sql) throws SQLException { public CallableStatement prepareCall(String sql) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.prepareCall(sql); try {
return super.prepareCall(sql);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public String nativeSQL(String sql) throws SQLException { public String nativeSQL(String sql) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.nativeSQL(sql); try {
return super.nativeSQL(sql);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }




Expand Down Expand Up @@ -496,19 +506,34 @@ public PreparedStatement prepareStatement(String sql, int resultSetType,
public CallableStatement prepareCall(String sql, int resultSetType, public CallableStatement prepareCall(String sql, int resultSetType,
int resultSetConcurrency) throws SQLException { int resultSetConcurrency) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.prepareCall(sql, resultSetType, resultSetConcurrency); try {
return super.prepareCall(sql, resultSetType, resultSetConcurrency);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public Map<String,Class<?>> getTypeMap() throws SQLException { public Map<String,Class<?>> getTypeMap() throws SQLException {
throwIfClosed(); throwIfClosed();
return super.getTypeMap(); try {
return super.getTypeMap();
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public void setTypeMap(Map<String,Class<?>> map) throws SQLException { public void setTypeMap(Map<String,Class<?>> map) throws SQLException {
throwIfClosed(); throwIfClosed();
super.setTypeMap(map); try {
super.setTypeMap(map);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
Expand All @@ -528,59 +553,104 @@ public CallableStatement prepareCall(String sql, int resultSetType,
int resultSetConcurrency, int resultSetConcurrency,
int resultSetHoldability) throws SQLException { int resultSetHoldability) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.prepareCall(sql, resultSetType, resultSetConcurrency, try {
resultSetHoldability); return super.prepareCall(sql, resultSetType, resultSetConcurrency,
resultSetHoldability);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public PreparedStatement prepareStatement(String sql, public PreparedStatement prepareStatement(String sql,
int autoGeneratedKeys) throws SQLException { int autoGeneratedKeys) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.prepareStatement(sql, autoGeneratedKeys); try {
return super.prepareStatement(sql, autoGeneratedKeys);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public PreparedStatement prepareStatement(String sql, public PreparedStatement prepareStatement(String sql,
int columnIndexes[]) throws SQLException { int columnIndexes[]) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.prepareStatement(sql, columnIndexes); try {
return super.prepareStatement(sql, columnIndexes);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public PreparedStatement prepareStatement(String sql, public PreparedStatement prepareStatement(String sql,
String columnNames[]) throws SQLException { String columnNames[]) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.prepareStatement(sql, columnNames); try {
return super.prepareStatement(sql, columnNames);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public Clob createClob() throws SQLException { public Clob createClob() throws SQLException {
throwIfClosed(); throwIfClosed();
return super.createClob(); try {
return super.createClob();
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public Blob createBlob() throws SQLException { public Blob createBlob() throws SQLException {
throwIfClosed(); throwIfClosed();
return super.createBlob(); try {
return super.createBlob();
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public NClob createNClob() throws SQLException { public NClob createNClob() throws SQLException {
throwIfClosed(); throwIfClosed();
return super.createNClob(); try {
return super.createNClob();
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public SQLXML createSQLXML() throws SQLException { public SQLXML createSQLXML() throws SQLException {
throwIfClosed(); throwIfClosed();
return super.createSQLXML(); try {
return super.createSQLXML();
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public boolean isValid(int timeout) throws SQLException { public boolean isValid(int timeout) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.isValid(timeout); try {
return super.isValid(timeout);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
Expand All @@ -590,7 +660,14 @@ public void setClientInfo(String name, String value) throws SQLClientInfoExcepti
} catch (AlreadyClosedSqlException e) { } catch (AlreadyClosedSqlException e) {
throw new SQLClientInfoException(e.getMessage(), null, e); throw new SQLClientInfoException(e.getMessage(), null, e);
} }
super.setClientInfo(name, value); try {
super.setClientInfo(name, value);
}
catch (UnsupportedOperationException e) {
SQLFeatureNotSupportedException intended =
new SQLFeatureNotSupportedException(e.getMessage(), e);
throw new SQLClientInfoException(e.getMessage(), null, intended);
}
} }


@Override @Override
Expand All @@ -600,31 +677,58 @@ public void setClientInfo(Properties properties) throws SQLClientInfoException {
} catch (AlreadyClosedSqlException e) { } catch (AlreadyClosedSqlException e) {
throw new SQLClientInfoException(e.getMessage(), null, e); throw new SQLClientInfoException(e.getMessage(), null, e);
} }
super.setClientInfo(properties); try {
super.setClientInfo(properties);
}
catch (UnsupportedOperationException e) {
SQLFeatureNotSupportedException intended =
new SQLFeatureNotSupportedException(e.getMessage(), e);
throw new SQLClientInfoException(e.getMessage(), null, intended);
}
} }


@Override @Override
public String getClientInfo(String name) throws SQLException { public String getClientInfo(String name) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.getClientInfo(name); try {
return super.getClientInfo(name);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public Properties getClientInfo() throws SQLException { public Properties getClientInfo() throws SQLException {
throwIfClosed(); throwIfClosed();
return super.getClientInfo(); try {
return super.getClientInfo();
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public Array createArrayOf(String typeName, Object[] elements) throws SQLException { public Array createArrayOf(String typeName, Object[] elements) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.createArrayOf(typeName, elements); try {
return super.createArrayOf(typeName, elements);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
public Struct createStruct(String typeName, Object[] attributes) throws SQLException { public Struct createStruct(String typeName, Object[] attributes) throws SQLException {
throwIfClosed(); throwIfClosed();
return super.createStruct(typeName, attributes); try {
return super.createStruct(typeName, attributes);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }


@Override @Override
Expand All @@ -648,7 +752,12 @@ public String getSchema() {
@Override @Override
public void abort(Executor executor) throws SQLException { public void abort(Executor executor) throws SQLException {
throwIfClosed(); throwIfClosed();
super.abort(executor); try {
super.abort(executor);
}
catch (UnsupportedOperationException e) {
throw new SQLFeatureNotSupportedException(e.getMessage(), e);
}
} }




Expand Down Expand Up @@ -692,6 +801,10 @@ void cleanup() {
closeOrWarn(serviceSet, "Exception while closing service set.", logger); closeOrWarn(serviceSet, "Exception while closing service set.", logger);
} }


// TODO(DRILL-xxxx): Eliminate this test-specific hack from production code.
// If we're not going to have tests themselves explicitly handle making names
// unique, then at least move this logic into a test base class, and have it
// go through DrillConnection.getClient().
/** /**
* Test only code to make JDBC tests run concurrently. If the property <i>drillJDBCUnitTests</i> is set to * Test only code to make JDBC tests run concurrently. If the property <i>drillJDBCUnitTests</i> is set to
* <i>true</i> in connection properties: * <i>true</i> in connection properties:
Expand Down
Expand Up @@ -317,6 +317,7 @@ else if ( returnTrueForNextCallToNext ) {
return true; return true;
} }
else { else {
accessors.clearLastColumnIndexedInRow();
return nextRowInternally(); return nextRowInternally();
} }
} }
Expand Down

0 comments on commit e4f257b

Please sign in to comment.