HIVE-28238: Open Hive ACID txn only for transactional resources #5216
Conversation
deniskuzZ
requested review from
kasakrisz
and removed request for
miklosgergely and
klcopp
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
|
Is there an apache jira about this change? |
| if (dbProduct.isDERBY()) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Does it mean that a txn has to be always opened if the backend db is derby? Even if there are no transactional tables in the statement.
There was a problem hiding this comment.
no one is using derby in production, but the main reason is that derby doesn't support multi-column IN construct.
https://stackoverflow.com/questions/48175692/multiple-columns-in-in-clause-apache-derby
tested so far on Postgres, MySQL, Oracle
There was a problem hiding this comment.
maybe we should consider moving to H2 in tests
deniskuzZ
changed the title
|
asf-ci-hive
added
tests pending
tests failed
and removed
tests passed
tests pending
labels
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
asf-ci-hive
added
tests failed
tests pending
and removed
tests pending
tests failed
labels
asf-ci-hive
added
tests failed
tests pending
and removed
tests pending
tests failed
labels
|
| @Override | ||
| public boolean hasAcidResourcesInQuery() { | ||
| // check DB tags once supported (i.e. ICEBERG_ONLY, ACID_ONLY, EXTERNAL_ONLY) | ||
| return true; |
| import org.apache.hadoop.mapred.InputFormat; | ||
| import org.apache.hadoop.mapred.InputSplit; | ||
| import org.apache.hadoop.mapred.JobConf; | ||
| import org.apache.hadoop.mapred.RecordReader; | ||
| import org.apache.hadoop.mapred.Reporter; | ||
| import org.apache.hadoop.mapred.lib.CombineFileSplit; | ||
|
|
||
| import static jodd.util.ClassUtil.isAssignableFrom; |
| @@ -330,7 +329,7 @@ private void markExplicitTransaction(QueryPlan queryPlan) throws LockException { | |||
| * @throws LockException | |||
| */ | |||
| private void verifyState(QueryPlan queryPlan) throws LockException { | |||
| if(!isTxnOpen()) { | |||
| if(!isTxnOpen() && queryPlan.hasAcidResourcesInQuery()) { | |||
There was a problem hiding this comment.
to support external tables without need of open txn
What changes were proposed in this pull request?
Open ACID txn only when transactional resources are present in a query
Why are the changes needed?
Backend db calls optimization
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
mvn test -Dtest=ITestDbTxnManager -Dtest.metastore.db=postgres