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 2022/05/11 13:18:22 UTC
[GitHub] [hive] SourabhBadhya opened a new pull request, #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
SourabhBadhya opened a new pull request, #3281:
URL: https://github.com/apache/hive/pull/3281
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
Making CTAS use Direct Insert semantics to a suffixed table location.
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
To improve performance of CTAS query.
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
No
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
QTests
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885410160
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
Review Comment:
we should exclude managed non-txn tables as well:
````
createTableUseSuffix &= AcidUtils.isTablePropertyTransactional(pCtx.getCreateTable().getTblProps());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r870317389
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
same function is already available: MetaStoreUtils.isNonNativeTable(), can't we use it?
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879104158
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
nit, please use HiveConf getter:
````
HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
````
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887625106
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
Review Comment:
Updated.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8229,9 +8299,17 @@ private void handleLineage(LoadTableDesc ltd, Operator output)
Path tlocation = null;
String tName = Utilities.getDbTableName(tableDesc.getDbTableName())[1];
try {
+ String suffix = "";
+ if (AcidUtils.isTransactionalTable(destinationTable)) {
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885410160
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
Review Comment:
we should exclude managed non-txn tables as well, should be applied directly to the useSuffix
````
AcidUtils.isTablePropertyTransactional(pCtx.getCreateTable().getTblProps())
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885472281
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
+ // Property SOFT_DELETE_TABLE needs to be added to indicate that suffixing is used.
+ if (enableSuffixing && tblDesc.getLocation().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
Review Comment:
can't we handle suffix here:
````
if (enableSuffixing) {
long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId)
destinationPath = new Path(destinationPath + suffix);
tblDesc.getTblProps().put(SOFT_DELETE_TABLE, Boolean.TRUE.toString());
}
````
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r870324421
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
Yes correct. I will use it.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r876944852
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = AcidUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
Review Comment:
Done.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r880044433
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,94 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
Review Comment:
I have added all MetadataTransformer tests in ctas_direct_with_transformers.q which contain CTAS queries with and without location specified wherein I have set MetadataTransformer as MetastoreDefaultTransformer. The other tests are without specifying the MetadataTransformer.
https://github.com/apache/hive/pull/3281/files#diff-3488c120c636fa4652899a6c639fd7818f2ff9e3abe354e48401d3fc6b73bc74
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885472281
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
+ // Property SOFT_DELETE_TABLE needs to be added to indicate that suffixing is used.
+ if (enableSuffixing && tblDesc.getLocation().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
Review Comment:
can't we handle suffix here:
````
if (createTableUseSuffix) {
long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId)
destinationPath = new Path(destinationPath + suffix);
tblDesc.getTblProps().put(SOFT_DELETE_TABLE, Boolean.TRUE.toString());
}
tblDesc.setLocation(destinationPath.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882645082
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
Review Comment:
I was thinking about the `CREATE TABLE IF NOT EXISTS`.
I would keep the `DROP TABLE IF EXISTS`, and only use `CREATE TABLE ....` instead
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882647109
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java:
##########
@@ -347,7 +347,13 @@ public static boolean isNonNativeTable(Table table) {
if (table == null || table.getParameters() == null) {
return false;
}
- return (table.getParameters().get(hive_metastoreConstants.META_TABLE_STORAGE) != null);
+ return isNonNativeTable(table.getParameters());
+ }
+
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
+ return tblProps.get(
+ org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.META_TABLE_STORAGE)
+ != null;
Review Comment:
I have missed that.
Sorry.
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882641114
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
Review Comment:
Do we need these settings?
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882716324
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
Review Comment:
Updated.
##########
ql/src/test/queries/clientpositive/ctas_direct_with_specified_locations.q:
##########
@@ -0,0 +1,116 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885410160
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
Review Comment:
we should exclude managed non-txn tables as well
````
AcidUtils.isTransactionalTable(tbl)
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885410160
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
Review Comment:
we should exclude managed non-txn tables as well, should be applied directly to the useSuffix
````
AcidUtils.isTransactionalTable(tbl)
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885432422
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
Review Comment:
should we check the empty location 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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885428121
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
Review Comment:
could we rename it to `getCtasLocation`, to follow same naming pattern `getDefaultCtasLocation`
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885492461
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
````
createTableUseSuffix &= AcidUtils.isTablePropertyTransactional(pCtx.getCreateViewDesc().getTblProps());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885497242
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
Review Comment:
could we extract this part into the helper method, it's repeating in multiple places
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882642371
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java:
##########
@@ -347,7 +347,13 @@ public static boolean isNonNativeTable(Table table) {
if (table == null || table.getParameters() == null) {
return false;
}
- return (table.getParameters().get(hive_metastoreConstants.META_TABLE_STORAGE) != null);
+ return isNonNativeTable(table.getParameters());
+ }
+
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
+ return tblProps.get(
+ org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.META_TABLE_STORAGE)
+ != null;
Review Comment:
Why do we need this change?
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882716593
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r874954703
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
could we create in MetaStoreUtils method that accepts Map<String, String> tblProps and reuse it in the original one?
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885472281
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
+ // Property SOFT_DELETE_TABLE needs to be added to indicate that suffixing is used.
+ if (enableSuffixing && tblDesc.getLocation().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
Review Comment:
can't we handle suffix here:
````
if (enableSuffixing) {
long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId)
destinationPath = new Path(destinationPath + suffix);
tblDesc.getTblProps().put(SOFT_DELETE_TABLE, Boolean.TRUE.toString());
}
tblDesc.setLocation(destinationPath.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887628060
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
Review Comment:
Updated and added a new function called "getTableOrMVSuffix"
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r870330328
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
MetaStoreUtils.isNonNativeTable() uses Table object as argument and I needed a util method with Properties of table since Table object is created at a later stage and properties are available before table creation in CTAS.
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882640206
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
Review Comment:
You have dropped the table in this test. If there is an issue and it still exists then we have an issue 😄
Also `IF EXISTS` might hide issues if we forget to drop a table in advance.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882643432
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
Review Comment:
Isnt this to make sure that the table doesnt exist prior to the test?
I can make the change to remove IF EXISTS in drop table commands.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r881219364
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7934,6 +7958,45 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
Review Comment:
Create table flow happens at the DDLTask which is after the query is analyzed, compiled & data is written in CTAS AFAIK. Hence this path is set before the create table flow. This path is used by the FileSinkOperators to write the data.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r870324421
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
Yes correct. I will use it.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r881374106
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,94 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
+
+CREATE TABLE IF NOT EXISTS test_orc_ctas STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_orc_mmctas STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_parquet_mmctas STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_avro_mmctas STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_textfile_mmctas STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_ctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_mmctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_parquet_mmctas PARTITIONED BY (cint) STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_avro_mmctas PARTITIONED BY (cint) STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_textfile_mmctas PARTITIONED BY (cint) STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
Review Comment:
Added tests for this.
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,94 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
+
+CREATE TABLE IF NOT EXISTS test_orc_ctas STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_orc_mmctas STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_parquet_mmctas STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_avro_mmctas STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_textfile_mmctas STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_ctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_mmctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_parquet_mmctas PARTITIONED BY (cint) STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_avro_mmctas PARTITIONED BY (cint) STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_textfile_mmctas PARTITIONED BY (cint) STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+SELECT * FROM test_orc_ctas ORDER BY cint;
+
+SELECT * FROM test_orc_mmctas ORDER BY cint;
+
+SELECT * FROM test_parquet_mmctas ORDER BY cint;
+
+SELECT * FROM test_avro_mmctas ORDER BY cint;
+
+SELECT * FROM test_textfile_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_orc_ctas ORDER BY cint;
+
+SELECT * FROM test_partition_orc_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_parquet_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_avro_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_textfile_mmctas ORDER BY cint;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r881372627
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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 #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ merged PR #3281:
URL: https://github.com/apache/hive/pull/3281
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885441938
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
+ String protoName = tblDesc.getDbTableName();
+ String[] names = Utilities.getDbTableName(protoName);
+ if (enableSuffixing) {
+ long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
+ if (!db.databaseExists(names[0])) {
+ throw new SemanticException("ERROR: The database " + names[0] + " does not exist.");
+ }
+
+ Warehouse wh = new Warehouse(conf);
+ location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, false);
+ } else {
+ location = new Path(tblDesc.getLocation());
+ }
+
+ // Handle table translation
+ // Property modifications of the table is handled later.
+ // We are interested in the location if it has changed
+ // due to table translation.
+ Table tbl = tblDesc.toTable(conf);
+ tbl = db.getTranslateTableDryrun(tbl.getTTable());
Review Comment:
shouldn't we pass through the transformers first and do the location check after?
````
tbl = db.getTranslateTableDryrun(tbl.getTTable());
if (tbl.getSd().getLocation() == null
|| tbl.getSd().getLocation().isEmpty()) {
location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1], false);
} else {
location = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
}
//add the suffix here
location = new Path(location + getTableSuffix(tbl));
tbl.getSd().setLocation(location.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885441938
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
+ String protoName = tblDesc.getDbTableName();
+ String[] names = Utilities.getDbTableName(protoName);
+ if (enableSuffixing) {
+ long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
+ if (!db.databaseExists(names[0])) {
+ throw new SemanticException("ERROR: The database " + names[0] + " does not exist.");
+ }
+
+ Warehouse wh = new Warehouse(conf);
+ location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, false);
+ } else {
+ location = new Path(tblDesc.getLocation());
+ }
+
+ // Handle table translation
+ // Property modifications of the table is handled later.
+ // We are interested in the location if it has changed
+ // due to table translation.
+ Table tbl = tblDesc.toTable(conf);
+ tbl = db.getTranslateTableDryrun(tbl.getTTable());
Review Comment:
shouldn't we pass through the transformers first and do the location check after?
````
tbl = db.getTranslateTableDryrun(tbl.getTTable());
if (tbl.getSd().getLocation() == null
|| tbl.getSd().getLocation().isEmpty()) {
location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, false);
} else {
location = wh.getDnsPath(new Path(tbl.getSd().getLocation() + suffix));
}
tbl.getSd().setLocation(location.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885665688
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8229,9 +8299,17 @@ private void handleLineage(LoadTableDesc ltd, Operator output)
Path tlocation = null;
String tName = Utilities.getDbTableName(tableDesc.getDbTableName())[1];
try {
+ String suffix = "";
+ if (AcidUtils.isTransactionalTable(destinationTable)) {
+ boolean useSuffix = Boolean.getBoolean(destinationTable.getProperty(SOFT_DELETE_TABLE));
Review Comment:
can we get a NullPointer when SOFT_DELETE_TABLE is not present?
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887629211
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
Should we do transactional check for Materialised views?
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r880044731
##########
ql/src/test/queries/clientpositive/ctas_direct_with_specified_locations.q:
##########
@@ -0,0 +1,92 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
Review Comment:
I have added all MetadataTransformer tests in ctas_direct_with_transformers.q which contain CTAS queries with and without location specified wherein I have set MetadataTransformer as MetastoreDefaultTransformer. The other tests are without specifying the MetadataTransformer.
https://github.com/apache/hive/pull/3281/files#diff-3488c120c636fa4652899a6c639fd7818f2ff9e3abe354e48401d3fc6b73bc74
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885476172
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
please rename to `createTableUseSuffix`
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885492461
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
createTableUseSuffix &=
AcidUtils.isTablePropertyTransactional(pCtx.getCreateViewDesc().getTblProps());
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879120122
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8223,9 +8286,27 @@ private void handleLineage(LoadTableDesc ltd, Operator output)
Path tlocation = null;
String tName = Utilities.getDbTableName(tableDesc.getDbTableName())[1];
try {
+ String suffix = "";
+ if (!tableDesc.isExternal()) {
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
you shouldn't be relying on configs here, but the SOFT_DELETE_TABLE prop value
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879104158
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
nit, please use HiveConf setter:
````
HiveConf.setBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
````
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r876944639
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = AcidUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
+ isMmTable = isMmCreate = true;
+ }
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || conf.getBoolVar(ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ String location = tblDesc.getLocation();
+ destinationPath = location == null ? getCTASDestinationTableLocation(tblDesc, enableSuffixing) : new Path(location);
Review Comment:
@pvary Thanks for pointing this out. I have updated the patch to handle the use of MetadataTransformer. Please check and let me know if there are any issues with it.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887627534
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887625663
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
Updated.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
+ // Property SOFT_DELETE_TABLE needs to be added to indicate that suffixing is used.
+ if (enableSuffixing && tblDesc.getLocation().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r881373707
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8223,9 +8286,27 @@ private void handleLineage(LoadTableDesc ltd, Operator output)
Path tlocation = null;
String tName = Utilities.getDbTableName(tableDesc.getDbTableName())[1];
try {
+ String suffix = "";
+ if (!tableDesc.isExternal()) {
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882646180
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,118 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
Review Comment:
Oh ok. I will make the changes for this.
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882640778
##########
ql/src/test/queries/clientpositive/ctas_direct_with_specified_locations.q:
##########
@@ -0,0 +1,116 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
Review Comment:
Do we need these settings?
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r874957184
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = AcidUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
Review Comment:
can't we simply use?
````
isMmTable = AcidUtils.isInsertOnlyTable(destinationTable.getParameters());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879107738
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || conf.getBoolVar(ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
Review Comment:
please check that during create SOFT_DELETE_TABLE prop is set, there is an if that skips this setter in case of manually set location
````
if (createTableUseSuffix) {
tbl.setProperty(SOFT_DELETE_TABLE, Boolean.TRUE.toString());
}
````
--
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: gitbox-unsubscribe@hive.apache.org
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] saihemanth-cloudera commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879902533
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,94 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
+
+CREATE TABLE IF NOT EXISTS test_orc_ctas STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_orc_mmctas STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_parquet_mmctas STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_avro_mmctas STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_textfile_mmctas STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_ctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_mmctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_parquet_mmctas PARTITIONED BY (cint) STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_avro_mmctas PARTITIONED BY (cint) STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_textfile_mmctas PARTITIONED BY (cint) STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
Review Comment:
Can you a couple of other tables like Create Managed table foo(i int) and Create Transcational table foo(i int);
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r876943222
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
Done
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885479005
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8229,9 +8299,17 @@ private void handleLineage(LoadTableDesc ltd, Operator output)
Path tlocation = null;
String tName = Utilities.getDbTableName(tableDesc.getDbTableName())[1];
try {
+ String suffix = "";
+ if (AcidUtils.isTransactionalTable(destinationTable)) {
Review Comment:
use AcidUtils.isTableSoftDeleteEnabled() it checks SOFT_DELETE_TABLE property 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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885441938
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
+ String protoName = tblDesc.getDbTableName();
+ String[] names = Utilities.getDbTableName(protoName);
+ if (enableSuffixing) {
+ long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
+ if (!db.databaseExists(names[0])) {
+ throw new SemanticException("ERROR: The database " + names[0] + " does not exist.");
+ }
+
+ Warehouse wh = new Warehouse(conf);
+ location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, false);
+ } else {
+ location = new Path(tblDesc.getLocation());
+ }
+
+ // Handle table translation
+ // Property modifications of the table is handled later.
+ // We are interested in the location if it has changed
+ // due to table translation.
+ Table tbl = tblDesc.toTable(conf);
+ tbl = db.getTranslateTableDryrun(tbl.getTTable());
Review Comment:
shouldn't we pass through the transformers first and do the location check after?
````
tbl = db.getTranslateTableDryrun(tbl.getTTable());
if (tbl.getSd().getLocation() == null
|| tbl.getSd().getLocation().isEmpty()) {
location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1], false);
} else {
location = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
}
location = new Path(location + getTableSuffix(tbl));
tbl.getSd().setLocation(location.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887629211
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
Should we do transactional check for Materialised views?
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887627260
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
Review Comment:
Location check happens now at the point where suffixing is decided. Updated.
https://github.com/apache/hive/pull/3281/files#diff-d4b1a32bbbd9e283893a6b52854c7aeb3e356a1ba1add2c4107e52901ca268f9R7616-R7618
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r873524614
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = AcidUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
+ isMmTable = isMmCreate = true;
+ }
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || conf.getBoolVar(ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ String location = tblDesc.getLocation();
+ destinationPath = location == null ? getCTASDestinationTableLocation(tblDesc, enableSuffixing) : new Path(location);
Review Comment:
How would this handle the `MetadataTransformer`s? These Transformers could alter the table location (external/managed directory changes, etc)
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887625817
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
+ String protoName = tblDesc.getDbTableName();
+ String[] names = Utilities.getDbTableName(protoName);
+ if (enableSuffixing) {
+ long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
+ if (!db.databaseExists(names[0])) {
+ throw new SemanticException("ERROR: The database " + names[0] + " does not exist.");
+ }
+
+ Warehouse wh = new Warehouse(conf);
+ location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, false);
+ } else {
+ location = new Path(tblDesc.getLocation());
+ }
+
+ // Handle table translation
+ // Property modifications of the table is handled later.
+ // We are interested in the location if it has changed
+ // due to table translation.
+ Table tbl = tblDesc.toTable(conf);
+ tbl = db.getTranslateTableDryrun(tbl.getTTable());
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887624982
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
Renamed to createTableOrMVUseSuffix. Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885472281
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
+ // Property SOFT_DELETE_TABLE needs to be added to indicate that suffixing is used.
+ if (enableSuffixing && tblDesc.getLocation().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
Review Comment:
can't we handle suffix here:
````
if (createTableUseSuffix) {
long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId)
destinationPath = new Path(destinationPath + suffix);
tblDesc.getTblProps().put(SOFT_DELETE_TABLE, Boolean.TRUE.toString());
}
tblDesc.setLocation(destinationPath.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885441938
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7940,6 +7970,46 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
+ Path location;
+ String suffix = "";
+ try {
+ // When location is specified, suffix is not added
+ if (tblDesc.getLocation() == null) {
+ String protoName = tblDesc.getDbTableName();
+ String[] names = Utilities.getDbTableName(protoName);
+ if (enableSuffixing) {
+ long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
+ if (!db.databaseExists(names[0])) {
+ throw new SemanticException("ERROR: The database " + names[0] + " does not exist.");
+ }
+
+ Warehouse wh = new Warehouse(conf);
+ location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, false);
+ } else {
+ location = new Path(tblDesc.getLocation());
+ }
+
+ // Handle table translation
+ // Property modifications of the table is handled later.
+ // We are interested in the location if it has changed
+ // due to table translation.
+ Table tbl = tblDesc.toTable(conf);
+ tbl = db.getTranslateTableDryrun(tbl.getTTable());
Review Comment:
shouldn't we pass through the transformers first and do the location check after?
````
tbl = db.getTranslateTableDryrun(tbl.getTTable());
if (tbl.getSd().getLocation() == null
|| tbl.getSd().getLocation().isEmpty()) {
location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1], false);
} else {
location = wh.getDnsPath(new Path(tbl.getSd().getLocation()));
}
tbl.getSd().setLocation(location.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885492461
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
````
createTableUseSuffix &=
AcidUtils.isTablePropertyTransactional(pCtx.getCreateViewDesc().getTblProps());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879122398
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7934,6 +7958,45 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
return output;
}
+ private Path getCTASDestinationTableLocation(CreateTableDesc tblDesc, boolean enableSuffixing) throws SemanticException {
Review Comment:
this part looks similar to what is done in create table. Did we set CTAS path here before or we went through the create table flow?
--
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: gitbox-unsubscribe@hive.apache.org
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] saihemanth-cloudera commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879904223
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,94 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
+
+CREATE TABLE IF NOT EXISTS source STORED AS ORC TBLPROPERTIES('transactional'='true') AS (SELECT cint, cfloat, cdouble, cstring1, ctimestamp1 FROM alltypesorc);
+
+CREATE TABLE IF NOT EXISTS test_orc_ctas STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_orc_mmctas STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_parquet_mmctas STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_avro_mmctas STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_textfile_mmctas STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_ctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_orc_mmctas PARTITIONED BY (cint) STORED AS ORC TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_parquet_mmctas PARTITIONED BY (cint) STORED AS PARQUET TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_avro_mmctas PARTITIONED BY (cint) STORED AS AVRO TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM source WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM source WHERE cint < -100 LIMIT 10));
+
+CREATE TABLE IF NOT EXISTS test_partition_textfile_mmctas PARTITIONED BY (cint) STORED AS TEXTFILE TBLPROPERTIES('transactional'='true', 'transactional_properties'='insert_only') AS ((SELECT * FROM alltypesorc WHERE cint > 200 LIMIT 10) UNION (SELECT * FROM alltypesorc WHERE cint < -100 LIMIT 10));
+
+SELECT * FROM test_orc_ctas ORDER BY cint;
+
+SELECT * FROM test_orc_mmctas ORDER BY cint;
+
+SELECT * FROM test_parquet_mmctas ORDER BY cint;
+
+SELECT * FROM test_avro_mmctas ORDER BY cint;
+
+SELECT * FROM test_textfile_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_orc_ctas ORDER BY cint;
+
+SELECT * FROM test_partition_orc_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_parquet_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_avro_mmctas ORDER BY cint;
+
+SELECT * FROM test_partition_textfile_mmctas ORDER BY cint;
+
+DROP TABLE IF EXISTS source;
+
+DROP TABLE IF EXISTS test_orc_ctas;
+
+DROP TABLE IF EXISTS test_orc_mmctas;
+
+DROP TABLE IF EXISTS test_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_avro_mmctas;
+
+DROP TABLE IF EXISTS test_textfile_mmctas;
+
+DROP TABLE IF EXISTS test_partition_orc_ctas;
+
+DROP TABLE IF EXISTS test_partition_orc_mmctas;
+
+DROP TABLE IF EXISTS test_partition_parquet_mmctas;
+
+DROP TABLE IF EXISTS test_partition_avro_mmctas;
+
+DROP TABLE IF EXISTS test_partition_textfile_mmctas;
Review Comment:
Nit: add new line at the end of the file.
--
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: gitbox-unsubscribe@hive.apache.org
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] saihemanth-cloudera commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879902026
##########
ql/src/test/queries/clientpositive/ctas_direct.q:
##########
@@ -0,0 +1,94 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
Review Comment:
Can you enable MetastoreDefault Transformer on this test? MetastoreDefaultTransformer is required to get the table location.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882532032
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || conf.getBoolVar(ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
Review Comment:
I have added the check here which adds the SOFT_DELETE_TABLE property when the location is suffixed.
https://github.com/apache/hive/pull/3281/files#diff-d4b1a32bbbd9e283893a6b52854c7aeb3e356a1ba1add2c4107e52901ca268f9R7614-R7615
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885472281
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7598,6 +7602,26 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = MetaStoreUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ isMmTable = isMmCreate = AcidUtils.isInsertOnlyTable(tblProps);
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ destinationPath = getCTASDestinationTableLocation(tblDesc, enableSuffixing);
+ // Setting the location so that metadata transformers
+ // does not change the location later while creating the table.
+ tblDesc.setLocation(destinationPath.toString());
+ // Property SOFT_DELETE_TABLE needs to be added to indicate that suffixing is used.
+ if (enableSuffixing && tblDesc.getLocation().matches("(.*)" + SOFT_DELETE_TABLE_PATTERN)) {
Review Comment:
can't we handle suffix here:
````
if (createTableUseSuffix) {
long txnId = ctx.getHiveTxnManager().getCurrentTxnId();
suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId)
tblDesc.getTblProps().put(SOFT_DELETE_TABLE, Boolean.TRUE.toString());
destinationPath = new Path(destinationPath + suffix);
}
tblDesc.setLocation(destinationPath.toString());
````
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885487387
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
Review Comment:
shouldn't be populating suffix if txnId=0
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885488362
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
Review Comment:
please rename to `createTableUseSuffix`
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r885496096
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
long txnId = Optional.ofNullable(pCtx.getContext())
.map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
Review Comment:
shouldn't be populating suffix if txnId=0
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r887697843
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java:
##########
@@ -517,17 +517,21 @@ private Path getDefaultCtasLocation(final ParseContext pCtx) throws SemanticExce
try {
String protoName = null, suffix = "";
boolean isExternal = false;
-
+ boolean useSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+
if (pCtx.getQueryProperties().isCTAS()) {
protoName = pCtx.getCreateTable().getDbTableName();
isExternal = pCtx.getCreateTable().isExternal();
-
+ if (!isExternal && useSuffix) {
+ long txnId = Optional.ofNullable(pCtx.getContext())
+ .map(ctx -> ctx.getHiveTxnManager().getCurrentTxnId()).orElse(0L);
+ suffix = SOFT_DELETE_PATH_SUFFIX + String.format(DELTA_DIGITS, txnId);
+ }
} else if (pCtx.getQueryProperties().isMaterializedView()) {
protoName = pCtx.getCreateViewDesc().getViewName();
- boolean createMVUseSuffix = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
- || HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
- if (createMVUseSuffix) {
+ if (useSuffix) {
Review Comment:
Since suffixing must be done for transactional MV, added a condition. Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] saihemanth-cloudera commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879904618
##########
ql/src/test/queries/clientpositive/ctas_direct_with_specified_locations.q:
##########
@@ -0,0 +1,92 @@
+-- SORT_QUERY_RESULTS
+--! qt:dataset:alltypesorc
+
+set hive.support.concurrency=true;
+set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
+set hive.acid.direct.insert.enabled=true;
+set hive.exec.max.dynamic.partitions=200;
+set hive.exec.max.dynamic.partitions.pernode=200;
+
Review Comment:
Same as above, can you set MetastoreStoreDefaultTransformer class here? and other qfiles also.
--
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: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r879110909
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7601,13 +7619,14 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
} catch (LockException ex) {
throw new SemanticException("Failed to allocate write Id", ex);
}
- if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
- isMmTable = isMmCreate = true;
+ if (isMmTable) {
if (tblDesc != null) {
- tblDesc.setInitialMmWriteId(writeId);
+ tblDesc.setInitialWriteId(writeId);
} else {
viewDesc.setInitialMmWriteId(writeId);
}
+ } else if (isDirectInsert) {
+ tblDesc.setInitialWriteId(writeId);
Review Comment:
can we enter here when creating a view?
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r876944639
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = AcidUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
+ isMmTable = isMmCreate = true;
+ }
+ if (!isNonNativeTable && !destTableIsTemporary && isCtas) {
+ destTableIsFullAcid = AcidUtils.isFullAcidTable(tblProps);
+ acidOperation = getAcidType(dest);
+ isDirectInsert = isDirectInsert(destTableIsFullAcid, acidOperation);
+ boolean enableSuffixing = conf.getBoolVar(ConfVars.HIVE_ACID_CREATE_TABLE_USE_SUFFIX)
+ || conf.getBoolVar(ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
+ if (isDirectInsert || isMmTable) {
+ String location = tblDesc.getLocation();
+ destinationPath = location == null ? getCTASDestinationTableLocation(tblDesc, enableSuffixing) : new Path(location);
Review Comment:
@pvary Thanks for pointing this out. I have edited the patch to handle the use of MetadataTransformer. Please check and let me know if there are any issues with it.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r876944852
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -7592,6 +7594,22 @@ protected Operator genFileSinkPlan(String dest, QB qb, Operator input)
destTableIsTransactional = tblProps != null && AcidUtils.isTablePropertyTransactional(tblProps);
if (destTableIsTransactional) {
+ isNonNativeTable = AcidUtils.isNonNativeTable(tblProps);
+ boolean isCtas = tblDesc != null && tblDesc.isCTAS();
+ if (AcidUtils.isInsertOnlyTable(tblProps, true)) {
Review Comment:
Updated.
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
Updated.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r876943222
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -2876,6 +2876,12 @@ private static boolean isLockableTable(Table t) {
}
}
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
Review Comment:
Done.
--
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: gitbox-unsubscribe@hive.apache.org
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] SourabhBadhya commented on a diff in pull request #3281: HIVE-26217: Make CTAS use Direct Insert Semantics
Posted by GitBox <gi...@apache.org>.
SourabhBadhya commented on code in PR #3281:
URL: https://github.com/apache/hive/pull/3281#discussion_r882645468
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java:
##########
@@ -347,7 +347,13 @@ public static boolean isNonNativeTable(Table table) {
if (table == null || table.getParameters() == null) {
return false;
}
- return (table.getParameters().get(hive_metastoreConstants.META_TABLE_STORAGE) != null);
+ return isNonNativeTable(table.getParameters());
+ }
+
+ public static boolean isNonNativeTable(Map<String, String> tblProps) {
+ return tblProps.get(
+ org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.META_TABLE_STORAGE)
+ != null;
Review Comment:
The function is used here -
https://github.com/apache/hive/pull/3281/files#diff-d4b1a32bbbd9e283893a6b52854c7aeb3e356a1ba1add2c4107e52901ca268f9R7599
--
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: gitbox-unsubscribe@hive.apache.org
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