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/04/12 21:23:39 UTC
[GitHub] [phoenix] gjacoby126 commented on a diff in pull request #1409: CDPD-34599 Add support for TRUNCATE TABLE
gjacoby126 commented on code in PR #1409:
URL: https://github.com/apache/phoenix/pull/1409#discussion_r848871706
##########
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();
+ Connection conn = DriverManager.getConnection(getUrl(), props);
+ String tableName = "nonExistentTable";
+ try {
+ conn.createStatement().execute("TRUNCATE TABLE " + tableName);
+ fail();
+ } catch (TableNotFoundException e) {
+ return;
+ }
+ }
+
+ @Test
+ public void testTruncateTableNonExistentSchema() throws SQLException {
+ Properties props = new Properties();
+ Connection conn = DriverManager.getConnection(getUrl(), props);
+ String schemaName = "nonExistentSchema";
+ String tableName = generateUniqueName();
+
+ try {
+ conn.createStatement()
+ .execute("CREATE TABLE " + tableName + " (C1 INTEGER PRIMARY KEY)");
+ conn.setAutoCommit(true);
+ PreparedStatement stmt = conn.prepareStatement(
+ "UPSERT INTO " + tableName + " VALUES(1)");
+ stmt.execute();
+ ResultSet rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tableName);
+ assertTrue(rs.next());
+ assertEquals(1, rs.getInt(1));
+ conn.createStatement().execute("TRUNCATE TABLE " + schemaName + "." + tableName);
+ fail();
+ } catch (SQLException e) {
+ conn.createStatement().execute("DROP TABLE " + tableName);
Review Comment:
Good to assert on the actual SQLException you're expecting.
##########
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();
+ Connection conn = DriverManager.getConnection(getUrl(), props);
Review Comment:
Please create the connection using try-with-resources to make sure that it gets closed at the end of the test.
##########
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();
+ try {
+ PTable ptable = connection.getTable(
+ new PTableKey(connection.getTenantId(), SchemaUtil.getTableName(schemaName, tableName)));
+ } catch (TableNotFoundException e) {
+ throw e;
+ }
+ try {
+ connection.getQueryServices().truncateTable(schemaName, tableName, isNamespaceMapped);
Review Comment:
There should be a check in here to make sure that we don't try to truncate a SYSTEM table, and if we do, throw a SQLException with a new SQLExceptionCode explaining we don't allow that.
##########
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();
+ try {
+ PTable ptable = connection.getTable(
+ new PTableKey(connection.getTenantId(), SchemaUtil.getTableName(schemaName, tableName)));
+ } catch (TableNotFoundException e) {
+ throw e;
+ }
+ try {
+ connection.getQueryServices().truncateTable(schemaName, tableName, isNamespaceMapped);
Review Comment:
Likewise, I don't think you should be able to truncate an index directly (but an index should get truncated if its base table gets truncated, as I think @chrajeshbabu pointed out.)
##########
phoenix-core/src/main/antlr3/PhoenixSQL.g:
##########
@@ -460,6 +462,12 @@ create_table_node returns [CreateTableStatement ret]
(SPLIT ON s=value_expression_list)?
{ret = factory.createTable(t, p, c, pk, s, PTableType.TABLE, ex!=null, null, null, getBindCount(), im!=null ? true : null); }
;
+
+// Parse a truncate table statement.
+truncate_table_node returns [TruncateTableStatement ret]
+ : TRUNCATE TABLE t=from_table_name
+ {ret = factory.truncateTable(t, PTableType.TABLE);}
Review Comment:
What happens if someone tries to truncate a view? Looks like we're assuming that from_table_name is a table, but there's nothing in the grammar to enforce that, is there?
##########
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();
+ Connection conn = DriverManager.getConnection(getUrl(), props);
+ String tableName = "nonExistentTable";
+ try {
+ conn.createStatement().execute("TRUNCATE TABLE " + tableName);
+ fail();
+ } catch (TableNotFoundException e) {
+ return;
+ }
+ }
+
+ @Test
+ public void testTruncateTableNonExistentSchema() throws SQLException {
+ Properties props = new Properties();
+ Connection conn = DriverManager.getConnection(getUrl(), props);
+ String schemaName = "nonExistentSchema";
+ String tableName = generateUniqueName();
+
+ try {
+ conn.createStatement()
+ .execute("CREATE TABLE " + tableName + " (C1 INTEGER PRIMARY KEY)");
+ conn.setAutoCommit(true);
+ PreparedStatement stmt = conn.prepareStatement(
+ "UPSERT INTO " + tableName + " VALUES(1)");
+ stmt.execute();
+ ResultSet rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tableName);
+ assertTrue(rs.next());
+ assertEquals(1, rs.getInt(1));
+ conn.createStatement().execute("TRUNCATE TABLE " + schemaName + "." + tableName);
+ fail();
+ } catch (SQLException e) {
+ conn.createStatement().execute("DROP TABLE " + tableName);
+ }
+ }
+
+ @Test
+ public void testTruncateTableWithImplicitSchema() throws SQLException {
+ Properties props = new Properties();
+ Connection conn = DriverManager.getConnection(getUrl(), props);
+ String schemaName = generateUniqueName();
+ String tableName = generateUniqueName();
+ String createTableWithSchema = "CREATE TABLE " + schemaName + "." + tableName + " (C1 char(2) NOT NULL PRIMARY KEY)";
+
+ try {
+ conn.createStatement().execute(createTableWithSchema);
+ conn.setAutoCommit(true);
+ PreparedStatement stmt = conn.prepareStatement("UPSERT INTO " + schemaName + "." + tableName + " values('a')");
+ stmt.execute();
+ ResultSet rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + schemaName + "." + tableName);
+ assertTrue(rs.next());
+ assertEquals(1, rs.getInt(1));
+ stmt = conn.prepareStatement("UPSERT INTO " + schemaName + "." + tableName + " values('b')");
+ stmt.execute();
+ rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + schemaName + "." + tableName);
+ assertTrue(rs.next());
+ assertEquals(2, rs.getInt(1));
+ conn.close();
+
+ conn = DriverManager.getConnection(getUrl(), props);
+ conn.createStatement().execute("TRUNCATE TABLE " + schemaName + "." + tableName);
+ conn.close();
+
+ conn = DriverManager.getConnection(getUrl(), props);
+ rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + schemaName + "." + tableName);
+ assertTrue(rs.next());
+ assertEquals(0, rs.getInt(1));
+ conn.close();
+
+ conn = DriverManager.getConnection(getUrl(), props);
+ conn.createStatement().execute("DROP TABLE " + schemaName + "." + tableName);
+ conn.close();
+ } catch (SQLException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testTruncateTableWithExplicitSchema() throws SQLException {
+ Properties props = new Properties();
+ props.setProperty(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(true));
Review Comment:
A given Phoenix cluster can only have Namespace mapping enabled or disabled; it can't be enabled for some tables and disabled for others. This usually means either having to shut down the minicluster and spin it back up with namespace enabled, or putting this is in a separate IT test.
##########
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);
Review Comment:
Best to create the connection using try-with-resources
##########
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);
Review Comment:
Consider dropping the table in a finally block
--
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