From 92b7fd4fe598fc339b4ac88aa0b031bc99a6911d Mon Sep 17 00:00:00 2001
From: Kevin Wooten <kevin@wooten.com>
Date: Thu, 25 Jun 2020 13:43:20 -0700
Subject: [PATCH] Fix string based null handling for scalars, records & arrays
 (#494)

---
 .../postgres/system/procs/Arrays.java         | 12 ++-
 .../postgres/system/procs/BaseDecoders.java   |  4 -
 .../postgres/system/procs/Records.java        |  8 +-
 .../impossibl/postgres/jdbc/ArrayTest.java    | 89 +++++++++++++++++++
 .../postgres/jdbc/ResultSetTest.java          | 32 +++++++
 .../impossibl/postgres/jdbc/StructTest.java   | 50 ++++++++++-
 6 files changed, 179 insertions(+), 16 deletions(-)

diff --git a/driver/src/main/java/com/impossibl/postgres/system/procs/Arrays.java b/driver/src/main/java/com/impossibl/postgres/system/procs/Arrays.java
index ed1e2855c..be21aaf01 100644
--- a/driver/src/main/java/com/impossibl/postgres/system/procs/Arrays.java
+++ b/driver/src/main/java/com/impossibl/postgres/system/procs/Arrays.java
@@ -284,6 +284,7 @@ int[] parseElementBuffers(char delim, CharSequence buffer, List<CharSequence> el
       int[] dimensions = new int[0];
       int len = buffer.length();
       StringBuilder elementText = null;
+      boolean quoted = false;
 
       int depth = -1;
       int charIdx;
@@ -305,7 +306,8 @@ int[] parseElementBuffers(char delim, CharSequence buffer, List<CharSequence> el
           case '}':
             if (elementText != null) {
               ++dimensions[depth];
-              elementText = addTextElement(elementText, elements);
+              elementText = addTextElement(elementText, quoted, elements);
+              quoted = false;
             }
             if (--depth < 0) {
               break scan;
@@ -318,6 +320,7 @@ int[] parseElementBuffers(char delim, CharSequence buffer, List<CharSequence> el
           case '"':
             elementText = new StringBuilder();
             charIdx = readString(buffer, charIdx, elementText);
+            quoted = true;
             break;
 
           case '[':
@@ -337,7 +340,8 @@ int[] parseElementBuffers(char delim, CharSequence buffer, List<CharSequence> el
             if (ch == delim) {
               if (elementText != null) {
                 ++dimensions[depth];
-                elementText = addTextElement(elementText, elements);
+                elementText = addTextElement(elementText, quoted, elements);
+                quoted = false;
               }
               break;
             }
@@ -353,9 +357,9 @@ int[] parseElementBuffers(char delim, CharSequence buffer, List<CharSequence> el
       return dimensions;
     }
 
-    StringBuilder addTextElement(StringBuilder text, List<CharSequence> elements) {
+    StringBuilder addTextElement(StringBuilder text, boolean quoted, List<CharSequence> elements) {
       String textStr = text.toString();
-      if (textStr.equalsIgnoreCase("NULL")) {
+      if (!quoted && textStr.equalsIgnoreCase("NULL")) {
         elements.add(null);
       }
       else {
diff --git a/driver/src/main/java/com/impossibl/postgres/system/procs/BaseDecoders.java b/driver/src/main/java/com/impossibl/postgres/system/procs/BaseDecoders.java
index 525105356..a756b989b 100644
--- a/driver/src/main/java/com/impossibl/postgres/system/procs/BaseDecoders.java
+++ b/driver/src/main/java/com/impossibl/postgres/system/procs/BaseDecoders.java
@@ -210,10 +210,6 @@ public Object decode(Context context, Type type, Short typeLength, Integer typeM
 
     targetClass = targetClass != null ? targetClass : getDefaultClass();
 
-    if (buffer.length() == 4 && buffer.toString().equalsIgnoreCase("NULL")) {
-      return null;
-    }
-
     try {
       return decodeValue(context, type, typeLength, typeModifier, buffer, targetClass, targetContext);
     }
diff --git a/driver/src/main/java/com/impossibl/postgres/system/procs/Records.java b/driver/src/main/java/com/impossibl/postgres/system/procs/Records.java
index a82bc7303..e5f307f6d 100644
--- a/driver/src/main/java/com/impossibl/postgres/system/procs/Records.java
+++ b/driver/src/main/java/com/impossibl/postgres/system/procs/Records.java
@@ -265,13 +265,7 @@ StringBuilder addTextElement(StringBuilder text, boolean empty, List<CharSequenc
         attributes.add(null);
       }
       else {
-        String textStr = text.toString();
-        if (textStr.equalsIgnoreCase("NULL")) {
-          attributes.add(null);
-        }
-        else {
-          attributes.add(textStr);
-        }
+        attributes.add(text.toString());
       }
       return null;
     }
diff --git a/driver/src/test/java/com/impossibl/postgres/jdbc/ArrayTest.java b/driver/src/test/java/com/impossibl/postgres/jdbc/ArrayTest.java
index 1989fba91..63da51235 100644
--- a/driver/src/test/java/com/impossibl/postgres/jdbc/ArrayTest.java
+++ b/driver/src/test/java/com/impossibl/postgres/jdbc/ArrayTest.java
@@ -112,6 +112,20 @@ public void testGetNull() throws SQLException {
     stmt.close();
   }
 
+  @Test
+  public void testTextModeGetNull() throws SQLException {
+    Statement stmt = conn.createStatement();
+
+    ResultSet rs = stmt.executeQuery("SELECT null; SELECT 1");
+    assertTrue(rs.next());
+    assertNull(rs.getArray(1));
+    assertNull(rs.getObject(1));
+    assertArrayEquals(null, rs.getObject(1, Integer[].class));
+
+    rs.close();
+    stmt.close();
+  }
+
   @Test
   public void testSendRecvMultiple() throws SQLException {
 
@@ -359,6 +373,81 @@ public void testNullValues() throws SQLException {
     stmt.close();
   }
 
+  @Test
+  public void testNullValuesTextMode() throws SQLException {
+
+    Statement stmt = conn.createStatement();
+    ResultSet rs = stmt.executeQuery("SELECT ARRAY[1,NULL,3]; SELECT 1");
+    assertTrue(rs.next());
+    Array arr = rs.getArray(1);
+    Integer[] i = (Integer[]) arr.getArray();
+    assertEquals(3, i.length);
+    assertEquals(1, i[0].intValue());
+    assertNull(i[1]);
+    assertEquals(3, i[2].intValue());
+    arr.free();
+    rs.close();
+    stmt.close();
+  }
+
+  @Test
+  public void testStringNullValues() throws SQLException {
+
+    try (Statement stmt = conn.createStatement()) {
+      ResultSet rs = stmt.executeQuery("SELECT ARRAY['null',NULL,'null',NULL,'null']");
+      assertTrue(rs.next());
+      Array arr = rs.getArray(1);
+      String[] i = (String[]) arr.getArray();
+      assertEquals(5, i.length);
+      assertEquals("null", i[0]);
+      assertEquals(null, i[1]);
+      assertEquals("null", i[2]);
+      assertEquals(null, i[3]);
+      assertEquals("null", i[4]);
+      arr.free();
+      rs.close();
+    }
+
+
+    try (Statement stmt = conn.createStatement()) {
+      ResultSet rs = stmt.executeQuery("SELECT ARRAY[NULL,'null',NULL,'null',NULL]");
+      assertTrue(rs.next());
+      Array arr = rs.getArray(1);
+      String[] i = (String[]) arr.getArray();
+      assertEquals(5, i.length);
+      assertEquals(null, i[0]);
+      assertEquals("null", i[1]);
+      assertEquals(null, i[2]);
+      assertEquals("null", i[3]);
+      assertEquals(null, i[4]);
+      arr.free();
+      rs.close();
+    }
+
+    try (Statement stmt = conn.createStatement()) {
+      ResultSet rs = stmt.executeQuery("SELECT ARRAY['null']");
+      assertTrue(rs.next());
+      Array arr = rs.getArray(1);
+      String[] i = (String[]) arr.getArray();
+      assertEquals(1, i.length);
+      assertEquals("null", i[0]);
+      arr.free();
+      rs.close();
+    }
+
+    try (Statement stmt = conn.createStatement()) {
+      ResultSet rs = stmt.executeQuery("SELECT ARRAY[null]::varchar[]");
+      assertTrue(rs.next());
+      Array arr = rs.getArray(1);
+      String[] i = (String[]) arr.getArray();
+      assertEquals(1, i.length);
+      assertEquals(null, i[0]);
+      arr.free();
+      rs.close();
+    }
+
+  }
+
   @Test
   public void testUnknownArrayType() throws SQLException {
     Statement stmt = conn.createStatement();
diff --git a/driver/src/test/java/com/impossibl/postgres/jdbc/ResultSetTest.java b/driver/src/test/java/com/impossibl/postgres/jdbc/ResultSetTest.java
index 922236d6d..c6c6fc7fc 100644
--- a/driver/src/test/java/com/impossibl/postgres/jdbc/ResultSetTest.java
+++ b/driver/src/test/java/com/impossibl/postgres/jdbc/ResultSetTest.java
@@ -64,6 +64,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -1010,4 +1011,35 @@ public void testTurkishLocale() throws SQLException {
     }
   }
 
+  @Test
+  public void testReadStringNull() throws Exception {
+    try (PreparedStatement pst = con.prepareStatement("INSERT INTO teststring(a) VALUES (?)")) {
+      pst.setString(1, "null");
+      pst.executeUpdate();
+      pst.setString(1, null);
+      pst.executeUpdate();
+    }
+
+    try (Statement st = con.createStatement()) {
+      ResultSet rs = st.executeQuery("SELECT a FROM teststring");
+      assertTrue(rs.next());
+      assertEquals("12345", rs.getString(1));
+      assertTrue(rs.next());
+      assertEquals("null", rs.getString(1));
+      assertTrue(rs.next());
+      assertNull(rs.getString(1));
+    }
+
+    // forced into text mode
+    try (Statement st = con.createStatement()) {
+      ResultSet rs = st.executeQuery("SELECT a FROM teststring; SELECT 1");
+      assertTrue(rs.next());
+      assertEquals("12345", rs.getString(1));
+      assertTrue(rs.next());
+      assertEquals("null", rs.getString(1));
+      assertTrue(rs.next());
+      assertNull(rs.getString(1));
+    }
+  }
+
 }
diff --git a/driver/src/test/java/com/impossibl/postgres/jdbc/StructTest.java b/driver/src/test/java/com/impossibl/postgres/jdbc/StructTest.java
index 57e3d12ff..517eb2167 100644
--- a/driver/src/test/java/com/impossibl/postgres/jdbc/StructTest.java
+++ b/driver/src/test/java/com/impossibl/postgres/jdbc/StructTest.java
@@ -275,7 +275,7 @@ public void testConnectionTypeMapFail() throws SQLException {
     assertTrue(rs.next());
 
     try {
-      TestStruct ts2 = (TestStruct) rs.getObject(1);
+      @SuppressWarnings("unused") TestStruct ts2 = (TestStruct) rs.getObject(1);
       Assert.fail("Cast should have failed");
     }
     catch (ClassCastException e) {
@@ -288,4 +288,52 @@ public void testConnectionTypeMapFail() throws SQLException {
 
   }
 
+  @Test
+  public void testTextModeNullHandling() throws SQLException {
+
+    TestStruct ts = new TestStruct(), ts2;
+    ts.str = "null";
+    ts.str2 = null;
+
+    PreparedStatement pst = conn.prepareStatement("INSERT INTO struct_test VALUES (?)");
+    pst.setObject(1, ts);
+    pst.executeUpdate();
+    pst.close();
+
+    Statement st = conn.createStatement();
+    ResultSet rs = st.executeQuery("SELECT * FROM struct_test; SELECT 1;");
+    assertTrue(rs.next());
+    assertNotNull(ts2 = rs.getObject(1, TestStruct.class));
+    assertEquals("null", ts2.str);
+    assertNull(ts2.str2);
+    assertNull(ts2.id);
+    assertNull(ts2.num);
+    rs.close();
+    st.close();
+  }
+
+  @Test
+  public void testNullHandling() throws SQLException {
+
+    TestStruct ts = new TestStruct(), ts2;
+    ts.str = "null";
+    ts.str2 = null;
+
+    PreparedStatement pst = conn.prepareStatement("INSERT INTO struct_test VALUES (?)");
+    pst.setObject(1, ts);
+    pst.executeUpdate();
+    pst.close();
+
+    Statement st = conn.createStatement();
+    ResultSet rs = st.executeQuery("SELECT * FROM struct_test;");
+    assertTrue(rs.next());
+    assertNotNull(ts2 = rs.getObject(1, TestStruct.class));
+    assertEquals("null", ts2.str);
+    assertNull(ts2.str2);
+    assertNull(ts2.id);
+    assertNull(ts2.num);
+    rs.close();
+    st.close();
+  }
+
 }