You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/07/23 17:32:25 UTC
[GitHub] [hive] kishendas opened a new pull request #2524: Advance write id for all the DDLs
kishendas opened a new pull request #2524:
URL: https://github.com/apache/hive/pull/2524
### What changes were proposed in this pull request?
Advance write ID during 12 DDLs
### Why are the changes needed?
HMS cache relies on ValidWriteIdList to serve the data consistently.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit tests
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] hsnusonic commented on a change in pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
hsnusonic commented on a change in pull request #2524:
URL: https://github.com/apache/hive/pull/2524#discussion_r681150853
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/owner/AlterTableSetOwnerAnalyzer.java
##########
@@ -26,7 +26,11 @@
import org.apache.hadoop.hive.ql.ddl.DDLSemanticAnalyzerFactory.DDLType;
import org.apache.hadoop.hive.ql.ddl.privilege.PrincipalDesc;
import org.apache.hadoop.hive.ql.ddl.table.AbstractAlterTableAnalyzer;
+import org.apache.hadoop.hive.ql.ddl.table.partition.add.AlterTableAddPartitionDesc;
+import org.apache.hadoop.hive.ql.exec.Task;
import org.apache.hadoop.hive.ql.exec.TaskFactory;
+import org.apache.hadoop.hive.ql.io.AcidUtils;
+import org.apache.hadoop.hive.ql.metadata.Table;
Review comment:
Do we need all these import?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kishendas commented on pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
kishendas commented on pull request #2524:
URL: https://github.com/apache/hive/pull/2524#issuecomment-891649173
@pvary @hsnusonic Please take a look now. Addressed all the comments and got a green run.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary merged pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
pvary merged pull request #2524:
URL: https://github.com/apache/hive/pull/2524
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2524:
URL: https://github.com/apache/hive/pull/2524#discussion_r681121522
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java
##########
@@ -432,6 +433,50 @@ public void testAddAndDropConstraintAdvancingWriteIds() throws Exception {
}
+ @Test
+ public void testDDLsAdvancingWriteIds() throws Exception {
+
+ String tableName = "alter_table";
+ runStatementOnDriver("drop table if exists " + tableName);
+ runStatementOnDriver(String.format("create table %s (a int, b string, c BIGINT, d INT) " +
+ "PARTITIONED BY (ds STRING)" +
+ "TBLPROPERTIES ('transactional'='true', 'transactional_properties'='insert_only')",
+ tableName));
+ runStatementOnDriver(String.format("insert into %s (a) values (0)", tableName));
+ IMetaStoreClient msClient = new HiveMetaStoreClient(hiveConf);
+ String validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:1:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s SET OWNER USER user_name", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:2:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s CLUSTERED BY(c) SORTED BY(d) INTO 32 BUCKETS", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:3:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s ADD PARTITION (ds='2013-04-05')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:4:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s SET SERDEPROPERTIES ('field.delim'='\\u0001')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:5:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') SET FILEFORMAT PARQUET", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:6:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') COMPACT 'minor'", tableName));
Review comment:
Why do we want to increase writeId for compaction 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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kishendas commented on a change in pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
kishendas commented on a change in pull request #2524:
URL: https://github.com/apache/hive/pull/2524#discussion_r681126333
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java
##########
@@ -432,6 +433,50 @@ public void testAddAndDropConstraintAdvancingWriteIds() throws Exception {
}
+ @Test
+ public void testDDLsAdvancingWriteIds() throws Exception {
+
+ String tableName = "alter_table";
+ runStatementOnDriver("drop table if exists " + tableName);
+ runStatementOnDriver(String.format("create table %s (a int, b string, c BIGINT, d INT) " +
+ "PARTITIONED BY (ds STRING)" +
+ "TBLPROPERTIES ('transactional'='true', 'transactional_properties'='insert_only')",
+ tableName));
+ runStatementOnDriver(String.format("insert into %s (a) values (0)", tableName));
+ IMetaStoreClient msClient = new HiveMetaStoreClient(hiveConf);
+ String validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:1:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s SET OWNER USER user_name", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:2:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s CLUSTERED BY(c) SORTED BY(d) INTO 32 BUCKETS", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:3:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s ADD PARTITION (ds='2013-04-05')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:4:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s SET SERDEPROPERTIES ('field.delim'='\\u0001')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:5:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') SET FILEFORMAT PARQUET", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:6:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') COMPACT 'minor'", tableName));
Review comment:
Done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kishendas commented on pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
kishendas commented on pull request #2524:
URL: https://github.com/apache/hive/pull/2524#issuecomment-891165578
Got a green build -> http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-2524/3/tests/
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2524:
URL: https://github.com/apache/hive/pull/2524#discussion_r681123834
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java
##########
@@ -432,6 +433,50 @@ public void testAddAndDropConstraintAdvancingWriteIds() throws Exception {
}
+ @Test
+ public void testDDLsAdvancingWriteIds() throws Exception {
+
+ String tableName = "alter_table";
+ runStatementOnDriver("drop table if exists " + tableName);
+ runStatementOnDriver(String.format("create table %s (a int, b string, c BIGINT, d INT) " +
+ "PARTITIONED BY (ds STRING)" +
+ "TBLPROPERTIES ('transactional'='true', 'transactional_properties'='insert_only')",
+ tableName));
+ runStatementOnDriver(String.format("insert into %s (a) values (0)", tableName));
+ IMetaStoreClient msClient = new HiveMetaStoreClient(hiveConf);
+ String validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:1:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s SET OWNER USER user_name", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:2:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s CLUSTERED BY(c) SORTED BY(d) INTO 32 BUCKETS", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:3:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s ADD PARTITION (ds='2013-04-05')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:4:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s SET SERDEPROPERTIES ('field.delim'='\\u0001')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:5:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') SET FILEFORMAT PARQUET", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:6:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') COMPACT 'minor'", tableName));
Review comment:
I see now 😄
Maybe a comment?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kishendas commented on a change in pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
kishendas commented on a change in pull request #2524:
URL: https://github.com/apache/hive/pull/2524#discussion_r681122678
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java
##########
@@ -432,6 +433,50 @@ public void testAddAndDropConstraintAdvancingWriteIds() throws Exception {
}
+ @Test
+ public void testDDLsAdvancingWriteIds() throws Exception {
+
+ String tableName = "alter_table";
+ runStatementOnDriver("drop table if exists " + tableName);
+ runStatementOnDriver(String.format("create table %s (a int, b string, c BIGINT, d INT) " +
+ "PARTITIONED BY (ds STRING)" +
+ "TBLPROPERTIES ('transactional'='true', 'transactional_properties'='insert_only')",
+ tableName));
+ runStatementOnDriver(String.format("insert into %s (a) values (0)", tableName));
+ IMetaStoreClient msClient = new HiveMetaStoreClient(hiveConf);
+ String validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:1:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s SET OWNER USER user_name", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:2:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("alter table %s CLUSTERED BY(c) SORTED BY(d) INTO 32 BUCKETS", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:3:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s ADD PARTITION (ds='2013-04-05')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:4:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s SET SERDEPROPERTIES ('field.delim'='\\u0001')", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:5:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') SET FILEFORMAT PARQUET", tableName));
+ validWriteIds = msClient.getValidWriteIds("default." + tableName).toString();
+ Assert.assertEquals("default.alter_table:6:9223372036854775807::", validWriteIds);
+
+ runStatementOnDriver(String.format("ALTER TABLE %s PARTITION (ds='2013-04-05') COMPACT 'minor'", tableName));
Review comment:
That test case actually ensures we don't increase the write ID during compaction !
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kishendas commented on a change in pull request #2524: HIVE-25372: Advance write id for all the DDLs
Posted by GitBox <gi...@apache.org>.
kishendas commented on a change in pull request #2524:
URL: https://github.com/apache/hive/pull/2524#discussion_r681550664
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/owner/AlterTableSetOwnerAnalyzer.java
##########
@@ -26,7 +26,11 @@
import org.apache.hadoop.hive.ql.ddl.DDLSemanticAnalyzerFactory.DDLType;
import org.apache.hadoop.hive.ql.ddl.privilege.PrincipalDesc;
import org.apache.hadoop.hive.ql.ddl.table.AbstractAlterTableAnalyzer;
+import org.apache.hadoop.hive.ql.ddl.table.partition.add.AlterTableAddPartitionDesc;
+import org.apache.hadoop.hive.ql.exec.Task;
import org.apache.hadoop.hive.ql.exec.TaskFactory;
+import org.apache.hadoop.hive.ql.io.AcidUtils;
+import org.apache.hadoop.hive.ql.metadata.Table;
Review comment:
Fixed it.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org