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 2020/11/26 02:03:36 UTC
[GitHub] [hive] nareshpr opened a new pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
nareshpr opened a new pull request #1712:
URL: https://github.com/apache/hive/pull/1712
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r534940570
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
Impala uses HMS API for transaction management. They rely on a TxnHandler.lock() method to handle concurrent txns.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r533195842
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
Could there be an issue if HMS API is used outside of Hive, for example via Impala?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r532463117
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
@nareshpr , do you know if partition name part (`name`=value) is already normalized 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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r533195842
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
Could there be an issue with partition key name normalization if HMS API is used outside of Hive, for example via Impala?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r535910850
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2877,6 +2877,20 @@ private static String normalizeCase(String s) {
return s == null ? null : s.toLowerCase();
}
+ private static String normalizePartitionCase(String s) {
Review comment:
@nareshpr, LGTM, however could you please try to reuse FileUtils.makePartName(List<String> partCols, List<String> vals)
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nareshpr commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
nareshpr commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r535899436
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
I changed it to split & convert partition key to lower key.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nareshpr commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
nareshpr commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r533640402
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
I doubt HMS API's will hit this flow. Let me know any example or testcase. If required, i can change code to split key=value & convert key to lowerCase and join. Let me know if that is required.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r532463117
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
@nareshpr , do you know if partition name part (<name>=value) is already normalized 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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ merged pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ merged pull request #1712:
URL: https://github.com/apache/hive/pull/1712
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] klcopp commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
klcopp commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r535906390
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2877,6 +2877,20 @@ private static String normalizeCase(String s) {
return s == null ? null : s.toLowerCase();
}
+ private static String normalizePartitionCase(String s) {
+ if (s == null) {
+ return null;
+ } else {
Review comment:
No need for the else clause
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r532463117
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
@nareshpr , do you know if partition key name (`name`=value) is already normalized 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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r535910850
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2877,6 +2877,20 @@ private static String normalizeCase(String s) {
return s == null ? null : s.toLowerCase();
}
+ private static String normalizePartitionCase(String s) {
Review comment:
@nareshpr, LGTM, however could you please try to reuse FileUtils.makePartName(List<String> partCols, List<String> vals):
Map<String, String> map = Splitter.on( "=" ).withKeyValueSeparator( Path.SEPARATOR ).split(lc.getPartitionname());
return FileUtils.makePartName(new ArrayList<>(map.keySet()), new ArrayList<>(map.values()));
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
pvargacl commented on pull request #1712:
URL: https://github.com/apache/hive/pull/1712#issuecomment-734158908
Hi @nareshpr thanks for picking this up. Could you add a test case to TestInitiator that covers this use case.
cc @klcopp @deniskuzZ
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nareshpr commented on pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
nareshpr commented on pull request #1712:
URL: https://github.com/apache/hive/pull/1712#issuecomment-734699849
Thanks for looking into this @pvargacl. I added testcase in TestInitiator, please review
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nareshpr commented on a change in pull request #1712: HIVE-24433 - AutoCompaction is not getting triggered for CamelCase Partition Values
Posted by GitBox <gi...@apache.org>.
nareshpr commented on a change in pull request #1712:
URL: https://github.com/apache/hive/pull/1712#discussion_r532821552
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -2725,7 +2725,7 @@ private void insertTxnComponents(long txnid, LockRequest rqst, Connection dbConn
}
String dbName = normalizeCase(lc.getDbname());
String tblName = normalizeCase(lc.getTablename());
- String partName = normalizeCase(lc.getPartitionname());
+ String partName = lc.getPartitionname();
Review comment:
Yes, i validated below 4 SQL's with my patch, partition(key=value) is already normalized
insert into table abc PARTITION(CitY='Bangalore') values('Dan');
insert overwrite table abc partition(CiTy='Bangalore') select Name from abc;
update table abc set Name='xy' where CiTy='Bangalore';
delete from abc where CiTy='Bangalore';
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org