PHOENIX-4864 Fix NullPointerException while Logging some DDL Statements

apache · Aug 24, 2018 · e43db87 · e43db87
1 parent 3b9a108
commit e43db87
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 3 deletions.
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java
@@ -20,6 +20,7 @@
 import com.google.common.collect.Maps;
 import org.apache.phoenix.jdbc.LoggingPhoenixConnection;
 import org.apache.phoenix.jdbc.PhoenixMetricsLog;
+import org.apache.phoenix.jdbc.LoggingPhoenixResultSet;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -68,6 +69,35 @@ public void beforeTest() throws Exception {
         loggedConn = getLoggingPhoenixConnection(testConn);
     }
 
+    @Test
+    public void testResultSetTypeForQueries() throws Exception {
+        String tableName3 = generateUniqueName();
+
+        String create = "CREATE TABLE " + tableName3 + " (K INTEGER PRIMARY KEY)";
+        assertTrue(executeAndGetResultSet(create) == null);
+
+        String upsert = "UPSERT INTO " + tableName3 + " VALUES (42)";
+        assertTrue(executeAndGetResultSet(upsert) == null);
+
+        String select = "SELECT * FROM " + tableName3;
+        assertTrue(executeAndGetResultSet(select) instanceof LoggingPhoenixResultSet);
+
+        String createView = "CREATE VIEW TEST_VIEW (K INTEGER) AS SELECT * FROM " + tableName3;
+        assertTrue(executeAndGetResultSet(createView) == null);
+
+        String createIndex = "CREATE INDEX TEST_INDEX ON " + tableName3 + " (K)";
+        assertTrue(executeAndGetResultSet(createIndex) == null);
+
+        String dropIndex = "DROP INDEX TEST_INDEX ON " + tableName3;
+        assertTrue(executeAndGetResultSet(dropIndex) == null);
+
+        String dropView = "DROP VIEW TEST_VIEW";
+        assertTrue(executeAndGetResultSet(dropView) == null);
+
+        String dropTable = "DROP TABLE " + tableName3;
+        assertTrue(executeAndGetResultSet(dropTable) == null);
+    }
+
     @Test
     public void testPhoenixMetricsLoggedOnCommit() throws Exception {
         // run SELECT to verify read metrics are logged
@@ -134,12 +164,22 @@ public void testPhoenixMetricsLoggedOnClose() throws Exception {
                 mutationReadMetricsMap.size() == 0);
     }
 
+    private ResultSet executeAndGetResultSet(String query) throws Exception {
+        Statement stmt = loggedConn.createStatement();
+        stmt.execute(query);
+        return stmt.getResultSet();
+    }
+
     private void verifyQueryLevelMetricsLogging(String query) throws SQLException {
         Statement stmt = loggedConn.createStatement();
         ResultSet rs = stmt.executeQuery(query);
+        assertTrue(rs instanceof LoggingPhoenixResultSet);
+        int rowsRetrievedCounter = 0;
         while (rs.next()) {
+            rowsRetrievedCounter++;
         }
         rs.close();
+        assertTrue(rowsRetrievedCounter == NUM_ROWS);
         assertTrue("Read metrics for not found for " + tableName1,
                 requestReadMetricsMap.get(tableName1).size() > 0);
         assertTrue("Logged query doesn't match actual query", loggedSql.equals(query));
@@ -192,4 +232,4 @@ public void logReadMetricInfoForMutationsSinceLastReset(
             }
         });
     }
-}
+}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java
@@ -45,7 +45,10 @@ public ResultSet executeQuery() throws SQLException {
 
     @Override
     public ResultSet getResultSet() throws SQLException {
-        return new LoggingPhoenixResultSet(super.getResultSet(), phoenixMetricsLog, sql);
+        // We cache the ResultSet value and reuse since call to getResultSet() is not idempotent
+        ResultSet resultSet = super.getResultSet();
+        return (resultSet == null) ? null : new LoggingPhoenixResultSet(resultSet,
+                phoenixMetricsLog, sql);
     }
 
     @Override

diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java
@@ -51,7 +51,10 @@ public int executeUpdate(String sql) throws SQLException {
 
     @Override
     public ResultSet getResultSet() throws SQLException {
-        return new LoggingPhoenixResultSet(super.getResultSet(), phoenixMetricsLog, this.sql);
+        // We cache the ResultSet value and reuse since call to getResultSet() is not idempotent
+        ResultSet resultSet = super.getResultSet();
+        return (resultSet == null) ? null : new LoggingPhoenixResultSet(resultSet,
+                phoenixMetricsLog, sql);
     }
 
     @Override