[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
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
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
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
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
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