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
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
