[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client

2020-07-08 Thread GitBox


pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r451351199



##
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##
@@ -1959,6 +1959,28 @@ public boolean listPartitionsByExpr(String catName, 
String db_name, String tbl_n
 return !r.isSetHasUnknownPartitions() || r.isHasUnknownPartitions(); // 
Assume the worst.
   }
 
+  @Override
+  public boolean listPartitionsSpecByExpr(PartitionsByExprRequest request, 
List partitionSpec)
+  throws TException {
+assert partitionSpec != null;
+PartitionsSpecByExprResult result;
+try {
+  result = client.get_partitions_spec_by_expr(request);
+} catch (TApplicationException te) {
+  if (te.getType() != TApplicationException.UNKNOWN_METHOD
+  && te.getType() != TApplicationException.WRONG_METHOD_NAME) {
+throw te;
+  }
+  throw new IncompatibleMetastoreException(
+  "Metastore doesn't support listPartitionsByExpr: " + 
te.getMessage());
+}
+
+result.setPartitionsSpec(FilterUtils.filterPartitionSpecsIfEnabled(
+isClientFilterEnabled, filterHook, result.getPartitionsSpec()));
+
+partitionSpec.addAll(result.getPartitionsSpec());
+return !result.isSetHasUnknownPartitions() || 
result.isHasUnknownPartitions();
+  }

Review comment:
   nit: missing newline?





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org



[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client

2020-07-07 Thread GitBox


pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450777027



##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3917,12 +3972,26 @@ public boolean dropPartition(String dbName, String 
tableName, List parti
   public boolean getPartitionsByExpr(Table tbl, ExprNodeGenericFuncDesc expr, 
HiveConf conf,
   List result) throws HiveException, TException {
 assert result != null;
+boolean hasUnknownParts;
 byte[] exprBytes = SerializationUtilities.serializeExpressionToKryo(expr);
 String defaultPartitionName = HiveConf.getVar(conf, 
ConfVars.DEFAULTPARTITIONNAME);
-List msParts =
-new ArrayList<>();
-boolean hasUnknownParts = 
getMSC().listPartitionsSpecByExpr(tbl.getDbName(),
-tbl.getTableName(), exprBytes, defaultPartitionName, (short)-1, 
msParts);
+List msParts = new 
ArrayList<>();
+
+if (AcidUtils.isTransactionalTable(tbl)) {
+  PartitionsByExprRequest req = new PartitionsByExprRequest();
+  req.setDbName(tbl.getDbName());
+  req.setTblName((tbl.getTableName()));
+  req.setDefaultPartitionName(defaultPartitionName);
+  req.setMaxParts((short) -1);
+  req.setExpr(exprBytes);
+  ValidWriteIdList validWriteIdList = getValidWriteIdList(tbl.getDbName(), 
tbl.getTableName());
+  req.setValidWriteIdList(validWriteIdList != null ? 
validWriteIdList.toString() : null);
+  hasUnknownParts = getMSC().listPartitionsSpecByExpr(req, msParts);
+} else {
+  hasUnknownParts = getMSC()
+  .listPartitionsSpecByExpr(tbl.getDbName(), tbl.getTableName(), 
exprBytes, defaultPartitionName, (short) -1,

Review comment:
   Maybe use the same listPartitionsSpecByExpr for both cases?





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org



[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client

2020-07-07 Thread GitBox


pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450754893



##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3580,7 +3592,19 @@ public boolean dropPartition(String dbName, String 
tableName, List parti
 List pvals = MetaStoreUtils.getPvals(t.getPartCols(), partSpec);
 
 try {
-  names = getMSC().listPartitionNames(dbName, tblName, pvals, max);
+  if (AcidUtils.isTransactionalTable(t)) {
+GetPartitionNamesPsRequest req = new GetPartitionNamesPsRequest();
+req.setTblName(tblName);
+req.setDbName(dbName);
+req.setPartValues(pvals);
+req.setMaxParts(max);
+ValidWriteIdList validWriteIdList = getValidWriteIdList(dbName, 
tblName);
+req.setValidWriteIdList(validWriteIdList != null ? 
validWriteIdList.toString() : null);
+GetPartitionNamesPsResponse res = 
getMSC().listPartitionNamesRequest(req);
+names = res.getNames();
+  } else {
+names = getMSC().listPartitionNames(dbName, tblName, pvals, max);

Review comment:
   Maybe use the same listPartitionNamesReques for acid and non-acid tables 
as well?

##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3602,9 +3626,27 @@ public boolean dropPartition(String dbName, String 
tableName, List parti
   exprBytes = SerializationUtilities.serializeExpressionToKryo(expr);
 }
 try {
+
   String defaultPartitionName = HiveConf.getVar(conf, 
ConfVars.DEFAULTPARTITIONNAME);
-  names = getMSC().listPartitionNames(tbl.getCatalogName(), 
tbl.getDbName(),
-  tbl.getTableName(), defaultPartitionName, exprBytes, order, 
maxParts);
+
+  if (AcidUtils.isTransactionalTable(tbl)) {
+PartitionsByExprRequest req =
+new PartitionsByExprRequest(tbl.getDbName(), tbl.getTableName(), 
ByteBuffer.wrap(exprBytes));
+req.setDefaultPartitionName(defaultPartitionName);
+if (maxParts >= 0) {
+  req.setMaxParts(maxParts);
+}
+req.setOrder(order);
+req.setCatName(tbl.getCatalogName());
+ValidWriteIdList validWriteIdList = 
getValidWriteIdList(tbl.getDbName(), tbl.getTableName());
+req.setValidWriteIdList(validWriteIdList != null ? 
validWriteIdList.toString() : null);
+names = getMSC().listPartitionNames(req);
+
+  } else {
+names = getMSC()
+.listPartitionNames(tbl.getCatalogName(), tbl.getDbName(), 
tbl.getTableName(), defaultPartitionName,

Review comment:
   Maybe use the same listPartitionNamesReques for acid and non-acid tables 
as well?





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org



[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client

2020-07-07 Thread GitBox


pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450752554



##
File path: 
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableOperation.java
##
@@ -62,7 +62,8 @@ public int execute() throws HiveException {
 if (desc.getReplicationSpec().isInReplicationScope()) {
   // If in replication scope, we should check if the object we're looking 
at exists, and if so,
   // trigger replace-mode semantics.
-  Table existingTable = context.getDb().getTable(tbl.getDbName(), 
tbl.getTableName(), false);
+  Table existingTable = context.getDb().getTable(tbl.getDbName(),
+  tbl.getTableName(), false, true);

Review comment:
   nit: Formatting - I think it is below 120, so we can put it into a 
single line. If not, then we should indent the second line with 4 space





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org



[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client

2020-07-07 Thread GitBox


pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450752554



##
File path: 
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableOperation.java
##
@@ -62,7 +62,8 @@ public int execute() throws HiveException {
 if (desc.getReplicationSpec().isInReplicationScope()) {
   // If in replication scope, we should check if the object we're looking 
at exists, and if so,
   // trigger replace-mode semantics.
-  Table existingTable = context.getDb().getTable(tbl.getDbName(), 
tbl.getTableName(), false);
+  Table existingTable = context.getDb().getTable(tbl.getDbName(),
+  tbl.getTableName(), false, true);

Review comment:
   nit: Formatting - I think it is below 120, so we can put it into a 
single line. If not, then we should indent the second line with 4





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org