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 2022/03/24 09:27:56 UTC

[GitHub] [phoenix] ragarkar opened a new pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

ragarkar opened a new pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409


   1. Added support for TRUNCATE TABLE <table> statement in phoenix.
   2. Added an integration test to test the feature.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#issuecomment-1078805209


   Or rather make preserving splits the default, and add an option to NOT preserver them.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#issuecomment-1078766834


   There are a few issues to be addressed:
   
   - child tables (global secondary index tables and and global secondary indexes on views) also need to be handled.
   - This doesn't work for multitenant tables from tenant-specific connections (i think simply erroring out in that case is fine)
   - Some sanity checks for critical system tables would also be needed.
   - Need to think about what happens when trying to access the tables while truncating.
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] chrajeshbabu commented on a change in pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
chrajeshbabu commented on a change in pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#discussion_r835102100



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -2223,6 +2223,24 @@ private void dropTable(byte[] tableNameToDelete) throws SQLException {
         dropTables(Collections.<byte[]>singletonList(tableNameToDelete));
     }
 
+    @Override
+    public void truncateTable(String schemaName, String tableName, boolean isNamespaceMapped) throws SQLException {
+        SQLException sqlE = null;
+        TableName hbaseTableName = SchemaUtil.getPhysicalTableName(SchemaUtil.getTableName(schemaName, tableName).getBytes(), isNamespaceMapped);
+        try {
+            Admin admin = getAdmin();
+            admin.disableTable(hbaseTableName);

Review comment:
       Truncate table automatically disables the table we need not do it explicitly.

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -2223,6 +2223,24 @@ private void dropTable(byte[] tableNameToDelete) throws SQLException {
         dropTables(Collections.<byte[]>singletonList(tableNameToDelete));
     }
 
+    @Override
+    public void truncateTable(String schemaName, String tableName, boolean isNamespaceMapped) throws SQLException {
+        SQLException sqlE = null;
+        TableName hbaseTableName = SchemaUtil.getPhysicalTableName(SchemaUtil.getTableName(schemaName, tableName).getBytes(), isNamespaceMapped);
+        try {
+            Admin admin = getAdmin();
+            admin.disableTable(hbaseTableName);
+            admin.truncateTable(hbaseTableName, true);

Review comment:
       After the truncate table the regions cache in hbase is not longer valid so would be better to call
   ConnectionQueryServicesImpl#clearTableRegionCache after that so that further queries fetch new locations of table regions.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/TruncateTableIT.java
##########
@@ -0,0 +1,186 @@
+package org.apache.phoenix.end2end;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.schema.TableNotFoundException;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(ParallelStatsDisabledTest.class)
+public class TruncateTableIT extends ParallelStatsDisabledIT {

Review comment:
       You can add the code to check the number of regions after truncate also.

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
##########
@@ -3303,6 +3304,30 @@ private static void throwIfInsufficientColumns(String schemaName, String tableNa
         }
     }
 
+    public MutationState truncateTable(TruncateTableStatement statement) throws SQLException {
+        String schemaName = connection.getSchema() != null && statement.getTableName().getSchemaName() == null
+            ? connection.getSchema() : statement.getTableName().getSchemaName();
+        String tableName = statement.getTableName().getTableName();
+        boolean isNamespaceMapped = SchemaUtil.isNamespaceMappingEnabled(statement.getTableType(), connection.getQueryServices().getProps());
+        boolean wasAutoCommit = connection.getAutoCommit();

Review comment:
       Since we are not doing any meta update changes here changing getting autocommit status and setting back is not much impact so better to remove it.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/TruncateTableIT.java
##########
@@ -0,0 +1,186 @@
+package org.apache.phoenix.end2end;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.schema.TableNotFoundException;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(ParallelStatsDisabledTest.class)
+public class TruncateTableIT extends ParallelStatsDisabledIT {
+
+    @Test
+    public void testTruncateTable() throws SQLException {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        try {
+            String createTableDDL =
+                "CREATE TABLE " + tableName + " (pk char(2) not null primary key)";
+            conn.createStatement().execute(createTableDDL);
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);
+            conn.setAutoCommit(true);

Review comment:
       This is by default not required.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/TruncateTableIT.java
##########
@@ -0,0 +1,186 @@
+package org.apache.phoenix.end2end;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.schema.TableNotFoundException;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(ParallelStatsDisabledTest.class)
+public class TruncateTableIT extends ParallelStatsDisabledIT {
+
+    @Test
+    public void testTruncateTable() throws SQLException {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        try {
+            String createTableDDL =
+                "CREATE TABLE " + tableName + " (pk char(2) not null primary key)";
+            conn.createStatement().execute(createTableDDL);
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);
+            conn.setAutoCommit(true);
+            PreparedStatement stmt = conn.prepareStatement(
+                "UPSERT INTO " + tableName + " VALUES('a')");
+            stmt.execute();
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);
+            ResultSet rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tableName);
+            assertTrue(rs.next());
+            assertEquals(1, rs.getInt(1));
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);
+            conn.createStatement().execute("TRUNCATE TABLE " + tableName);
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);
+            rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tableName);
+            assertTrue(rs.next());
+            assertEquals(0, rs.getInt(1));
+            conn.createStatement().execute("DROP TABLE " + tableName);
+            conn.close();
+        } catch (SQLException e) {
+            fail();
+        }
+    }
+
+    @Test
+    public void testTruncateTableNotExist() throws Exception {
+        Properties props = new Properties();

Review comment:
       Need not pass empty properties, can call directly DriverManager.getConnection(getUrl())

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -2223,6 +2223,24 @@ private void dropTable(byte[] tableNameToDelete) throws SQLException {
         dropTables(Collections.<byte[]>singletonList(tableNameToDelete));
     }
 
+    @Override
+    public void truncateTable(String schemaName, String tableName, boolean isNamespaceMapped) throws SQLException {
+        SQLException sqlE = null;
+        TableName hbaseTableName = SchemaUtil.getPhysicalTableName(SchemaUtil.getTableName(schemaName, tableName).getBytes(), isNamespaceMapped);
+        try {
+            Admin admin = getAdmin();
+            admin.disableTable(hbaseTableName);
+            admin.truncateTable(hbaseTableName, true);

Review comment:
       Have you checked the truncate table always waiting for the table to get created and enabled? If not we might need to add retries in case if required.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/TruncateTableIT.java
##########
@@ -0,0 +1,186 @@
+package org.apache.phoenix.end2end;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.schema.TableNotFoundException;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(ParallelStatsDisabledTest.class)
+public class TruncateTableIT extends ParallelStatsDisabledIT {

Review comment:
       You can add a test case creating tables with split keys which is missing.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/TruncateTableIT.java
##########
@@ -0,0 +1,186 @@
+package org.apache.phoenix.end2end;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.schema.TableNotFoundException;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(ParallelStatsDisabledTest.class)
+public class TruncateTableIT extends ParallelStatsDisabledIT {
+
+    @Test
+    public void testTruncateTable() throws SQLException {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        try {
+            String createTableDDL =
+                "CREATE TABLE " + tableName + " (pk char(2) not null primary key)";
+            conn.createStatement().execute(createTableDDL);
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);
+            conn.setAutoCommit(true);
+            PreparedStatement stmt = conn.prepareStatement(
+                "UPSERT INTO " + tableName + " VALUES('a')");
+            stmt.execute();
+            conn.close();
+
+            conn = DriverManager.getConnection(getUrl(), props);

Review comment:
       Need not recreate the connection every time. Can make use the same connection and close it at the end of test case.




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#issuecomment-1078804384


   That sounds good, @ragarkar .


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#issuecomment-1078760708


   Please do not use private JIRA IDs in the Apache project.
   Use the PHOENIX-300 ticket ID.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] ragarkar commented on pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
ragarkar commented on pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#issuecomment-1078803241


   > 
   
   Thanks for looking into these changes. How about we provide an extension to the TRUNCATE TABLE statement to preserve the splits? We can have an optional clause something like:
   
   TRUNCATE TABLE <tablename> PRESERVE SPLITS;


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty edited a comment on pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE

Posted by GitBox <gi...@apache.org>.
stoty edited a comment on pull request #1409:
URL: https://github.com/apache/phoenix/pull/1409#issuecomment-1078805209


   Or rather make preserving splits the default, and add an option to NOT preserve them.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org