From 804871ba0192bef51cb11d1dd77783572dbef2fb Mon Sep 17 00:00:00 2001 From: rusher Date: Wed, 30 Aug 2017 14:58:22 +0200 Subject: [PATCH 1/2] [CONJ-514] ResultSet method wasNull() always return true after a call on a "null-date" field binary protocol handling --- .../com/read/resultset/SelectResultSet.java | 22 ++++++++++++------- src/test/java/org/mariadb/jdbc/DateTest.java | 6 +++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java b/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java index 7cc7bb1f7..d396e670e 100644 --- a/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java +++ b/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java @@ -990,10 +990,13 @@ private String getInternalString(ColumnInformation columnInfo, Calendar cal) thr if (isBinaryEncoded) { Date date = getInternalDate(columnInfo, cal); if (date == null) { - //row data is not null but result is null -> this is "zero-date" - //specific for "zero-date", getString will return "zero-date" value -> wasNull() must then return false - lastValueNull ^= BIT_LAST_ZERO_DATE; - return new String(row.buf, row.pos, row.length, Buffer.UTF_8); + if (!isBinaryEncoded) { + //row data is not null but result is null -> this is "zero-date" + //specific for "zero-date", getString will return "zero-date" value -> wasNull() must then return false + lastValueNull ^= BIT_LAST_ZERO_DATE; + return new String(row.buf, row.pos, row.length, Buffer.UTF_8); + } + return null; } return date.toString(); } @@ -1011,10 +1014,13 @@ private String getInternalString(ColumnInformation columnInfo, Calendar cal) thr case DATETIME: Timestamp timestamp = getInternalTimestamp(columnInfo, cal); if (timestamp == null) { - //row data is not null but result is null -> this is "zero-date" - //specific for "zero-date", getString will return "zero-date" value -> wasNull() must then return false - lastValueNull ^= BIT_LAST_ZERO_DATE; - return new String(row.buf, row.pos, row.length, Buffer.UTF_8); + if (!isBinaryEncoded) { + //row data is not null but result is null -> this is "zero-date" + //specific for "zero-date", getString will return "zero-date" value -> wasNull() must then return false + lastValueNull ^= BIT_LAST_ZERO_DATE; + return new String(row.buf, row.pos, row.length, Buffer.UTF_8); + } + return null; } return timestamp.toString(); case DECIMAL: diff --git a/src/test/java/org/mariadb/jdbc/DateTest.java b/src/test/java/org/mariadb/jdbc/DateTest.java index f33becff7..6a4de921d 100644 --- a/src/test/java/org/mariadb/jdbc/DateTest.java +++ b/src/test/java/org/mariadb/jdbc/DateTest.java @@ -584,10 +584,14 @@ public void nullDateString() throws Throwable { assertTrue(rs.next()); if (sharedUsePrepare()) { assertNull(rs.getString(1)); + assertTrue(rs.wasNull()); assertNull(rs.getDate(1)); + assertTrue(rs.wasNull()); } else { assertEquals("0000-00-00", rs.getString(1)); + assertFalse(rs.wasNull()); assertNull(rs.getDate(1)); + assertTrue(rs.wasNull()); } } catch (SQLDataException sqldataException) { //'0000-00-00' doesn't work anymore on mysql 5.7. @@ -647,8 +651,10 @@ public void getZeroDateString() throws SQLException { assertEquals(null, resultSet.getDate(1)); if (sharedUsePrepare()) { assertEquals(null, resultSet.getString(1)); + assertTrue(resultSet.wasNull()); } else { assertTrue(resultSet.getString(1).contains("0000-00-00 00:00:00")); + assertFalse(resultSet.wasNull()); } resultSet = statement.executeQuery("SELECT * from zeroTimestamp"); From ff8f116fde7cbf2ae6ee6588918cd967cae774aa Mon Sep 17 00:00:00 2001 From: rusher Date: Wed, 30 Aug 2017 18:31:19 +0200 Subject: [PATCH 2/2] [misc] correction of possible race condition when multi threading on result set with fetch --- .../com/read/resultset/SelectResultSet.java | 142 +++++++----------- 1 file changed, 56 insertions(+), 86 deletions(-) diff --git a/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java b/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java index d396e670e..3658023f0 100644 --- a/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java +++ b/src/main/java/org/mariadb/jdbc/internal/com/read/resultset/SelectResultSet.java @@ -136,7 +136,7 @@ public class SelectResultSet implements ResultSet { private boolean returnTableAlias; private boolean isClosed; private boolean eofDeprecated; - + private ReentrantLock lock; /** * Create Streaming resultSet. @@ -185,6 +185,7 @@ public SelectResultSet(ColumnInformation[] columnInformation, Results results, P fetchAllResults(); streaming = false; } else { + this.lock = protocol.getLock(); protocol.setActiveStreamingResult(results); protocol.removeHasMoreResults(); data = new byte[Math.max(10, fetchSize)][]; @@ -225,13 +226,14 @@ public SelectResultSet(ColumnInformation[] columnInformation, List resul this.columnInformationLength = columnInformation.length; this.isEof = true; this.isBinaryEncoded = false; - this.fetchSize = 1; + this.fetchSize = 0; this.resultSetScrollType = resultSetScrollType; this.data = resultSet.toArray(new byte[10][]); this.dataSize = resultSet.size(); this.dataFetchTime = 0; this.rowPointer = -1; this.callableResult = false; + this.streaming = false; } /** @@ -321,50 +323,34 @@ private void fetchAllResults() throws IOException, SQLException { * @throws SQLException if any error occur */ public void fetchRemaining() throws SQLException { - try { - try { - if (!isEof) { - lastRowPointer = -1; - ReentrantLock lock = protocol.getLock(); - lock.lock(); - try { - while (!isEof) { - addStreamingValue(); - } - } finally { - lock.unlock(); - } - } - - } catch (IOException ioexception) { - throw new SQLException("Could not close resultSet : " + ioexception.getMessage(), - CONNECTION_EXCEPTION.getSqlState(), ioexception); - } - } catch (SQLException queryException) { - ExceptionMapper.throwException(queryException, null, this.statement); - } - - dataFetchTime++; - } - - private void fetchRemainingLock() throws SQLException { if (!isEof) { - //load remaining results - ReentrantLock lock = protocol.getLock(); lock.lock(); try { - fetchRemaining(); - } catch (SQLException ioe) { - throw new SQLException("Server has closed the connection. If result set contain huge amount of data, Server expects client to" - + " read off the result set relatively fast. " - + "In this case, please consider increasing net_wait_timeout session variable." - + " / processing your result set faster (check Streaming result sets documentation for more information)", ioe); + lastRowPointer = -1; + while (!isEof) { + addStreamingValue(); + } + + } catch (SQLException queryException) { + throw ExceptionMapper.getException(queryException, null, statement, false); + } catch (IOException ioe) { + throw handleIoException(ioe); } finally { lock.unlock(); } + dataFetchTime++; } } + private SQLException handleIoException(IOException ioe) { + return ExceptionMapper.getException(new SQLException("Server has closed the connection. " + + "If result set contain huge amount of data, Server expects client to" + + " read off the result set relatively fast. " + + "In this case, please consider increasing net_wait_timeout session variable." + + " / processing your result set faster (check Streaming result sets documentation for more information)", + CONNECTION_EXCEPTION.getSqlState(), ioe), null, statement, false); + } + /** * This permit to replace current stream results by next ones. * @@ -413,14 +399,13 @@ public boolean readNextValue() throws IOException, SQLException { protocol.removeHasMoreResults(); protocol.setHasWarnings(false); ErrorPacket errorPacket = new ErrorPacket(new Buffer(buf)); - protocol = null; - reader = null; - isEof = true; + resetVariables(); if (statement != null) { - throw new SQLException("(conn:" + statement.getServerThreadId() + ") " + errorPacket.getMessage(), - errorPacket.getSqlState(), errorPacket.getErrorNumber()); + throw ExceptionMapper.getException(new SQLException("(conn:" + statement.getServerThreadId() + ") " + errorPacket.getMessage(), + errorPacket.getSqlState(), errorPacket.getErrorNumber()), null, statement, false); } else { - throw new SQLException(errorPacket.getMessage(), errorPacket.getSqlState(), errorPacket.getErrorNumber()); + throw ExceptionMapper.getException(new SQLException(errorPacket.getMessage(), errorPacket.getSqlState(), + errorPacket.getErrorNumber()), null, statement, false); } } @@ -458,9 +443,7 @@ public boolean readNextValue() throws IOException, SQLException { protocol.setHasWarnings(warnings > 0); if ((serverStatus & MORE_RESULTS_EXISTS) == 0) protocol.removeActiveStreamingResult(); - isEof = true; - protocol = null; - reader = null; + resetVariables(); return false; } @@ -514,8 +497,7 @@ private void growDataArray() { */ public void close() throws SQLException { isClosed = true; - if (protocol != null) { - ReentrantLock lock = protocol.getLock(); + if (!isEof) { lock.lock(); try { while (!isEof) { @@ -523,23 +505,16 @@ public void close() throws SQLException { readNextValue(); } - } catch (IOException ioexception) { - ExceptionMapper.throwException(new SQLException( - "Could not close resultSet : " + ioexception.getMessage() + protocol.getTraces(), - CONNECTION_EXCEPTION.getSqlState(), ioexception), null, this.statement); } catch (SQLException queryException) { - ExceptionMapper.throwException(queryException, null, this.statement); + throw ExceptionMapper.getException(queryException, null, this.statement, false); + } catch (IOException ioe) { + throw handleIoException(ioe); } finally { - isEof = true; - protocol = null; - reader = null; + resetVariables(); lock.unlock(); } - } else { - protocol = null; - reader = null; - isEof = true; } + resetVariables(); //keep garbage easy for (int i = 0; i < data.length; i++) { @@ -552,6 +527,12 @@ public void close() throws SQLException { } } + private void resetVariables() { + protocol = null; + reader = null; + isEof = true; + } + @Override public boolean next() throws SQLException { if (isClosed) throw new SQLException("Operation not permit on a closed resultSet", "HY000"); @@ -560,15 +541,11 @@ public boolean next() throws SQLException { return true; } else { if (streaming && !isEof) { - ReentrantLock lock = protocol.getLock(); lock.lock(); try { - nextStreamingValue(); + if (!isEof) nextStreamingValue(); } catch (IOException ioe) { - throw new SQLException("Server has closed the connection. If result set contain huge amount of data, Server expects client to" - + " read off the result set relatively fast. " - + "In this case, please consider increasing net_wait_timeout session variable." - + " / processing your result set faster (check Streaming result sets documentation for more information)", ioe); + throw handleIoException(ioe); } finally { lock.unlock(); } @@ -650,16 +627,12 @@ public boolean isAfterLast() throws SQLException { //has to read more result to know if it's finished or not //(next packet may be new data or an EOF packet indicating that there is no more data) - ReentrantLock lock = protocol.getLock(); lock.lock(); try { //this time, fetch is added even for streaming forward type only to keep current pointer row. - addStreamingValue(); + if (!isEof) addStreamingValue(); } catch (IOException ioe) { - throw new SQLException("Server has closed the connection. If result set contain huge amount of data, Server expects client to" - + " read off the result set relatively fast. " - + "In this case, please consider increasing net_wait_timeout session variable." - + " / processing your result set faster (check Streaming result sets documentation for more information)", ioe); + throw handleIoException(ioe); } finally { lock.unlock(); } @@ -690,15 +663,11 @@ public boolean isLast() throws SQLException { } else { //when streaming and not having read all results, //must read next packet to know if next packet is an EOF packet or some additional data - ReentrantLock lock = protocol.getLock(); lock.lock(); try { - addStreamingValue(); + if (!isEof) addStreamingValue(); } catch (IOException ioe) { - throw new SQLException("Server has closed the connection. If result set contain huge amount of data, Server expects client to" - + " read off the result set relatively fast. " - + "In this case, please consider increasing net_wait_timeout session variable." - + " / processing your result set faster (check Streaming result sets documentation for more information)", ioe); + throw handleIoException(ioe); } finally { lock.unlock(); } @@ -726,7 +695,7 @@ public void beforeFirst() throws SQLException { @Override public void afterLast() throws SQLException { checkClose(); - fetchRemainingLock(); + fetchRemaining(); rowPointer = dataSize; } @@ -745,7 +714,7 @@ public boolean first() throws SQLException { @Override public boolean last() throws SQLException { checkClose(); - fetchRemainingLock(); + fetchRemaining(); rowPointer = dataSize - 1; return rowPointer > 0; } @@ -773,7 +742,7 @@ public boolean absolute(int row) throws SQLException { } //if streaming, must read additional results. - fetchRemainingLock(); + fetchRemaining(); if (row >= 0) { @@ -847,15 +816,16 @@ public int getFetchSize() throws SQLException { @Override public void setFetchSize(int fetchSize) throws SQLException { if (streaming && fetchSize == 0) { - + lock.lock(); try { - + //fetch all results while (!isEof) { - //fetch all results addStreamingValue(); } - } catch (IOException ioException) { - throw new SQLException(ioException); + } catch (IOException ioe) { + throw handleIoException(ioe); + } finally { + lock.unlock(); } streaming = dataFetchTime == 1;