[
https://issues.apache.org/jira/browse/HIVE-26580?focusedWorklogId=820927&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-820927
]
ASF GitHub Bot logged work on HIVE-26580:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 27/Oct/22 11:02
Start Date: 27/Oct/22 11:02
Worklog Time Spent: 10m
Work Description: deniskuzZ commented on code in PR #3708:
URL: https://github.com/apache/hive/pull/3708#discussion_r1006701744
##########
ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java:
##########
@@ -2388,6 +2396,65 @@ public void testShowCompactionInputValidation() throws
Exception {
"STATUS 'ready for clean'");//validates compaction status
}
+ @Test
+ public void testShowCompactionFilterSortingAndLimit()throws Exception {
+ runStatementOnDriver("drop database if exists mydb1 cascade");
+ runStatementOnDriver("create database mydb1");
+ runStatementOnDriver("create table mydb1.tbl0 " + "(a int, b int)
partitioned by (p string) clustered by (a) into " +
+ BUCKET_COUNT + " buckets stored as orc TBLPROPERTIES
('transactional'='true')");
+ runStatementOnDriver("insert into mydb1.tbl0" + " PARTITION(p) " +
+ "
values(1,2,'p1'),(3,4,'p1'),(1,2,'p2'),(3,4,'p2'),(1,2,'p3'),(3,4,'p3')");
+ runStatementOnDriver("alter table mydb1.tbl0" + " PARTITION(p='p1')
compact 'MAJOR'");
+ TestTxnCommands2.runWorker(hiveConf);
+ runStatementOnDriver("alter table mydb1.tbl0" + " PARTITION(p='p2')
compact 'MAJOR'");
+ TestTxnCommands2.runWorker(hiveConf);
+
+
+ runStatementOnDriver("drop database if exists mydb cascade");
+ runStatementOnDriver("create database mydb");
+ runStatementOnDriver("create table mydb.tbl " + "(a int, b int)
partitioned by (ds string) clustered by (a) into " +
+ BUCKET_COUNT + " buckets stored as orc TBLPROPERTIES
('transactional'='true')");
+ runStatementOnDriver("insert into mydb.tbl" + " PARTITION(ds) " +
+ "
values(1,2,'mon'),(3,4,'tue'),(1,2,'mon'),(3,4,'tue'),(1,2,'wed'),(3,4,'wed')");
+ runStatementOnDriver("alter table mydb.tbl" + " PARTITION(ds='mon')
compact 'MAJOR'");
+ TestTxnCommands2.runWorker(hiveConf);
+ runStatementOnDriver("alter table mydb.tbl" + " PARTITION(ds='tue')
compact 'MAJOR'");
+ TestTxnCommands2.runWorker(hiveConf);
+
+ runStatementOnDriver("create table mydb.tbl2 " + "(a int, b int)
partitioned by (dm string) clustered by (a) into " +
+ BUCKET_COUNT + " buckets stored as orc TBLPROPERTIES
('transactional'='true')");
+ runStatementOnDriver("insert into mydb.tbl2" + " PARTITION(dm) " +
+ "
values(1,2,'xxx'),(3,4,'xxx'),(1,2,'yyy'),(3,4,'yyy'),(1,2,'zzz'),(3,4,'zzz')");
+ runStatementOnDriver("alter table mydb.tbl2" + " PARTITION(dm='yyy')
compact 'MAJOR'");
+ TestTxnCommands2.runWorker(hiveConf);
+ runStatementOnDriver("alter table mydb.tbl2" + " PARTITION(dm='zzz')
compact 'MAJOR'");
+ TestTxnCommands2.runWorker(hiveConf);
+
+ ShowCompactResponse rsp = txnHandler.showCompact(new ShowCompactRequest());
+
+ //includes Header row
+ List<String> r = runStatementOnDriver("SHOW COMPACTIONS");
+ Assert.assertEquals(rsp.getCompacts().size() + 1, r.size());
+ r = runStatementOnDriver("SHOW COMPACTIONS LIMIT 3");
+ Assert.assertEquals(4, r.size());
+ r = runStatementOnDriver("SHOW COMPACTIONS SCHEMA mydb TYPE 'MAJOR' LIMIT
2");
+ Assert.assertEquals(3, r.size());
+
+ r = runStatementOnDriver("SHOW COMPACTIONS SCHEMA mydb TYPE 'MAJOR' ORDER
BY CC_TABLE DESC, CC_PARTITION ASC");
Review Comment:
there is no `CC_` prefix in the show compactions output. That would be
confusing
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsAnalyzer.java:
##########
@@ -50,6 +50,8 @@ public void analyzeInternal(ASTNode root) throws
SemanticException {
String compactionType = null;
String compactionStatus = null;
long compactionId = 0;
+ String order = null;
+ short limit = 0;
Review Comment:
maybe -1, meaning no limit?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsAnalyzer.java:
##########
@@ -80,15 +82,38 @@ public void analyzeInternal(ASTNode root) throws
SemanticException {
case HiveParser.TOK_COMPACT_ID:
compactionId = Long.parseLong(child.getChild(0).getText());
break;
+ case HiveParser.TOK_LIMIT:
+ limit = Short.valueOf((child.getChild(0)).getText());
+ break;
+ case HiveParser.TOK_ORDERBY:
+ order = processSortOrderSpec(child);
+ break;
default:
dbName = stripQuotes(child.getText());
}
}
ShowCompactionsDesc desc = new ShowCompactionsDesc(ctx.getResFile(),
compactionId, dbName, tblName, poolName, compactionType,
- compactionStatus, partitionSpec);
+ compactionStatus, partitionSpec, limit, order);
Task<DDLWork> task = TaskFactory.get(new DDLWork(getInputs(),
getOutputs(), desc));
rootTasks.add(task);
task.setFetchSource(true);
setFetchTask(createFetchTask(ShowCompactionsDesc.SCHEMA));
}
+ private String processSortOrderSpec(ASTNode sortNode)
+ {
+ StringBuilder orderByExp = new StringBuilder();
+ ArrayList<PTFInvocationSpec.OrderExpression> orderExp =
processOrderSpec(sortNode).getExpressions();
+ PTFInvocationSpec.OrderExpression exp ;
+ for (int i = 0; i < orderExp.size() - 1; i++) {
Review Comment:
to avoid duplication, you could first collect all of the expressions into a
list and then use StringUtils.join(" ,", list)
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsDesc.java:
##########
@@ -93,6 +97,16 @@ public String getCompactionStatus() {
public Map<String, String> getPartSpec() {
return partSpec;
}
+ @Explain(displayName = "limit", explainLevels = {Level.USER,
Level.DEFAULT, Level.EXTENDED})
+ public short getLimit() {
+ return limit;
+ }
+
+ @Explain(displayName = "order", explainLevels = {Level.USER,
Level.DEFAULT, Level.EXTENDED})
+ public String getOrder() {
Review Comment:
getOrderBy()
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1984,4 +1984,28 @@ public ParseContext getParseContext() {
return pCtx;
}
+ public PTFInvocationSpec.OrderSpec processOrderSpec(ASTNode sortNode) {
+ PTFInvocationSpec.OrderSpec oSpec = new PTFInvocationSpec.OrderSpec();
+ int exprCnt = sortNode.getChildCount();
+ for(int i=0; i < exprCnt; i++) {
+ PTFInvocationSpec.OrderExpression exprSpec = new
PTFInvocationSpec.OrderExpression();
+ ASTNode orderSpec = (ASTNode) sortNode.getChild(i);
+ ASTNode nullOrderSpec = (ASTNode) orderSpec.getChild(0);
+ exprSpec.setExpression((ASTNode) nullOrderSpec.getChild(0));
+ if ( orderSpec.getType() == HiveParser.TOK_TABSORTCOLNAMEASC ) {
+
exprSpec.setOrder(org.apache.hadoop.hive.ql.parse.PTFInvocationSpec.Order.ASC);
+ }
+ else {
+
exprSpec.setOrder(org.apache.hadoop.hive.ql.parse.PTFInvocationSpec.Order.DESC);
+ }
+ if ( nullOrderSpec.getType() == HiveParser.TOK_NULLS_FIRST ) {
Review Comment:
nit: remove extra space
##########
ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java:
##########
@@ -2155,6 +2155,11 @@ public void testShowCompactions() throws Exception {
ShowCompactResponse rsp = txnHandler.showCompact(new ShowCompactRequest());
List<String> r = runStatementOnDriver("SHOW COMPACTIONS");
Assert.assertEquals(rsp.getCompacts().size()+1, r.size());//includes
Header row
+ r = runStatementOnDriver("SHOW COMPACTIONS LIMIT 3");
Review Comment:
why do we need this if we have `testShowCompactionFilterSortingAndLimit`?
##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -1351,7 +1351,9 @@ struct ShowCompactRequest {
4: required string tablename,
5: optional string partitionname,
6: required CompactionType type,
- 7: required string state
+ 7: required string state,
+ 8: optional i64 limit,
+ 9: optional string order
Review Comment:
wouldn't it be better to have map here, Map<String (column), String (order)>
ordeerBy?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java:
##########
@@ -4013,6 +4016,12 @@ private String
getShowCompactFilterClause(ShowCompactRequest request) {
" WHERE " + StringUtils.join(" AND ", params) : EMPTY;
}
+ private String getShowCompactSortingOrderClause(ShowCompactRequest request){
+ String sortingOrder = request.getOrder();
+ return isNotBlank(sortingOrder) ?
TxnQueries.SHOW_COMPACTION_ORDERBY_CLAUSE + " , " + sortingOrder :
Review Comment:
if custom order is provided we should use that and don't append the default
one
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsAnalyzer.java:
##########
@@ -80,15 +82,38 @@ public void analyzeInternal(ASTNode root) throws
SemanticException {
case HiveParser.TOK_COMPACT_ID:
compactionId = Long.parseLong(child.getChild(0).getText());
break;
+ case HiveParser.TOK_LIMIT:
+ limit = Short.valueOf((child.getChild(0)).getText());
+ break;
+ case HiveParser.TOK_ORDERBY:
+ order = processSortOrderSpec(child);
+ break;
default:
dbName = stripQuotes(child.getText());
}
}
ShowCompactionsDesc desc = new ShowCompactionsDesc(ctx.getResFile(),
compactionId, dbName, tblName, poolName, compactionType,
- compactionStatus, partitionSpec);
+ compactionStatus, partitionSpec, limit, order);
Task<DDLWork> task = TaskFactory.get(new DDLWork(getInputs(),
getOutputs(), desc));
rootTasks.add(task);
task.setFetchSource(true);
setFetchTask(createFetchTask(ShowCompactionsDesc.SCHEMA));
}
+ private String processSortOrderSpec(ASTNode sortNode)
+ {
Review Comment:
formatting
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsDesc.java:
##########
@@ -45,10 +45,12 @@ public class ShowCompactionsDesc implements DDLDesc,
Serializable {
private final String compactionType;
private final String compactionStatus;
private final Map<String, String> partSpec;
+ private final short limit;
+ private final String order;
public ShowCompactionsDesc(Path resFile, long compactionId, String dbName,
String tbName, String poolName, String compactionType,
- String compactionStatus, Map<String, String>
partSpec) {
+ String compactionStatus, Map<String, String>
partSpec, short limit, String order) {
Review Comment:
orderBy
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1984,4 +1984,28 @@ public ParseContext getParseContext() {
return pCtx;
}
+ public PTFInvocationSpec.OrderSpec processOrderSpec(ASTNode sortNode) {
+ PTFInvocationSpec.OrderSpec oSpec = new PTFInvocationSpec.OrderSpec();
+ int exprCnt = sortNode.getChildCount();
+ for(int i=0; i < exprCnt; i++) {
+ PTFInvocationSpec.OrderExpression exprSpec = new
PTFInvocationSpec.OrderExpression();
+ ASTNode orderSpec = (ASTNode) sortNode.getChild(i);
+ ASTNode nullOrderSpec = (ASTNode) orderSpec.getChild(0);
+ exprSpec.setExpression((ASTNode) nullOrderSpec.getChild(0));
+ if ( orderSpec.getType() == HiveParser.TOK_TABSORTCOLNAMEASC ) {
Review Comment:
nit: remove extra space
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1984,4 +1984,28 @@ public ParseContext getParseContext() {
return pCtx;
}
+ public PTFInvocationSpec.OrderSpec processOrderSpec(ASTNode sortNode) {
+ PTFInvocationSpec.OrderSpec oSpec = new PTFInvocationSpec.OrderSpec();
+ int exprCnt = sortNode.getChildCount();
+ for(int i=0; i < exprCnt; i++) {
Review Comment:
nit: space
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1984,4 +1984,28 @@ public ParseContext getParseContext() {
return pCtx;
}
+ public PTFInvocationSpec.OrderSpec processOrderSpec(ASTNode sortNode) {
+ PTFInvocationSpec.OrderSpec oSpec = new PTFInvocationSpec.OrderSpec();
Review Comment:
use static import
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1984,4 +1984,28 @@ public ParseContext getParseContext() {
return pCtx;
}
+ public PTFInvocationSpec.OrderSpec processOrderSpec(ASTNode sortNode) {
+ PTFInvocationSpec.OrderSpec oSpec = new PTFInvocationSpec.OrderSpec();
+ int exprCnt = sortNode.getChildCount();
+ for(int i=0; i < exprCnt; i++) {
+ PTFInvocationSpec.OrderExpression exprSpec = new
PTFInvocationSpec.OrderExpression();
+ ASTNode orderSpec = (ASTNode) sortNode.getChild(i);
+ ASTNode nullOrderSpec = (ASTNode) orderSpec.getChild(0);
+ exprSpec.setExpression((ASTNode) nullOrderSpec.getChild(0));
+ if ( orderSpec.getType() == HiveParser.TOK_TABSORTCOLNAMEASC ) {
+
exprSpec.setOrder(org.apache.hadoop.hive.ql.parse.PTFInvocationSpec.Order.ASC);
Review Comment:
can we use static import of
`org.apache.hadoop.hive.ql.parse.PTFInvocationSpec`
##########
ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java:
##########
@@ -2155,6 +2155,11 @@ public void testShowCompactions() throws Exception {
ShowCompactResponse rsp = txnHandler.showCompact(new ShowCompactRequest());
List<String> r = runStatementOnDriver("SHOW COMPACTIONS");
Assert.assertEquals(rsp.getCompacts().size()+1, r.size());//includes
Header row
+ r = runStatementOnDriver("SHOW COMPACTIONS LIMIT 3");
+ Assert.assertEquals(4, r.size());
+
+ r = runStatementOnDriver("SHOW COMPACTIONS SCHEMA mydb1 TYPE 'MAJOR' ORDER
BY CC_PARTITION ASC , CC_TABLE DESC");
Review Comment:
why do we need this if we have `testShowCompactionFilterSortingAndLimit`?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/process/show/compactions/ShowCompactionsAnalyzer.java:
##########
@@ -80,15 +82,38 @@ public void analyzeInternal(ASTNode root) throws
SemanticException {
case HiveParser.TOK_COMPACT_ID:
compactionId = Long.parseLong(child.getChild(0).getText());
break;
+ case HiveParser.TOK_LIMIT:
+ limit = Short.valueOf((child.getChild(0)).getText());
+ break;
+ case HiveParser.TOK_ORDERBY:
+ order = processSortOrderSpec(child);
+ break;
default:
dbName = stripQuotes(child.getText());
}
}
ShowCompactionsDesc desc = new ShowCompactionsDesc(ctx.getResFile(),
compactionId, dbName, tblName, poolName, compactionType,
- compactionStatus, partitionSpec);
+ compactionStatus, partitionSpec, limit, order);
Task<DDLWork> task = TaskFactory.get(new DDLWork(getInputs(),
getOutputs(), desc));
rootTasks.add(task);
task.setFetchSource(true);
setFetchTask(createFetchTask(ShowCompactionsDesc.SCHEMA));
}
+ private String processSortOrderSpec(ASTNode sortNode)
+ {
+ StringBuilder orderByExp = new StringBuilder();
+ ArrayList<PTFInvocationSpec.OrderExpression> orderExp =
processOrderSpec(sortNode).getExpressions();
Review Comment:
use List interface
##########
ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java:
##########
@@ -2187,6 +2192,9 @@ public void testShowCompactions() throws Exception {
Assert.assertTrue(p.matcher(r.get(i)).matches());
}
+ r = runStatementOnDriver("SHOW COMPACTIONS SCHEMA mydb1 TYPE 'MAJOR' LIMIT
2");
Review Comment:
why do we need this if we have `testShowCompactionFilterSortingAndLimit`?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java:
##########
@@ -3881,15 +3881,18 @@ public boolean submitForCleanup(CompactionRequest rqst,
long highestWriteId, lon
public ShowCompactResponse showCompact(ShowCompactRequest rqst) throws
MetaException {
try {
ShowCompactResponse response = new ShowCompactResponse(new
ArrayList<>());
- String query = TxnQueries.SHOW_COMPACTION_QUERY +
getShowCompactFilterClause(rqst) +
- TxnQueries.SHOW_COMPACTION_ORDERBY_CLAUSE;
+ String query = TxnQueries.SHOW_COMPACTION_QUERY +
+ getShowCompactFilterClause(rqst) +
+ getShowCompactSortingOrderClause(rqst) ;
List<String> params = getShowCompactParamList(rqst);
-
Review Comment:
keep the new line here
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java:
##########
@@ -3881,15 +3881,18 @@ public boolean submitForCleanup(CompactionRequest rqst,
long highestWriteId, lon
public ShowCompactResponse showCompact(ShowCompactRequest rqst) throws
MetaException {
try {
ShowCompactResponse response = new ShowCompactResponse(new
ArrayList<>());
- String query = TxnQueries.SHOW_COMPACTION_QUERY +
getShowCompactFilterClause(rqst) +
- TxnQueries.SHOW_COMPACTION_ORDERBY_CLAUSE;
+ String query = TxnQueries.SHOW_COMPACTION_QUERY +
+ getShowCompactFilterClause(rqst) +
+ getShowCompactSortingOrderClause(rqst) ;
List<String> params = getShowCompactParamList(rqst);
-
try (Connection dbConn =
getDbConn(Connection.TRANSACTION_READ_COMMITTED);
PreparedStatement stmt =
sqlGenerator.prepareStmtWithParameters(dbConn, query, params)) {
if (rqst.isSetId()) {
stmt.setLong(1, rqst.getId());
}
+ int rowLimit = (int) rqst.getLimit();
+ if (rowLimit > 0)
Review Comment:
wrap with braces
Issue Time Tracking
-------------------
Worklog Id: (was: 820927)
Time Spent: 0.5h (was: 20m)
> SHOW COMPACTIONS should support ordering and limiting functionality in
> filtering options
> ----------------------------------------------------------------------------------------
>
> Key: HIVE-26580
> URL: https://issues.apache.org/jira/browse/HIVE-26580
> Project: Hive
> Issue Type: Improvement
> Affects Versions: 3.0.0
> Reporter: KIRTI RUGE
> Assignee: KIRTI RUGE
> Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> SHOW COMPACTION should provide ordering by defied table . It should also
> support limitation of fetched records
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
