You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/03/10 18:50:56 UTC

[GitHub] [phoenix] siddhimehta opened a new pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

siddhimehta opened a new pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731
 
 
   Master patch

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [phoenix] swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731#discussion_r391394340
 
 

 ##########
 File path: phoenix-core/src/test/java/org/apache/phoenix/execute/MutationStateTest.java
 ##########
 @@ -175,4 +188,36 @@ public void testGetMutationBatchList() {
         }
 
     }
+
+    @Rule
+    public ExpectedException exceptionRule = ExpectedException.none();
+
+    @Test
+    public void testPendingMutationsOnDDL() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        props.setProperty(QueryServices.PENDING_MUTATIONS_DDL_THROW_ATTRIB, "true");
+        try (Connection conn = DriverManager.getConnection(getUrl(), props);
+                PhoenixConnection spy = spy((PhoenixConnection) conn)) {
 
 Review comment:
   nit: variable name from spy to suffixed with spy, something like pConnSpy etc. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [phoenix] swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731#discussion_r391394885
 
 

 ##########
 File path: phoenix-core/src/main/java/org/apache/phoenix/query/QueryServices.java
 ##########
 @@ -19,14 +19,14 @@
 
 import java.util.concurrent.ThreadPoolExecutor;
 
-import net.jcip.annotations.Immutable;
-
 import org.apache.phoenix.iterate.SpoolTooBigToDiskException;
 import org.apache.phoenix.memory.MemoryManager;
 import org.apache.phoenix.optimize.QueryOptimizer;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.apache.phoenix.util.SQLCloseable;
 
+import net.jcip.annotations.Immutable;
 
 Review comment:
   Is it used anywhere?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [phoenix] swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731#discussion_r391393732
 
 

 ##########
 File path: phoenix-core/src/test/java/org/apache/phoenix/schema/MutationTest.java
 ##########
 @@ -55,9 +55,13 @@ private void testDurability(boolean disableWAL) throws Exception {
             assertDurability(conn,expectedDurability);
             conn.createStatement().execute("DELETE FROM t1 WHERE k=1");
             assertDurability(conn,expectedDurability);
-            conn.createStatement().execute("DROP TABLE t1");
         } finally {
-            conn.close();
+            try {
+                conn.rollback();
 
 Review comment:
   want to use try-with-resource instead?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [phoenix] swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731#discussion_r391394473
 
 

 ##########
 File path: phoenix-core/src/test/java/org/apache/phoenix/execute/MutationStateTest.java
 ##########
 @@ -175,4 +188,36 @@ public void testGetMutationBatchList() {
         }
 
     }
+
+    @Rule
+    public ExpectedException exceptionRule = ExpectedException.none();
+
+    @Test
+    public void testPendingMutationsOnDDL() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        props.setProperty(QueryServices.PENDING_MUTATIONS_DDL_THROW_ATTRIB, "true");
+        try (Connection conn = DriverManager.getConnection(getUrl(), props);
+                PhoenixConnection spy = spy((PhoenixConnection) conn)) {
+            MutationState mutationState = mock(MutationState.class);
+            when(mutationState.getNumRows()).thenReturn(1);
+
+            // Create a connection with mutation state and mock it
+            doReturn(mutationState).when(spy).getMutationState();
+            exceptionRule.expect(SQLException.class);
+            exceptionRule.expectMessage(
+                SQLExceptionCode.CANNOT_PERFORM_DDL_WITH_PENDING_MUTATIONS.getMessage());
+
+            spy.createStatement().execute("create table MUTATION_TEST1"
+                    + "( id1 UNSIGNED_INT not null primary key," + "appId1 VARCHAR)");
+        }
+
+    }
+
+    @Test
+    public void testNoPendingMutationsOnDDL() throws Exception {
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
 
 Review comment:
   what is the assertion here?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [phoenix] swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731#discussion_r391394772
 
 

 ##########
 File path: phoenix-core/src/test/java/org/apache/phoenix/execute/MutationStateTest.java
 ##########
 @@ -175,4 +188,36 @@ public void testGetMutationBatchList() {
         }
 
     }
+
+    @Rule
+    public ExpectedException exceptionRule = ExpectedException.none();
+
+    @Test
+    public void testPendingMutationsOnDDL() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        props.setProperty(QueryServices.PENDING_MUTATIONS_DDL_THROW_ATTRIB, "true");
+        try (Connection conn = DriverManager.getConnection(getUrl(), props);
+                PhoenixConnection spy = spy((PhoenixConnection) conn)) {
+            MutationState mutationState = mock(MutationState.class);
 
 Review comment:
   Let's use Mockito annotation instead of inline initialization. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [phoenix] siddhimehta closed pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …

Posted by GitBox <gi...@apache.org>.
siddhimehta closed pull request #731: PHOENIX-5673 : The mutation state is silently getting cleared on the …
URL: https://github.com/apache/phoenix/pull/731
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services