Repository: pgjdbc/pgjdbc
Pull request #751 (2017-02-15 09:38:03)

fix: use SQLWarning(String reason) constructor for correct DriverMana…
…ger logging

Previously, the default constructor was used which only logs "SQLWarning: ", but no information about the warning.

vlsi: @jamesthomp , could you please add a test or two to validate the change?
codecov-io: # [Codecov](https://codecov.io/gh/pgjdbc/pgjdbc/pull/751?src=pr&el=h1) Report
> Merging [#751](https://codecov.io/gh/pgjdbc/pgjdbc/pull/751?src=pr&el=desc) into [master](https://codecov.io/gh/pgjdbc/pgjdbc/commit/b3553276ad48cbb47cd4849d3a08b7ee61be8ace?src=pr&el=desc) will **decrease** coverage by `-0.01%`.
> The diff coverage is `100%`.



```diff
@@             Coverage Diff              @@
##             master     #751      +/-   ##
============================================
- Coverage     65.62%   65.61%   -0.01%     
+ Complexity     3502     3501       -1     
============================================
  Files           164      164              
  Lines         15091    15091              
  Branches       2427     2427              
============================================
- Hits           9903     9902       -1     
  Misses         4035     4035              
- Partials       1153     1154       +1
```

------

[Continue to review full report at Codecov](https://codecov.io/gh/pgjdbc/pgjdbc/pull/751?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pgjdbc/pgjdbc/pull/751?src=pr&el=footer). Last update [b355327...1adbcdf](https://codecov.io/gh/pgjdbc/pgjdbc/compare/b3553276ad48cbb47cd4849d3a08b7ee61be8ace...1adbcdfa3cc1cbb886cd83ea0c51487d67a986ca?src=pr&el=footer&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
jamesthomp: Thanks for your prompt reply, @vlsi.
I have added a test demonstrating that the `DriverManager` now logs the full message with this change.

Let me know if any other changes are needed.
vlsi: >I have added a test demonstrating that the DriverManager now logs the full message with this change.

I thought of more like a end-to-end test. That is execute SQL with error and ensure the message is there.

As far as I can see, it is even better to use `SQLWarning(String reason, String SQLState, int vendorCode)` constructor like in `PSQLException`. Can you do that?
jamesthomp: I didn't know what a good value for the vendorCode would be, so I just switched it to the `SQLWarning(String reason, String SQLState)` constructor.

I agree that an ETE test would be nicer than hardcoding the test, though I'm actually not very familiar with this codebase so I'd need to poke around a bit if you would like me to add that kind of test. Alternatively, I'm happy to add you as a collaborator on my fork @vlsi if you would prefer to just make the addition yourself (or you can just recreate this branch on this repo and then add the ETE test). Let me know which will work best for you.
vlsi: @jamesthomp , could you check something around https://github.com/pgjdbc/pgjdbc/blob/faab499853c56f67cb70fb242f75b918452f2a6f/pgjdbc/src/test/java/org/postgresql/test/jdbc2/StatementTest.java#L403-L412, add a test that would issue [raise notice](http://stackoverflow.com/a/18828523/1261287) and assert if warning gets printed as desired


toString, getMessage, and getSQLState should be removed since proper `super(...)` should be enough.
davecramer: @jamesthomp wondering if you have time to address @vlsi questions?
jamesthomp: Sorry for the delay - I was meaning to take another look at this but didn't get around to it just yet.
jamesthomp: @vlsi I've kept `getMessage`, as removing it causes the format to slightly change which makes one of the tests in `CallableStmtTest` fail.

Patch:
diff --git a/docs/documentation/head/listennotify.md b/docs/documentation/head/listennotify.md
index b5863ba3..0352a0b3 100644
--- a/docs/documentation/head/listennotify.md
+++ b/docs/documentation/head/listennotify.md
@@ -24,6 +24,8 @@ any outstanding notifications.
 
 > A key limitation of the JDBC driver is that it cannot receive asynchronous
 notifications and must poll the backend to check if any notifications were issued.
+I timeout can be given to the poll function, but then the execution of statements
+from other threads will block.  
 
 <a name="listen-notify-example"></a>
 **Example 9.2. Receiving Notifications**
@@ -72,20 +74,16 @@ class Listener extends Thread
 
 	public void run()
 	{
-		while (true)
+		try
 		{
-			try
+			while (true)
 			{
-				// issue a dummy query to contact the backend
-				// and receive any pending notifications.
-
-				Statement stmt = conn.createStatement();
-				ResultSet rs = stmt.executeQuery("SELECT 1");
-				rs.close();
-				stmt.close();
-
 				org.postgresql.PGNotification notifications[] = pgconn.getNotifications();
 				
+				// If this thread is the only one that uses the connection, a timeout can be used to 
+				// receive notifications immediately:
+				// org.postgresql.PGNotification notifications[] = pgconn.getNotifications(10000);
+				
 				if (notifications != null)
 				{
 					for (int i=0; i&lt;notifications.length; i++)
@@ -97,14 +95,14 @@ class Listener extends Thread
 				
 				Thread.sleep(500);
 			}
-			catch (SQLException sqle)
-			{
-				sqle.printStackTrace();
-			}
-			catch (InterruptedException ie)
-			{
-				ie.printStackTrace();
-			}
+		}
+		catch (SQLException sqle)
+		{
+			sqle.printStackTrace();
+		}
+		catch (InterruptedException ie)
+		{
+			ie.printStackTrace();
 		}
 	}
 }
diff --git a/pgjdbc/pom.xml b/pgjdbc/pom.xml
index 0d2ba622..f78fa4f6 100644
--- a/pgjdbc/pom.xml
+++ b/pgjdbc/pom.xml
@@ -266,6 +266,7 @@
           </dependencies>
           <configuration>
             <configLocation>src/main/checkstyle/checks.xml</configLocation>
+	    <suppressionsLocation>src/main/checkstyle/suppressions.xml</suppressionsLocation>
             <violationSeverity>error</violationSeverity>
             <failOnViolation>true</failOnViolation>
             <failsOnError>true</failsOnError>
diff --git a/pgjdbc/src/main/checkstyle/suppressions.xml b/pgjdbc/src/main/checkstyle/suppressions.xml
new file mode 100644
index 00000000..76e50708
--- /dev/null
+++ b/pgjdbc/src/main/checkstyle/suppressions.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE suppressions PUBLIC
+        "-//Puppy Crawl//DTD Suppressions 1.1//EN"
+        "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
+
+<suppressions>
+    <suppress files="WriterHandler\.java" checks="RegexpHeader"/>
+</suppressions>
diff --git a/pgjdbc/src/main/java/org/postgresql/Driver.java b/pgjdbc/src/main/java/org/postgresql/Driver.java
index d471f4d9..27c6b486 100644
--- a/pgjdbc/src/main/java/org/postgresql/Driver.java
+++ b/pgjdbc/src/main/java/org/postgresql/Driver.java
@@ -6,12 +6,14 @@
 package org.postgresql;
 
 import org.postgresql.jdbc.PgConnection;
+
 import org.postgresql.util.ExpressionProperties;
 import org.postgresql.util.GT;
 import org.postgresql.util.HostSpec;
 import org.postgresql.util.PSQLException;
 import org.postgresql.util.PSQLState;
 import org.postgresql.util.SharedTimer;
+import org.postgresql.util.WriterHandler;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -29,8 +31,11 @@ import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.Properties;
 import java.util.Set;
+import java.util.logging.Formatter;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.logging.SimpleFormatter;
+import java.util.logging.StreamHandler;
 
 /**
  * The Java SQL framework allows for multiple database drivers. Each driver should supply a class
@@ -308,7 +313,7 @@ public class Driver implements java.sql.Driver {
       loggerHandlerFile = null;
     }
 
-    java.util.logging.Handler handler = new java.util.logging.ConsoleHandler();
+    java.util.logging.Handler handler = null;
     if (driverLogFile != null) {
       try {
         handler = new java.util.logging.FileHandler(driverLogFile);
@@ -318,7 +323,21 @@ public class Driver implements java.sql.Driver {
       }
     }
 
-    handler.setFormatter(new java.util.logging.SimpleFormatter());
+    Formatter formatter = new SimpleFormatter();
+
+    if ( handler == null ) {
+      if (DriverManager.getLogWriter() != null) {
+        handler = new WriterHandler(DriverManager.getLogWriter());
+      } else if ( DriverManager.getLogStream() != null) {
+        handler = new StreamHandler(DriverManager.getLogStream(), formatter);
+      } else {
+        handler = new StreamHandler(System.err, formatter);
+      }
+    } else {
+      handler.setFormatter(formatter);
+    }
+
+    handler.setLevel(PARENT_LOGGER.getLevel());
     PARENT_LOGGER.setUseParentHandlers(false);
     PARENT_LOGGER.addHandler(handler);
   }
diff --git a/pgjdbc/src/main/java/org/postgresql/PGConnection.java b/pgjdbc/src/main/java/org/postgresql/PGConnection.java
index 7a6b6d8e..3182ebae 100644
--- a/pgjdbc/src/main/java/org/postgresql/PGConnection.java
+++ b/pgjdbc/src/main/java/org/postgresql/PGConnection.java
@@ -31,6 +31,20 @@ public interface PGConnection {
    */
   PGNotification[] getNotifications() throws SQLException;
 
+  /**
+   * This method returns any notifications that have been received since the last call to this
+   * method. Returns null if there have been no notifications. A timeout can be specified so the
+   * driver waits for notifications.
+   *
+   * @param timeoutMillis when 0, blocks forever. when &gt; 0, blocks up to the specified number of millies
+   *        or until at least one notification has been received. If more than one notification is
+   *        about to be received, these will be returned in one batch.
+   * @return notifications that have been received
+   * @throws SQLException if something wrong happens
+   * @since 43
+   */
+  PGNotification[] getNotifications(int timeoutMillis) throws SQLException;
+
   /**
    * This returns the COPY API for the current connection.
    *
diff --git a/pgjdbc/src/main/java/org/postgresql/core/Encoding.java b/pgjdbc/src/main/java/org/postgresql/core/Encoding.java
index 557470f2..51fc774a 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/Encoding.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/Encoding.java
@@ -12,13 +12,20 @@ import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Reader;
 import java.io.Writer;
+import java.nio.charset.Charset;
 import java.util.HashMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Representation of a particular character encoding.
  */
 public class Encoding {
-  private static final Encoding DEFAULT_ENCODING = new Encoding(null);
+
+  private static final Logger LOGGER = Logger.getLogger(Encoding.class.getName());
+
+  private static final Encoding DEFAULT_ENCODING = new Encoding();
+  private static final Encoding UTF8_ENCODING = new Encoding("UTF-8");
 
   /*
    * Preferred JVM encodings for backend encodings.
@@ -28,10 +35,9 @@ public class Encoding {
   static {
     //Note: this list should match the set of supported server
     // encodings found in backend/util/mb/encnames.c
-    encodings.put("SQL_ASCII", new String[]{"ASCII", "us-ascii"});
+    encodings.put("SQL_ASCII", new String[]{"ASCII", "US-ASCII"});
     encodings.put("UNICODE", new String[]{"UTF-8", "UTF8"});
-    encodings.put("UTF8",
-        new String[]{"UTF-8", "UTF8"}); // 8.1's canonical name for UNICODE changed.
+    encodings.put("UTF8", new String[]{"UTF-8", "UTF8"});
     encodings.put("LATIN1", new String[]{"ISO8859_1"});
     encodings.put("LATIN2", new String[]{"ISO8859_2"});
     encodings.put("LATIN3", new String[]{"ISO8859_3"});
@@ -73,9 +79,28 @@ public class Encoding {
   private final String encoding;
   private final boolean fastASCIINumbers;
 
+  /**
+   * Uses the default charset of the JVM.
+   */
+  private Encoding() {
+    this(Charset.defaultCharset().name());
+  }
+
+  /**
+   * Use the charset passed as parameter.
+   *
+   * @param encoding charset name to use
+   */
   protected Encoding(String encoding) {
+    if (encoding == null) {
+      throw new NullPointerException("Null encoding charset not supported");
+    }
     this.encoding = encoding;
     fastASCIINumbers = testAsciiNumbers();
+    if (LOGGER.isLoggable(Level.FINEST)) {
+      LOGGER.log(Level.FINEST, "Creating new Encoding {0} with fastASCIINumbers {1}",
+          new Object[]{encoding, fastASCIINumbers});
+    }
   }
 
   /**
@@ -96,14 +121,13 @@ public class Encoding {
    * default JVM encoding if the specified encoding is unavailable.
    */
   public static Encoding getJVMEncoding(String jvmEncoding) {
-    if (isAvailable(jvmEncoding)) {
-      if (jvmEncoding.equals("UTF-8") || jvmEncoding.equals("UTF8")) {
-        return new UTF8Encoding(jvmEncoding);
-      } else {
-        return new Encoding(jvmEncoding);
-      }
+    if ("UTF-8".equals(jvmEncoding)) {
+      return new UTF8Encoding(jvmEncoding);
+    }
+    if (Charset.isSupported(jvmEncoding)) {
+      return new Encoding(jvmEncoding);
     } else {
-      return defaultEncoding();
+      return DEFAULT_ENCODING;
     }
   }
 
@@ -115,14 +139,17 @@ public class Encoding {
    * default JVM encoding if the specified encoding is unavailable.
    */
   public static Encoding getDatabaseEncoding(String databaseEncoding) {
+    if ("UTF8".equals(databaseEncoding)) {
+      return UTF8_ENCODING;
+    }
     // If the backend encoding is known and there is a suitable
     // encoding in the JVM we use that. Otherwise we fall back
     // to the default encoding of the JVM.
-
     String[] candidates = encodings.get(databaseEncoding);
     if (candidates != null) {
       for (String candidate : candidates) {
-        if (isAvailable(candidate)) {
+        LOGGER.log(Level.FINEST, "Search encoding candidate {0}", candidate);
+        if (Charset.isSupported(candidate)) {
           return new Encoding(candidate);
         }
       }
@@ -130,12 +157,13 @@ public class Encoding {
 
     // Try the encoding name directly -- maybe the charset has been
     // provided by the user.
-    if (isAvailable(databaseEncoding)) {
+    if (Charset.isSupported(databaseEncoding)) {
       return new Encoding(databaseEncoding);
     }
 
     // Fall back to default JVM encoding.
-    return defaultEncoding();
+    LOGGER.log(Level.FINEST, "{0} encoding not found, returning default encoding", databaseEncoding);
+    return DEFAULT_ENCODING;
   }
 
   /**
@@ -144,7 +172,7 @@ public class Encoding {
    * @return the JVM encoding name used by this instance.
    */
   public String name() {
-    return encoding;
+    return Charset.isSupported(encoding) ? Charset.forName(encoding).name() : encoding;
   }
 
   /**
@@ -159,10 +187,6 @@ public class Encoding {
       return null;
     }
 
-    if (encoding == null) {
-      return s.getBytes();
-    }
-
     return s.getBytes(encoding);
   }
 
@@ -177,10 +201,6 @@ public class Encoding {
    * @throws IOException if something goes wrong
    */
   public String decode(byte[] encodedString, int offset, int length) throws IOException {
-    if (encoding == null) {
-      return new String(encodedString, offset, length);
-    }
-
     return new String(encodedString, offset, length, encoding);
   }
 
@@ -203,10 +223,6 @@ public class Encoding {
    * @throws IOException if something goes wrong
    */
   public Reader getDecodingReader(InputStream in) throws IOException {
-    if (encoding == null) {
-      return new InputStreamReader(in);
-    }
-
     return new InputStreamReader(in, encoding);
   }
 
@@ -218,10 +234,6 @@ public class Encoding {
    * @throws IOException if something goes wrong
    */
   public Writer getEncodingWriter(OutputStream out) throws IOException {
-    if (encoding == null) {
-      return new OutputStreamWriter(out);
-    }
-
     return new OutputStreamWriter(out, encoding);
   }
 
@@ -234,23 +246,8 @@ public class Encoding {
     return DEFAULT_ENCODING;
   }
 
-  /**
-   * Test if an encoding is available in the JVM.
-   *
-   * @param encodingName the JVM encoding name to test
-   * @return true iff the encoding is supported
-   */
-  private static boolean isAvailable(String encodingName) {
-    try {
-      "DUMMY".getBytes(encodingName); //NOSONAR
-      return true;
-    } catch (java.io.UnsupportedEncodingException e) {
-      return false;
-    }
-  }
-
   public String toString() {
-    return (encoding == null ? "<default JVM encoding>" : encoding);
+    return encoding;
   }
 
   /**
diff --git a/pgjdbc/src/main/java/org/postgresql/core/QueryExecutor.java b/pgjdbc/src/main/java/org/postgresql/core/QueryExecutor.java
index fc061346..db58a091 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/QueryExecutor.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/QueryExecutor.java
@@ -217,6 +217,16 @@ public interface QueryExecutor extends TypeTransferModeRegistry {
    */
   void processNotifies() throws SQLException;
 
+  /**
+   * Prior to attempting to retrieve notifications, we need to pull any recently received
+   * notifications off of the network buffers. The notification retrieval in ProtocolConnection
+   * cannot do this as it is prone to deadlock, so the higher level caller must be responsible which
+   * requires exposing this method. This variant supports blocking for the given time in millis.
+   *
+   * @throws SQLException if and error occurs while fetching notifications
+   */
+  void processNotifies(int timeoutMillis) throws SQLException;
+
   //
   // Fastpath interface.
   //
diff --git a/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java b/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java
index f8708187..732d3a29 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java
@@ -375,4 +375,8 @@ public abstract class QueryExecutorBase implements QueryExecutor {
   public void setFlushCacheOnDeallocate(boolean flushCacheOnDeallocate) {
     this.flushCacheOnDeallocate = flushCacheOnDeallocate;
   }
+
+  protected boolean hasNotifications() {
+    return notifications.size() > 0;
+  }
 }
diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
index b4c827da..25400f6f 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
@@ -45,6 +45,9 @@ import java.io.IOException;
 import java.lang.ref.PhantomReference;
 import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
+import java.net.Socket;
+import java.net.SocketException;
+import java.net.SocketTimeoutException;
 import java.sql.SQLException;
 import java.sql.SQLWarning;
 import java.sql.Statement;
@@ -644,35 +647,96 @@ public class QueryExecutorImpl extends QueryExecutorBase {
     pgStream.flush();
   }
 
+  // Just for API compatibility with previous versions.
   public synchronized void processNotifies() throws SQLException {
+    processNotifies(-1);
+  }
+
+  /**
+   * @param timeoutMillis when &gt; 0, block for this time
+   *                      when =0, block forever
+   *                      when &lt; 0, don't block
+   */
+  public synchronized void processNotifies(int timeoutMillis) throws SQLException {
     waitOnLock();
     // Asynchronous notifies only arrive when we are not in a transaction
     if (getTransactionState() != TransactionState.IDLE) {
       return;
     }
 
+    if (hasNotifications()) {
+      // No need to timeout when there are already notifications. We just check for more in this case.
+      timeoutMillis = -1;
+    }
+
+    boolean useTimeout = timeoutMillis > 0;
+    long startTime = 0;
+    int oldTimeout = 0;
+    if (useTimeout) {
+      startTime = System.currentTimeMillis();
+      try {
+        oldTimeout = pgStream.getSocket().getSoTimeout();
+      } catch (SocketException e) {
+        throw new PSQLException(GT.tr("An error occurred while trying to get the socket "
+          + "timeout."), PSQLState.CONNECTION_FAILURE, e);
+      }
+    }
+
     try {
-      while (pgStream.hasMessagePending()) {
+      while (pgStream.hasMessagePending() || timeoutMillis >= 0 ) {
+        if (useTimeout && timeoutMillis >= 0) {
+          setSocketTimeout(timeoutMillis);
+        }
         int c = pgStream.receiveChar();
+        if (useTimeout && timeoutMillis >= 0) {
+          setSocketTimeout(0); // Don't timeout after first char
+        }
         switch (c) {
           case 'A': // Asynchronous Notify
             receiveAsyncNotify();
-            break;
+            timeoutMillis = -1;
+            continue;
           case 'E':
             // Error Response (response to pretty much everything; backend then skips until Sync)
             throw receiveErrorResponse();
           case 'N': // Notice Response (warnings / info)
             SQLWarning warning = receiveNoticeResponse();
             addWarning(warning);
+            if (useTimeout) {
+              long newTimeMillis = System.currentTimeMillis();
+              timeoutMillis += startTime - newTimeMillis; // Overflows after 49 days, ignore that
+              startTime = newTimeMillis;
+              if (timeoutMillis == 0) {
+                timeoutMillis = -1; // Don't accidentially wait forever
+              }
+            }
             break;
           default:
             throw new PSQLException(GT.tr("Unknown Response Type {0}.", (char) c),
                 PSQLState.CONNECTION_FAILURE);
         }
       }
+    } catch (SocketTimeoutException ioe) {
+      // No notifications this time...
     } catch (IOException ioe) {
       throw new PSQLException(GT.tr("An I/O error occurred while sending to the backend."),
           PSQLState.CONNECTION_FAILURE, ioe);
+    } finally {
+      if (useTimeout) {
+        setSocketTimeout(oldTimeout);
+      }
+    }
+  }
+
+  private void setSocketTimeout(int millis) throws PSQLException {
+    try {
+      Socket s = pgStream.getSocket();
+      if (!s.isClosed()) { // Is this check required?
+        pgStream.getSocket().setSoTimeout(millis);
+      }
+    } catch (SocketException e) {
+      throw new PSQLException(GT.tr("An error occurred while trying to reset the socket timeout."),
+        PSQLState.CONNECTION_FAILURE, e);
     }
   }
 
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
index 4c0951a3..ac12ce88 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
@@ -973,8 +973,13 @@ public class PgConnection implements BaseConnection {
 
   @Override
   public PGNotification[] getNotifications() throws SQLException {
+    return getNotifications(-1);
+  }
+
+  @Override
+  public PGNotification[] getNotifications(int timeoutMillis) throws SQLException {
     checkClosed();
-    getQueryExecutor().processNotifies();
+    getQueryExecutor().processNotifies(timeoutMillis);
     // Backwards-compatibility hand-holding.
     PGNotification[] notifications = queryExecutor.getNotifications();
     return (notifications.length == 0 ? null : notifications);
diff --git a/pgjdbc/src/main/java/org/postgresql/util/PSQLWarning.java b/pgjdbc/src/main/java/org/postgresql/util/PSQLWarning.java
index 0a81fb0a..c52ff22d 100644
--- a/pgjdbc/src/main/java/org/postgresql/util/PSQLWarning.java
+++ b/pgjdbc/src/main/java/org/postgresql/util/PSQLWarning.java
@@ -13,17 +13,10 @@ public class PSQLWarning extends SQLWarning {
   private ServerErrorMessage serverError;
 
   public PSQLWarning(ServerErrorMessage err) {
+    super(err.toString(), err.getSQLState());
     this.serverError = err;
   }
 
-  public String toString() {
-    return serverError.toString();
-  }
-
-  public String getSQLState() {
-    return serverError.getSQLState();
-  }
-
   public String getMessage() {
     return serverError.getMessage();
   }
diff --git a/pgjdbc/src/main/java/org/postgresql/util/WriterHandler.java b/pgjdbc/src/main/java/org/postgresql/util/WriterHandler.java
new file mode 100644
index 00000000..822f9536
--- /dev/null
+++ b/pgjdbc/src/main/java/org/postgresql/util/WriterHandler.java
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.postgresql.util;
+
+import java.io.Writer;
+
+import java.util.logging.ErrorManager;
+import java.util.logging.Formatter;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import java.util.logging.SimpleFormatter;
+
+public class WriterHandler extends Handler {
+
+  private boolean doneHeader;
+
+  private Writer writer;
+
+  public WriterHandler(Writer inWriter) {
+    super();
+    setLevel(Level.INFO);
+    setFilter(null);
+    setFormatter(new SimpleFormatter());
+    writer = inWriter;
+  }
+
+  public WriterHandler(Writer inWriter, Formatter formatter) {
+    super();
+    setLevel(Level.INFO);
+    setFilter(null);
+    setFormatter(formatter);
+    writer = inWriter;
+
+  }
+
+  public synchronized void publish(final LogRecord record) {
+    if (!isLoggable(record)) {
+      return;
+    }
+    String msg;
+    try {
+      msg = getFormatter().format(record);
+    } catch (Exception ex) {
+      // We don't want to throw an exception here, but we
+      // report the exception to any registered ErrorManager.
+      reportError(null, ex, ErrorManager.FORMAT_FAILURE);
+      return;
+    }
+
+    try {
+      if (!doneHeader) {
+        writer.write(getFormatter().getHead(this));
+        doneHeader = true;
+      }
+      writer.write(msg);
+    } catch (Exception ex) {
+      // We don't want to throw an exception here, but we
+      // report the exception to any registered ErrorManager.
+      reportError(null, ex, ErrorManager.WRITE_FAILURE);
+    }
+  }
+
+  public boolean isLoggable(final LogRecord record) {
+    if (writer == null || record == null) {
+      return false;
+    }
+    return super.isLoggable(record);
+  }
+
+  public synchronized void flush() {
+    if (writer != null) {
+      try {
+        writer.flush();
+      } catch (Exception ex) {
+        // We don't want to throw an exception here, but we
+        // report the exception to any registered ErrorManager.
+        reportError(null, ex, ErrorManager.FLUSH_FAILURE);
+      }
+    }
+  }
+
+  private synchronized void flushAndClose() throws SecurityException {
+
+    if (writer != null) {
+      try {
+        if (!doneHeader) {
+          writer.write(getFormatter().getHead(this));
+          doneHeader = true;
+        }
+        writer.write(getFormatter().getTail(this));
+        writer.flush();
+        writer.close();
+      } catch (Exception ex) {
+        // We don't want to throw an exception here, but we
+        // report the exception to any registered ErrorManager.
+        reportError(null, ex, ErrorManager.CLOSE_FAILURE);
+      }
+      writer = null;
+
+    }
+  }
+
+  public synchronized void close() throws SecurityException {
+    flushAndClose();
+  }
+}


Test patch:
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
index 9d26beaf..07416d81 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
@@ -13,15 +13,20 @@ import static org.junit.Assert.fail;
 
 import org.postgresql.Driver;
 import org.postgresql.test.TestUtil;
+import org.postgresql.util.NullOutputStream;
+import org.postgresql.util.WriterHandler;
 
 import org.junit.Test;
 
+import java.io.PrintWriter;
 import java.lang.reflect.Method;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Properties;
+import java.util.logging.Handler;
+import java.util.logging.Logger;
 
 /*
  * Tests the dynamically created class org.postgresql.Driver
@@ -180,4 +185,74 @@ public class DriverTest {
     }
     fail("Driver has not been found in DriverManager's list but it should be registered");
   }
+
+  @Test
+  public void testSetLogWriter() throws Exception {
+
+    // this is a dummy to make sure TestUtil is initialized
+    Connection con = DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
+    con.close();
+    String loggerLevel = System.getProperty("loggerLevel");
+    String loggerFile = System.getProperty("loggerFile");
+
+    try {
+
+      PrintWriter printWriter = new PrintWriter(new NullOutputStream(System.err));
+      DriverManager.setLogWriter(printWriter);
+      assertEquals(DriverManager.getLogWriter(), printWriter);
+      System.clearProperty("loggerFile");
+      System.clearProperty("loggerLevel");
+      Properties props = new Properties();
+      props.setProperty("user", TestUtil.getUser());
+      props.setProperty("password", TestUtil.getPassword());
+      props.setProperty("loggerLevel", "DEBUG");
+      con = DriverManager.getConnection(TestUtil.getURL(), props);
+
+      Logger logger = Logger.getLogger("org.postgresql");
+      Handler[] handlers = logger.getHandlers();
+      assertTrue(handlers[0] instanceof WriterHandler );
+      con.close();
+    } finally {
+      DriverManager.setLogWriter(null);
+      System.setProperty("loggerLevel", loggerLevel);
+      System.setProperty("loggerFile", loggerFile);
+
+    }
+
+  }
+
+  @Test
+  public void testSetLogStream() throws Exception {
+
+    // this is a dummy to make sure TestUtil is initialized
+    Connection con = DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
+    con.close();
+    String loggerLevel = System.getProperty("loggerLevel");
+    String loggerFile = System.getProperty("loggerFile");
+
+    try {
+
+      DriverManager.setLogStream(new NullOutputStream(System.err));
+      System.clearProperty("loggerFile");
+      System.clearProperty("loggerLevel");
+      Properties props = new Properties();
+      props.setProperty("user", TestUtil.getUser());
+      props.setProperty("password", TestUtil.getPassword());
+      props.setProperty("loggerLevel", "DEBUG");
+      con = DriverManager.getConnection(TestUtil.getURL(), props);
+
+      Logger logger = Logger.getLogger("org.postgresql");
+      Handler []handlers = logger.getHandlers();
+      assertTrue( handlers[0] instanceof WriterHandler );
+      con.close();
+    } finally {
+      DriverManager.setLogStream(null);
+      System.setProperty("loggerLevel", loggerLevel);
+      System.setProperty("loggerFile", loggerFile);
+
+
+    }
+
+  }
+
 }
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/NotifyTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/NotifyTest.java
index 7c0daa4d..d4d7206b 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/NotifyTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/NotifyTest.java
@@ -7,12 +7,14 @@ package org.postgresql.test.jdbc2;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 import org.postgresql.PGNotification;
 import org.postgresql.core.ServerVersion;
 import org.postgresql.test.TestUtil;
 
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -33,7 +35,7 @@ public class NotifyTest {
     TestUtil.closeDB(conn);
   }
 
-  @Test
+  @Test(timeout = 60000)
   public void testNotify() throws SQLException {
     Statement stmt = conn.createStatement();
     stmt.executeUpdate("LISTEN mynotification");
@@ -48,7 +50,7 @@ public class NotifyTest {
     stmt.close();
   }
 
-  @Test
+  @Test(timeout = 60000)
   public void testNotifyArgument() throws Exception {
     if (!TestUtil.haveMinimumServerVersion(conn, ServerVersion.v9_0) || TestUtil.isProtocolVersion(conn, 2)) {
       return;
@@ -67,28 +69,26 @@ public class NotifyTest {
     stmt.close();
   }
 
-  @Test
+  @Test(timeout = 60000)
   public void testAsyncNotify() throws Exception {
     Statement stmt = conn.createStatement();
     stmt.executeUpdate("LISTEN mynotification");
 
     // Notify on a separate connection to get an async notify on the first.
-    Connection conn2 = TestUtil.openDB();
-    try {
-      Statement stmt2 = conn2.createStatement();
-      stmt2.executeUpdate("NOTIFY mynotification");
-      stmt2.close();
-    } finally {
-      conn2.close();
-    }
+    connectAndNotify("mynotification");
 
-    // Wait a bit to let the notify come through..
+    // Wait a bit to let the notify come through... Changed this so the test takes ~2 seconds
+    // less to run and is still as effective.
+    PGNotification notifications[] = null;
     try {
-      Thread.sleep(2000);
+      int retries = 20;
+      while (retries-- > 0
+        && (notifications = ((org.postgresql.PGConnection) conn).getNotifications()) == null ) {
+        Thread.sleep(100);
+      }
     } catch (InterruptedException ie) {
     }
 
-    PGNotification notifications[] = ((org.postgresql.PGConnection) conn).getNotifications();
     assertNotNull(notifications);
     assertEquals(1, notifications.length);
     assertEquals("mynotification", notifications[0].getName());
@@ -96,4 +96,160 @@ public class NotifyTest {
 
     stmt.close();
   }
+
+  /**
+   * To test timeouts we have to send the notification from another thread, because we
+   * listener is blocking.
+   */
+  @Test(timeout = 60000)
+  public void testAsyncNotifyWithTimeout() throws Exception {
+    Statement stmt = conn.createStatement();
+    stmt.executeUpdate("LISTEN mynotification");
+
+    // Here we let the getNotifications() timeout.
+    long startMillis = System.currentTimeMillis();
+    PGNotification notifications[] = ((org.postgresql.PGConnection) conn).getNotifications(500);
+    long endMillis = System.currentTimeMillis();
+    long runtime = endMillis - startMillis;
+    assertNull("There have been notifications, althought none have been expected.",notifications);
+    Assert.assertTrue("We didn't wait long enough! runtime=" + runtime, runtime > 450);
+
+    stmt.close();
+  }
+
+  @Test(timeout = 60000)
+  public void testAsyncNotifyWithTimeoutAndMessagesAvailableWhenStartingListening() throws Exception {
+    Statement stmt = conn.createStatement();
+    stmt.executeUpdate("LISTEN mynotification");
+
+    // Now we check the case where notifications are already available while we are starting to
+    // listen for notifications
+    connectAndNotify("mynotification");
+
+    PGNotification notifications[] = ((org.postgresql.PGConnection) conn).getNotifications(10000);
+    assertNotNull(notifications);
+    assertEquals(1, notifications.length);
+    assertEquals("mynotification", notifications[0].getName());
+    assertEquals("", notifications[0].getParameter());
+
+    stmt.close();
+  }
+
+  @Test(timeout = 60000)
+  public void testAsyncNotifyWithEndlessTimeoutAndMessagesAvailableWhenStartingListening() throws Exception {
+    Statement stmt = conn.createStatement();
+    stmt.executeUpdate("LISTEN mynotification");
+
+    // Now we check the case where notifications are already available while we are waiting forever
+    connectAndNotify("mynotification");
+
+    PGNotification notifications[] = ((org.postgresql.PGConnection) conn).getNotifications(0);
+    assertNotNull(notifications);
+    assertEquals(1, notifications.length);
+    assertEquals("mynotification", notifications[0].getName());
+    assertEquals("", notifications[0].getParameter());
+
+    stmt.close();
+  }
+
+  @Test(timeout = 60000)
+  public void testAsyncNotifyWithTimeoutAndMessagesSendAfter() throws Exception {
+    Statement stmt = conn.createStatement();
+    stmt.executeUpdate("LISTEN mynotification");
+
+    // Now we check the case where notifications are send after we have started to listen for
+    // notifications
+    new Thread( new Runnable() {
+      public void run() {
+        try {
+          Thread.sleep(200);
+        } catch (InterruptedException ie) {
+        }
+        connectAndNotify("mynotification");
+      }
+    }).start();
+
+    PGNotification notifications[] = ((org.postgresql.PGConnection) conn).getNotifications(10000);
+    assertNotNull(notifications);
+    assertEquals(1, notifications.length);
+    assertEquals("mynotification", notifications[0].getName());
+    assertEquals("", notifications[0].getParameter());
+
+    stmt.close();
+  }
+
+  @Test(timeout = 60000)
+  public void testAsyncNotifyWithEndlessTimeoutAndMessagesSendAfter() throws Exception {
+    Statement stmt = conn.createStatement();
+    stmt.executeUpdate("LISTEN mynotification");
+
+    // Now we check the case where notifications are send after we have started to listen for
+    // notifications forever
+    new Thread( new Runnable() {
+      public void run() {
+        try {
+          Thread.sleep(200);
+        } catch (InterruptedException ie) {
+        }
+        connectAndNotify("mynotification");
+      }
+    }).start();
+
+    PGNotification notifications[] = ((org.postgresql.PGConnection) conn).getNotifications(0);
+    assertNotNull(notifications);
+    assertEquals(1, notifications.length);
+    assertEquals("mynotification", notifications[0].getName());
+    assertEquals("", notifications[0].getParameter());
+
+    stmt.close();
+  }
+
+  @Test(timeout = 60000)
+  public void testAsyncNotifyWithTimeoutAndSocketThatBecomesClosed() throws Exception {
+    Statement stmt = conn.createStatement();
+    stmt.executeUpdate("LISTEN mynotification");
+
+    // Here we check what happens when the connection gets closed from another thread. This
+    // should be able, and this test ensures that no synchronized statements will stop the
+    // connection from becoming closed.
+    new Thread( new Runnable() {
+      public void run() {
+        try {
+          Thread.sleep(500);
+        } catch (InterruptedException ie) {
+        }
+        try {
+          conn.close();
+        } catch (SQLException e) {
+        }
+      }
+    }).start();
+
+    try {
+      ((org.postgresql.PGConnection) conn).getNotifications(40000);
+      Assert.fail("The getNotifications(...) call didn't return when the socket closed.");
+    } catch (SQLException e) {
+      // We expected thta
+    }
+
+    stmt.close();
+  }
+
+  private static void connectAndNotify(String channel) {
+    Connection conn2 = null;
+    try {
+      conn2 = TestUtil.openDB();
+      Statement stmt2 = conn2.createStatement();
+      stmt2.executeUpdate("NOTIFY " + channel);
+      stmt2.close();
+    } catch (Exception e) {
+      throw new RuntimeException("Couldn't notify '" + channel + "'.",e);
+    } finally {
+      try {
+        conn2.close();
+      } catch (SQLException e) {
+      }
+    }
+  }
+
 }
diff --git a/pgjdbc/src/test/java/org/postgresql/util/NullOutputStream.java b/pgjdbc/src/test/java/org/postgresql/util/NullOutputStream.java
new file mode 100644
index 00000000..764dc1ac
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/util/NullOutputStream.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.util;
+
+import java.io.OutputStream;
+import java.io.PrintStream;
+
+/**
+ * Created by davec on 3/14/17.
+ */
+public class NullOutputStream extends PrintStream {
+
+  public NullOutputStream(OutputStream out) {
+    super(out);
+  }
+
+  @Override
+  public void write(int b) {
+
+  }
+
+  @Override
+  public void write(byte[] buf, int off, int len) {
+
+  }
+}
diff --git a/pgjdbc/src/test/java/org/postgresql/util/PSQLWarningTest.java b/pgjdbc/src/test/java/org/postgresql/util/PSQLWarningTest.java
new file mode 100644
index 00000000..04853ddf
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/util/PSQLWarningTest.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2016, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.util;
+
+import static org.junit.Assert.assertTrue;
+
+import org.postgresql.test.TestUtil;
+
+import org.junit.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.Statement;
+
+
+public class PSQLWarningTest {
+
+  @Test
+  public void testPSQLLogsToDriverManagerMessage() throws Exception {
+    Connection con = TestUtil.openDB();
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    DriverManager.setLogWriter(new PrintWriter(new OutputStreamWriter(baos, "ASCII")));
+
+    Statement stmt = con.createStatement();
+    stmt.execute("DO language plpgsql $$ BEGIN RAISE NOTICE 'test notice'; END $$;");
+    assertTrue(baos.toString().contains("NOTICE: test notice"));
+
+    stmt.close();
+    con.close();
+  }
+}