You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by ashuparekh <gi...@git.apache.org> on 2018/08/24 02:08:41 UTC
[GitHub] phoenix pull request #336: PHOENIX-4864 Fix NullPointerException while Loggi...
GitHub user ashuparekh opened a pull request:
https://github.com/apache/phoenix/pull/336
PHOENIX-4864 Fix NullPointerException while Logging some DDL Statements
Fixed the error. Still to add corresponding test case. @vincentpoon @karanmehta93
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ashuparekh/phoenix 4.x-HBase-1.4
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/phoenix/pull/336.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #336
----
commit e34d4ee4e81ede77a3076815f86c2d2d10917e5a
Author: Ashutosh <ap...@...>
Date: 2018-08-23T19:11:23Z
PHOENIX-4864 Fix NullPointerException while Logging some DDL Statements
----
---
[GitHub] phoenix pull request #336: PHOENIX-4864 Fix NullPointerException while Loggi...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/336#discussion_r212756649
--- Diff: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java ---
@@ -91,19 +125,19 @@ public void testPhoenixMetricsLoggedOnCommit() throws Exception {
// Assert that metrics logging happens only once
loggedConn.close();
- assertTrue("Mutation write metrics are not logged again.",
- mutationWriteMetricsMap.size() == 0);
- assertTrue("Mutation read metrics are not logged again.",
- mutationReadMetricsMap.size() == 0);
+ assertTrue("Mutation write metrics are not logged again.", mutationWriteMetricsMap.size()
--- End diff --
nit: unnecessary diff (in lot of places)
---
[GitHub] phoenix issue #336: PHOENIX-4864 Fix NullPointerException while Logging some...
Posted by ashuparekh <gi...@git.apache.org>.
Github user ashuparekh commented on the issue:
https://github.com/apache/phoenix/pull/336
Added the corresponding test case. @vincentpoon @karanmehta93
---
[GitHub] phoenix pull request #336: PHOENIX-4864 Fix NullPointerException while Loggi...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/336#discussion_r212757022
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java ---
@@ -45,7 +45,9 @@ public ResultSet executeQuery() throws SQLException {
@Override
public ResultSet getResultSet() throws SQLException {
- return new LoggingPhoenixResultSet(super.getResultSet(), phoenixMetricsLog, sql);
+ ResultSet resultSet = super.getResultSet();
--- End diff --
nit: add a comment that call to `getResultSet()` is not idempotent. Hence we cache and return accordingly.
---
[GitHub] phoenix pull request #336: PHOENIX-4864 Fix NullPointerException while Loggi...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/336#discussion_r212550265
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java ---
@@ -45,7 +45,9 @@ public ResultSet executeQuery() throws SQLException {
@Override
public ResultSet getResultSet() throws SQLException {
- return new LoggingPhoenixResultSet(super.getResultSet(), phoenixMetricsLog, sql);
+ ResultSet resultSet = super.getResultSet();
+ return (resultSet == null) ? null : new LoggingPhoenixResultSet(super.getResultSet(),
--- End diff --
This is still a bug. `super.getResultSet()` call is not idempotent. You should replace this with `resultSet` local variable
---
[GitHub] phoenix issue #336: PHOENIX-4864 Fix NullPointerException while Logging some...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on the issue:
https://github.com/apache/phoenix/pull/336
Nice test coverage @ashuparekh
This PR is almost good to go!
---
[GitHub] phoenix pull request #336: PHOENIX-4864 Fix NullPointerException while Loggi...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/336#discussion_r212756862
--- Diff: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java ---
@@ -32,6 +35,7 @@
import static org.junit.Assert.assertTrue;
+
--- End diff --
nit
---
[GitHub] phoenix issue #336: PHOENIX-4864 Fix NullPointerException while Logging some...
Posted by ashuparekh <gi...@git.apache.org>.
Github user ashuparekh commented on the issue:
https://github.com/apache/phoenix/pull/336
@karanmehta93 Incorporated the above suggestions
---