Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ merged PR #4761: URL: https://github.com/apache/hive/pull/4761 -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1982974644 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1981653980 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1978205696 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1511019923
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr, (Expression)
SessionStateUtil.getResource(conf,
Review Comment:
could you please check the filter construction for
merge_iceberg_partitioned_orc.q
merge into target_ice as t using source src ON t.a = src.a
when matched and t.a > 100 THEN DELETE
when matched then update set b = 'Merged', c = t.c + 10
when not matched then insert values (src.a, src.b, src.c);
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1976235469 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1975754906 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1975476545 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1510432984
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestUtilCyclicBarrier.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.CyclicBarrier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestUtilCyclicBarrier {
+ private static final Logger LOG =
LoggerFactory.getLogger(TestUtilCyclicBarrier.class);
+ private static TestUtilCyclicBarrier instance;
+ private final CyclicBarrier cyclicBarrier;
+
+ private TestUtilCyclicBarrier(int parties) {
+cyclicBarrier = new CyclicBarrier(parties);
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1972895055 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508758046
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestUtilCyclicBarrier.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.CyclicBarrier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestUtilCyclicBarrier {
+ private static final Logger LOG =
LoggerFactory.getLogger(TestUtilCyclicBarrier.class);
+ private static TestUtilCyclicBarrier instance;
+ private final CyclicBarrier cyclicBarrier;
+
+ private TestUtilCyclicBarrier(int parties) {
+cyclicBarrier = new CyclicBarrier(parties);
Review Comment:
my bad, I think it would be better to use Phaser here, since we want dynamic
number of parties
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Phaser.html
final Phaser phaser = new Phaser(1); // "1" to register self
// create and start threads
for (final Runnable task : tasks) {
phaser.register();
new Thread() {
public void run() {
phaser.arriveAndAwaitAdvance(); // await all creation
task.run();
}
}.start();
}
// allow threads to start and deregister self
phaser.arriveAndDeregister();
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508743321
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestUtilCyclicBarrier.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.CyclicBarrier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestUtilCyclicBarrier {
Review Comment:
what i meant here is if could we embed this class inside of
HiveIcebergStorageHandlerStub.
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1971838444 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4761=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508029266
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestUtilCyclicBarrier.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.CyclicBarrier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestUtilCyclicBarrier {
Review Comment:
- We need to instantiate the cyclic barrier before we concurrently start 2
threads to run the update queries via the shell.executeStatement() in unit test
. For the cyclic barrier to work, we need to pass the same instance to both the
threads. Therefore, i had to implement a singleton and then use it in the
Stub.
```
TestUtilCyclicBarrier.getInstance(2);
try {
Tasks.range(2).executeWith(Executors.newFixedThreadPool(2)).run(i -> {
init(shell, testTables, temp, executionEngine);
HiveConf.setBoolVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, isVectorized);
HiveConf.setVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_FETCH_TASK_CONVERSION, "none");
HiveConf.setVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_QUERY_REEXECUTION_STRATEGIES,
RETRY_STRATEGIES_WITHOUT_WRITE_CONFLICT);
shell.executeStatement(singleFilterQuery[i]);
shell.closeSession();
});
```
- Also, i thought we can reuse the cyclic barrier code in other concurrent
tests in the future and having it as a separate class would be useful
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508014085
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr, (Expression)
SessionStateUtil.getResource(conf,
Review Comment:
The following example creates 2 job contexts per update query:
Thread 1: UPDATE customers SET first_name='Changed' WHERE
last_name='Taylor'
Thread 2: UPDATE customers SET first_name='Changed' WHERE
last_name='Donnel'
In the previous commit it would have multiple duplicate filters with AND.
But i this commit, i have added a if condition to update the the filter only if
they are different. This should help minimise the duplicate filters as seen in
image below.
https://github.com/apache/hive/assets/26441677/11f02125-5bb4-44e0-a6c0-9d9813279c80;>
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508014085
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr, (Expression)
SessionStateUtil.getResource(conf,
Review Comment:
The following example creates 2 job contexts per update query:
Thread 1: UPDATE customers SET first_name='Changed' WHERE
last_name='Taylor'
Thread 2: UPDATE customers SET first_name='Changed' WHERE
last_name='Donnel'
In the previous commit it would have multiple duplicate filters with AND.
But i this commit, i have added a if condition to update the the filter only if
they are different. This should help minimise the duplicate filters as seen in
image below.
https://github.com/apache/hive/assets/26441677/fccb5ecb-8fe1-48d1-9e14-ecbc222961af;>
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508000921
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -384,9 +384,12 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+Expression expr = (Expression) HiveIcebergInputFormat.getFilterExpr(conf,
predicate.pushedPredicate);
Review Comment:
fixed
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.BrokenBarrierException;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info(" Using HiveIcebergStorageHandlerStub for unit tests");
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1508000635
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergInputFormat.java:
##
@@ -102,12 +102,25 @@ static Expression
icebergDataFilterFromHiveConf(Configuration conf) {
if (hiveFilter != null) {
ExprNodeGenericFuncDesc exprNodeDesc = SerializationUtilities
.deserializeObject(hiveFilter, ExprNodeGenericFuncDesc.class);
- SearchArgument sarg = ConvertAstToSearchArg.create(conf, exprNodeDesc);
- try {
+ return getFilterExpr(conf, exprNodeDesc);
+}
+return null;
+ }
+
+ /**
+ * getFilterExpr extracts search argument from ExprNodeGenericFuncDesc and
returns Iceberg Filter Expression
+ * @param conf - job conf
+ * @param exprNodeDesc - Describes a GenericFunc node
+ * @return Iceberg Filter Expression
+ */
+ public static Expression getFilterExpr(Configuration conf,
ExprNodeGenericFuncDesc exprNodeDesc) {
+try {
Review Comment:
done
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
Review Comment:
done
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -769,6 +771,9 @@ public void storageHandlerCommit(Properties
commitProperties, Operation operatio
}
}
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+return new HiveIcebergOutputCommitter();
+ }
@Override
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502764575
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr, (Expression)
SessionStateUtil.getResource(conf,
Review Comment:
what would be a final filter in case of multiple jobContexts? multiple
duplicate filter expressions joined with `and`?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502764575
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr, (Expression)
SessionStateUtil.getResource(conf,
Review Comment:
what would be a final filter in case of multiple jobContexts? multiple
duplicate expressions joined with `and`?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502667923
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.BrokenBarrierException;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info(" Using HiveIcebergStorageHandlerStub for unit tests");
Review Comment:
all logging under debug, keep only what is needed
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502666193
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestUtilCyclicBarrier.java:
##
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.concurrent.CyclicBarrier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestUtilCyclicBarrier {
Review Comment:
why do you need a separate class for that? can't you do the same within Stub?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502651293
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -384,9 +384,12 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+Expression expr = (Expression) HiveIcebergInputFormat.getFilterExpr(conf,
predicate.pushedPredicate);
Review Comment:
please rename expr to filterExpr
if (filterExpr != null) {
SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, expr
!= filterExpr);
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1502651293 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -384,9 +384,12 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +Expression expr = (Expression) HiveIcebergInputFormat.getFilterExpr(conf, predicate.pushedPredicate); Review Comment: please rename expr to filterExpr -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1502625594 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -384,9 +384,12 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +Expression expr = (Expression) HiveIcebergInputFormat.getFilterExpr(conf, predicate.pushedPredicate); Review Comment: minor: (Expression) HiveIcebergInputFormat.getFilterExpr(conf, pushedPredicate) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502617418
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergInputFormat.java:
##
@@ -102,12 +102,25 @@ static Expression
icebergDataFilterFromHiveConf(Configuration conf) {
if (hiveFilter != null) {
ExprNodeGenericFuncDesc exprNodeDesc = SerializationUtilities
.deserializeObject(hiveFilter, ExprNodeGenericFuncDesc.class);
- SearchArgument sarg = ConvertAstToSearchArg.create(conf, exprNodeDesc);
- try {
+ return getFilterExpr(conf, exprNodeDesc);
+}
+return null;
+ }
+
+ /**
+ * getFilterExpr extracts search argument from ExprNodeGenericFuncDesc and
returns Iceberg Filter Expression
+ * @param conf - job conf
+ * @param exprNodeDesc - Describes a GenericFunc node
+ * @return Iceberg Filter Expression
+ */
+ public static Expression getFilterExpr(Configuration conf,
ExprNodeGenericFuncDesc exprNodeDesc) {
+try {
Review Comment:
keep in try only what is needed, just wrap the return. also package-private
seems enough
static Expression getFilterExpr(Configuration conf,
ExprNodeGenericFuncDesc exprNodeDesc){
if (exprNodeDesc != null) {
SearchArgument sarg = ConvertAstToSearchArg.create(conf, exprNodeDesc);
try {
return HiveIcebergFilterFactory.generateFilterExpression(sarg);
} catch (UnsupportedOperationException e) {
LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
}
}
return null;
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502630183
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -769,6 +771,9 @@ public void storageHandlerCommit(Properties
commitProperties, Operation operatio
}
}
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+return new HiveIcebergOutputCommitter();
+ }
@Override
Review Comment:
nit: new line
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1502625594 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -384,9 +384,12 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +Expression expr = (Expression) HiveIcebergInputFormat.getFilterExpr(conf, predicate.pushedPredicate); Review Comment: minor: (Expression) HiveIcebergInputFormat.getFilterExpr(conf, pushedPredicate) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1502625594 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -384,9 +384,12 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +Expression expr = (Expression) HiveIcebergInputFormat.getFilterExpr(conf, predicate.pushedPredicate); Review Comment: (Expression) HiveIcebergInputFormat.getFilterExpr(conf, pushedPredicate) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502618769
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -416,12 +417,13 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
Review Comment:
new line
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1502617418
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergInputFormat.java:
##
@@ -102,12 +102,25 @@ static Expression
icebergDataFilterFromHiveConf(Configuration conf) {
if (hiveFilter != null) {
ExprNodeGenericFuncDesc exprNodeDesc = SerializationUtilities
.deserializeObject(hiveFilter, ExprNodeGenericFuncDesc.class);
- SearchArgument sarg = ConvertAstToSearchArg.create(conf, exprNodeDesc);
- try {
+ return getFilterExpr(conf, exprNodeDesc);
+}
+return null;
+ }
+
+ /**
+ * getFilterExpr extracts search argument from ExprNodeGenericFuncDesc and
returns Iceberg Filter Expression
+ * @param conf - job conf
+ * @param exprNodeDesc - Describes a GenericFunc node
+ * @return Iceberg Filter Expression
+ */
+ public static Expression getFilterExpr(Configuration conf,
ExprNodeGenericFuncDesc exprNodeDesc) {
+try {
Review Comment:
keep in try only what is needed, just wrap the return
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1962777239 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1501685506
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
Review Comment:
done
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -510,6 +512,8 @@ private void commitWrite(Table table, String branchName,
Long snapshotId, long s
if (snapshotId != null) {
write.validateFromSnapshot(snapshotId);
}
+ write.conflictDetectionFilter(filterExpr);
Review Comment:
fixed
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1501685487
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
Fixed. Added cyclic barrier.
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
+ }
+} catch (UnsupportedOperationException e) {
+ LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
+}
+SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, expr);
+ }
+
+ /**
+ * Retrieves the query filter expression from the configuration.
+ *
+ * @param config The configuration to retrieve the filter expression from.
+ * @return The query filter expression or alwaysTrue if not found.
+ */
+ public static Expression getQueryFilter(Configuration config) {
Review Comment:
done
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440678452
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
+ }
+} catch (UnsupportedOperationException e) {
+ LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
+}
+SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, expr);
+ }
+
+ /**
+ * Retrieves the query filter expression from the configuration.
+ *
+ * @param config The configuration to retrieve the filter expression from.
+ * @return The query filter expression or alwaysTrue if not found.
+ */
+ public static Expression getQueryFilter(Configuration config) {
Review Comment:
why is it static? it you need static why it's in HiveIcebergStorageHandler
and not in iceberg util class?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440865605
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -408,12 +409,12 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr,
HiveIcebergStorageHandler.getQueryFilter(conf));
Review Comment:
if (filterExpr != null) {
filterExpr = SessionStateUtil.getResource(x.getJobConf(),
InputFormatConfig.QUERY_FILTERS)
.orElse(Expressions.alwaysTrue())
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440814569
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
+ }
+} catch (UnsupportedOperationException e) {
+ LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
+}
+SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, expr);
+ }
+
+ /**
+ * Retrieves the query filter expression from the configuration.
+ *
+ * @param config The configuration to retrieve the filter expression from.
+ * @return The query filter expression or alwaysTrue if not found.
+ */
+ public static Expression getQueryFilter(Configuration config) {
Review Comment:
remove this and use SessionStateUtil.getResource directly in
HiveIcebergOutputCommitter
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440736786
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
+ }
+} catch (UnsupportedOperationException e) {
+ LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
Review Comment:
move to constant as it's used in 2 places already
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440809493
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
Review Comment:
remove 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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1440793096 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +setFilterExpression(predicate); Review Comment: SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, HiveIcebergInputFormat.getFilterExpr(pushedPredicate)) refactor `icebergDataFilterFromHiveConf` to reuse `getFilterExpr` -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440757397
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
Review Comment:
how about `IcebergInputFormat#applyConfig`?
if (filter != null) {
setFilterExpr(filter)
}
private static setFilterExpr(Expression filter) {
SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS,
filter);
scan = scan.filter(filter);
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1440793096 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +setFilterExpression(predicate); Review Comment: SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, HiveIcebergInputFormat.getFilterExpr(pushedPredicate)) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761: URL: https://github.com/apache/hive/pull/4761#discussion_r1440793096 ## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java: ## @@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf jobConf, Deserializer dese } } predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate; +setFilterExpression(predicate); Review Comment: setFilterExpr(pushedPredicate) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440757397
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
Review Comment:
how about `IcebergInputFormat#applyConfig`?
if (filter != null) {
setFilterExpr(filter)
}
private static setFilterExpr(Expression filter) {
SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS,
filter);
scan = scan.filter(filter);
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440661822
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
Review Comment:
why not just pass `ExprNodeGenericFuncDesc pushedPredicate`? rename to
`setFilterExpr` and move to HiveIcebergInputFormat,
see `icebergDataFilterFromHiveConf`
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440661822
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
Review Comment:
why not just pass `ExprNodeGenericFuncDesc pushedPredicate`? rename to
`setFilterExpr` and move to HiveIcebergInputFormat,
see `icebergDataFilterFromHiveConf`
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440736786
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
+ }
+} catch (UnsupportedOperationException e) {
+ LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
Review Comment:
move to constant as it's used in 2 places already
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440661822
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
Review Comment:
why not just pass `ExprNodeGenericFuncDesc pushedPredicate`? rename to
`setFilterExpr` and move to iceberg util
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440661822
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
Review Comment:
why not just pass `ExprNodeGenericFuncDesc pushedPredicate`? rename to
`setFilterExpr`
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440678452
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
+ }
+} catch (UnsupportedOperationException e) {
+ LOG.warn("Unable to create Iceberg filter, continuing without filter
(will be applied by Hive later): ", e);
+}
+SessionStateUtil.addResource(conf, InputFormatConfig.QUERY_FILTERS, expr);
+ }
+
+ /**
+ * Retrieves the query filter expression from the configuration.
+ *
+ * @param config The configuration to retrieve the filter expression from.
+ * @return The query filter expression or alwaysTrue if not found.
+ */
+ public static Expression getQueryFilter(Configuration config) {
Review Comment:
why is it static? it you need static why it's in HiveIcebergStorageHandler
and not in iceberg util class?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440675961
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
> I tried the cyclicBarrier, both the threads would go to. sleep and never
wake up... -
it means that the code was not correct, `sleep` is a bad practice
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
> I tried the cyclicBarrier, both the threads would go to. sleep and never
wake up...
it means that the code was not correct, `sleep` is a bad practice
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440661822
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
Review Comment:
why not just pass `ExprNodeGenericFuncDesc pushedPredicate`?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1440656044
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -510,6 +512,8 @@ private void commitWrite(Table table, String branchName,
Long snapshotId, long s
if (snapshotId != null) {
write.validateFromSnapshot(snapshotId);
}
+ write.conflictDetectionFilter(filterExpr);
Review Comment:
should we add the filter in `OverwriteFiles`?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1860265636 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [12 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true) No data about Coverage No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1429871447
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
I tried the cyclicBarrier, both the threads would go to. sleep and never
wake up... The test would timeout as well.
I also tried the ALTER table but the storage_handler was being overwritten
here as earlier.
Now with this change,
https://github.com/apache/hive/pull/4761/files#diff-29da52649e7d8e89dcb9bba9617422fa9f6d6ab1df16733463d2d84e0900c697R393
. The storage handler mentioned during create table is propagated correctly.
So we won't need alter table command.
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1429866876
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
Review Comment:
`HiveIcebergFilterFactory.generateFilterExpression ` is invoked in multiple
places.
https://github.com/search?q=repo%3Aapache%2Fhive%20HiveIcebergFilterFactory.generateFilterExpression=code
We just want this to be set during decomposePredicate.
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1428900076
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -469,12 +470,13 @@ private Long getSnapshotId(Table table, String
branchName) {
/**
* Creates and commits an Iceberg change with the provided data and delete
files.
* If there are no delete files then an Iceberg 'append' is created,
otherwise Iceberg 'overwrite' is created.
- * @param table The table we are changing
- * @param startTime The start time of the commit - used only for logging
- * @param results The object containing the new files we would like to add
to the table
+ * @param table The table we are changing
+ * @param startTime The start time of the commit - used only for logging
+ * @param resultsThe object containing the new files we would like to
add to the table
+ * @param conflictDetectionFilter Filter expression for conflict detection
filter
*/
private void commitWrite(Table table, String branchName, Long snapshotId,
long startTime,
- FilesForCommit results, Operation operation) {
+ FilesForCommit results, Operation operation, Expression
conflictDetectionFilter) {
Review Comment:
done
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -510,6 +512,8 @@ private void commitWrite(Table table, String branchName,
Long snapshotId, long s
if (snapshotId != null) {
write.validateFromSnapshot(snapshotId);
}
+ write.conflictDetectionFilter(conflictDetectionFilter);
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1428899967
##
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##
@@ -2231,7 +2231,6 @@ public static enum ConfVars {
"If this is set to true the URI for auth will have the default
location masked with DEFAULT_TABLE_LOCATION"),
HIVE_ICEBERG_ALLOW_DATAFILES_IN_TABLE_LOCATION_ONLY("hive.iceberg.allow.datafiles.in.table.location.only",
false,
"If this is set to true, then all the data files being read should be
withing the table location"),
-
Review Comment:
Done
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -388,8 +390,11 @@ private void setHmsTableParameters(String
newMetadataLocation, Table tbl, TableM
// If needed set the 'storage_handler' property to enable query from Hive
if (hiveEngineEnabled) {
- parameters.put(hive_metastoreConstants.META_TABLE_STORAGE,
- "org.apache.iceberg.mr.hive.HiveIcebergStorageHandler");
+ String storageHandler =
parameters.get(hive_metastoreConstants.META_TABLE_STORAGE);
+ // Check if META_TABLE_STORAGE is not present or does not contain
ICEBERG_STORAGE_HANDLER
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425550230
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
35sec sleep? can't you use some barier?
barrier = new CyclicBarrier(N);
barrier.await(35, TimeUnit.SECONDS);
in test replace storage handler with the stub right before running the
concurrent operations
shell.executeStatement("ALTER TABLE customers SET TBLPROPERTIES " +
"'storage_handler'='HiveIcebergStorageHandlerStub'");
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425524946
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -469,12 +470,13 @@ private Long getSnapshotId(Table table, String
branchName) {
/**
* Creates and commits an Iceberg change with the provided data and delete
files.
* If there are no delete files then an Iceberg 'append' is created,
otherwise Iceberg 'overwrite' is created.
- * @param table The table we are changing
- * @param startTime The start time of the commit - used only for logging
- * @param results The object containing the new files we would like to add
to the table
+ * @param table The table we are changing
+ * @param startTime The start time of the commit - used only for logging
+ * @param resultsThe object containing the new files we would like to
add to the table
+ * @param conflictDetectionFilter Filter expression for conflict detection
filter
*/
private void commitWrite(Table table, String branchName, Long snapshotId,
long startTime,
- FilesForCommit results, Operation operation) {
+ FilesForCommit results, Operation operation, Expression
conflictDetectionFilter) {
Review Comment:
please rename method param to `filterExpr`
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425550230
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
35sec sleep? can't you use some barier?
barrier = new CyclicBarrier(N);
barrier.await(35, TimeUnit.SECONDS);
in test replace storage handler with the stub right before running the
concurrent operations
shell.executeStatement("ALTER TABLE customers SET TBLPROPERTIES " +
"'storage_handler'='[HiveIcebergStorageHandlerStub')");
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425550230
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
35sec sleep? can't you use some barier?
barrier = new CyclicBarrier(N);
barrier.await(35, TimeUnit.SECONDS);
in test replace storage handler with the stub before running concurrent
operation
shell.executeStatement("ALTER TABLE customers SET TBLPROPERTIES " +
"'storage_handler'='[HiveIcebergStorageHandlerStub')");
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425550230
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerStub.java:
##
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.mr.hive;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * HiveIcebergStorageHandlerStub is used only for unit tests.
+ * Currently, we use it to achieve a specific thread interleaving to simulate
conflicts in concurrent writes
+ * deterministically.
+ */
+public class HiveIcebergStorageHandlerStub extends HiveIcebergStorageHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveIcebergStorageHandlerStub.class);
+
+ @Override
+ public HiveIcebergOutputCommitter getOutputCommitter() {
+
+try {
+ LOG.info("Using HiveIcebergStorageHandlerStub for unit tests");
+ LOG.info("Sleeping thread: {} ", Thread.currentThread().getName());
+ Thread.sleep(35000);
Review Comment:
35sec sleep? can't you use some barier?
barrier = new CyclicBarrier(N);
barrier.await(35, TimeUnit.SECONDS);
replace storage handler before running concurrent operation
shell.executeStatement("ALTER TABLE customers SET TBLPROPERTIES " +
"'storage_handler'='[HiveIcebergStorageHandlerStub')");
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425530365
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -381,9 +381,39 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = HiveIcebergFilterFactory.generateFilterExpression(sarg);
Review Comment:
why don't we set the filter inside
`HiveIcebergFilterFactory.generateFilterExpression` when it's invoked
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425525469
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -510,6 +512,8 @@ private void commitWrite(Table table, String branchName,
Long snapshotId, long s
if (snapshotId != null) {
write.validateFromSnapshot(snapshotId);
}
+ write.conflictDetectionFilter(conflictDetectionFilter);
Review Comment:
`filterExpr`
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425524946
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -469,12 +470,13 @@ private Long getSnapshotId(Table table, String
branchName) {
/**
* Creates and commits an Iceberg change with the provided data and delete
files.
* If there are no delete files then an Iceberg 'append' is created,
otherwise Iceberg 'overwrite' is created.
- * @param table The table we are changing
- * @param startTime The start time of the commit - used only for logging
- * @param results The object containing the new files we would like to add
to the table
+ * @param table The table we are changing
+ * @param startTime The start time of the commit - used only for logging
+ * @param resultsThe object containing the new files we would like to
add to the table
+ * @param conflictDetectionFilter Filter expression for conflict detection
filter
*/
private void commitWrite(Table table, String branchName, Long snapshotId,
long startTime,
- FilesForCommit results, Operation operation) {
+ FilesForCommit results, Operation operation, Expression
conflictDetectionFilter) {
Review Comment:
filterExpr
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425520564
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -408,12 +409,12 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr,
HiveIcebergStorageHandler.getQueryFilter(conf));
Review Comment:
why do we need this in a jobContext loop? we'll have multiple jobs in case
of split-update and we'll add duplicate filters?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425520564
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergOutputCommitter.java:
##
@@ -408,12 +409,12 @@ private void commitTable(FileIO io, ExecutorService
executor, OutputTable output
Table table = null;
String branchName = null;
-
+Expression filterExpr = Expressions.alwaysTrue();
for (JobContext jobContext : outputTable.jobContexts) {
JobConf conf = jobContext.getJobConf();
table = Optional.ofNullable(table).orElse(Catalogs.loadTable(conf,
catalogProperties));
branchName = conf.get(InputFormatConfig.OUTPUT_TABLE_SNAPSHOT_REF);
-
+ filterExpr = Expressions.and(filterExpr,
HiveIcebergStorageHandler.getQueryFilter(conf));
Review Comment:
why do we need this in a jobContext loop?
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425477828
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -591,4 +596,19 @@ HiveLock lockObject(TableMetadata metadata) {
return new NoLock();
}
}
+
+ /**
+ * Checks if the storage_handler property is already set to
HIVE_ICEBERG_STORAGE_HANDLER.
+ * @param storageHandler Storage Handler class
+ * @return true if the storage_handler property is set to
HIVE_ICEBERG_STORAGE_HANDLER
+ */
+ private static boolean isHiveIcebergStorageHandler(String storageHandler) {
Review Comment:
remove 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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425476080
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -388,8 +390,11 @@ private void setHmsTableParameters(String
newMetadataLocation, Table tbl, TableM
// If needed set the 'storage_handler' property to enable query from Hive
if (hiveEngineEnabled) {
- parameters.put(hive_metastoreConstants.META_TABLE_STORAGE,
- "org.apache.iceberg.mr.hive.HiveIcebergStorageHandler");
+ String storageHandler =
parameters.get(hive_metastoreConstants.META_TABLE_STORAGE);
+ // Check if META_TABLE_STORAGE is not present or does not contain
ICEBERG_STORAGE_HANDLER
Review Comment:
if (storageHandler == null ||
HiveIcebergStorageHandler.class.isAssignableFrom(
Class.forName(storageHandler)) {
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425471176
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -85,6 +85,8 @@ public class HiveTableOperations extends
BaseMetastoreTableOperations {
private static final String NO_LOCK_EXPECTED_VALUE =
"expected_parameter_value";
private static final long HIVE_TABLE_PROPERTY_MAX_SIZE_DEFAULT = 32672;
+ private static final String HIVE_ICEBERG_STORAGE_HANDLER =
"org.apache.iceberg.mr.hive.HiveIcebergStorageHandler";
Review Comment:
why do you need 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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425477828
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -591,4 +596,19 @@ HiveLock lockObject(TableMetadata metadata) {
return new NoLock();
}
}
+
+ /**
+ * Checks if the storage_handler property is already set to
HIVE_ICEBERG_STORAGE_HANDLER.
+ * @param storageHandler Storage Handler class
+ * @return true if the storage_handler property is set to
HIVE_ICEBERG_STORAGE_HANDLER
+ */
+ private static boolean isHiveIcebergStorageHandler(String storageHandler) {
Review Comment:
remove 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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425472876
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -591,4 +596,19 @@ HiveLock lockObject(TableMetadata metadata) {
return new NoLock();
}
}
+
+ /**
+ * Checks if the storage_handler property is already set to
HIVE_ICEBERG_STORAGE_HANDLER.
+ * @param storageHandler Storage Handler class
+ * @return true if the storage_handler property is set to
HIVE_ICEBERG_STORAGE_HANDLER
+ */
+ private static boolean isHiveIcebergStorageHandler(String storageHandler) {
+try {
+ Class storageHandlerClass = Class.forName(storageHandler);
+ Class icebergStorageHandlerClass =
Class.forName(HIVE_ICEBERG_STORAGE_HANDLER);
+ return icebergStorageHandlerClass.isAssignableFrom(storageHandlerClass);
Review Comment:
HiveIcebergStorageHandler.class.isAssignableFrom(Class.forName(storageHandler))
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425476080
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -388,8 +390,11 @@ private void setHmsTableParameters(String
newMetadataLocation, Table tbl, TableM
// If needed set the 'storage_handler' property to enable query from Hive
if (hiveEngineEnabled) {
- parameters.put(hive_metastoreConstants.META_TABLE_STORAGE,
- "org.apache.iceberg.mr.hive.HiveIcebergStorageHandler");
+ String storageHandler =
parameters.get(hive_metastoreConstants.META_TABLE_STORAGE);
+ // Check if META_TABLE_STORAGE is not present or does not contain
ICEBERG_STORAGE_HANDLER
Review Comment:
if (storageHandler == null ||
HiveIcebergStorageHandler.class.isAssignableFrom(
Class.forName(storageHandler)) {
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425476080
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -388,8 +390,11 @@ private void setHmsTableParameters(String
newMetadataLocation, Table tbl, TableM
// If needed set the 'storage_handler' property to enable query from Hive
if (hiveEngineEnabled) {
- parameters.put(hive_metastoreConstants.META_TABLE_STORAGE,
- "org.apache.iceberg.mr.hive.HiveIcebergStorageHandler");
+ String storageHandler =
parameters.get(hive_metastoreConstants.META_TABLE_STORAGE);
+ // Check if META_TABLE_STORAGE is not present or does not contain
ICEBERG_STORAGE_HANDLER
Review Comment:
if (storageHandler == null ||
HiveIcebergStorageHandler.class.isAssignableFrom(
Class.forName(storageHandler)) {
}
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425472876
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -591,4 +596,19 @@ HiveLock lockObject(TableMetadata metadata) {
return new NoLock();
}
}
+
+ /**
+ * Checks if the storage_handler property is already set to
HIVE_ICEBERG_STORAGE_HANDLER.
+ * @param storageHandler Storage Handler class
+ * @return true if the storage_handler property is set to
HIVE_ICEBERG_STORAGE_HANDLER
+ */
+ private static boolean isHiveIcebergStorageHandler(String storageHandler) {
+try {
+ Class storageHandlerClass = Class.forName(storageHandler);
+ Class icebergStorageHandlerClass =
Class.forName(HIVE_ICEBERG_STORAGE_HANDLER);
+ return icebergStorageHandlerClass.isAssignableFrom(storageHandlerClass);
Review Comment:
HiveIcebergStorageHandler.class.isAssignableFrom(Class.forName(storageHandler))
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425471176
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -85,6 +85,8 @@ public class HiveTableOperations extends
BaseMetastoreTableOperations {
private static final String NO_LOCK_EXPECTED_VALUE =
"expected_parameter_value";
private static final long HIVE_TABLE_PROPERTY_MAX_SIZE_DEFAULT = 32672;
+ private static final String HIVE_ICEBERG_STORAGE_HANDLER =
"org.apache.iceberg.mr.hive.HiveIcebergStorageHandler";
Review Comment:
why do you need 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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425452337
##
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##
@@ -388,8 +390,11 @@ private void setHmsTableParameters(String
newMetadataLocation, Table tbl, TableM
// If needed set the 'storage_handler' property to enable query from Hive
if (hiveEngineEnabled) {
- parameters.put(hive_metastoreConstants.META_TABLE_STORAGE,
- "org.apache.iceberg.mr.hive.HiveIcebergStorageHandler");
+ String storageHandler =
parameters.get(hive_metastoreConstants.META_TABLE_STORAGE);
+ // Check if META_TABLE_STORAGE is not present or does not contain
ICEBERG_STORAGE_HANDLER
Review Comment:
or is not an instance of ICEBERG_STORAGE_HANDLER
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
deniskuzZ commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1425447874
##
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##
@@ -2231,7 +2231,6 @@ public static enum ConfVars {
"If this is set to true the URI for auth will have the default
location masked with DEFAULT_TABLE_LOCATION"),
HIVE_ICEBERG_ALLOW_DATAFILES_IN_TABLE_LOCATION_ONLY("hive.iceberg.allow.datafiles.in.table.location.only",
false,
"If this is set to true, then all the data files being read should be
withing the table location"),
-
Review Comment:
remove 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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1852835441 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4761) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [3 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=true) No data about Coverage No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4761) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1851715131 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1850899483 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1849830864 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1847092758 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1843498648 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1842146722 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1833604993 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1827118247 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1826943830 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1816828368 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1794330923 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1792578739 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1771641212 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1770429400 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1365054556
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestConflictingDataFiles.java:
##
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.List;
+import java.util.concurrent.Executors;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.mr.TestHelper;
+import org.apache.iceberg.relocated.com.google.common.base.Throwables;
+import org.apache.iceberg.util.Tasks;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static
org.apache.iceberg.mr.hive.HiveIcebergStorageHandlerTestUtils.init;
+
+public class TestConflictingDataFiles extends
HiveIcebergStorageHandlerWithEngineBase {
+
+ @Test
+ public void testSingleFilterUpdate() {
+
+PartitionSpec spec =
+
PartitionSpec.builderFor(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA).identity("last_name")
+.bucket("customer_id", 16).build();
+
+// create and insert an initial batch of records
+testTables.createTable(shell, "customers",
HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, spec, fileFormat,
+HiveIcebergStorageHandlerTestUtils.OTHER_CUSTOMER_RECORDS_2, 2);
+// insert one more batch so that we have multiple data files within the
same partition
+
shell.executeStatement(testTables.getInsertQuery(HiveIcebergStorageHandlerTestUtils.OTHER_CUSTOMER_RECORDS_1,
+TableIdentifier.of("default", "customers"), false));
+
+String[] singleFilterQuery = new String[] { "UPDATE customers SET
first_name='Changed' WHERE last_name='Taylor'",
+"UPDATE customers SET first_name='Changed' WHERE last_name='Donnel'"
};
+
+try {
+ Tasks.range(2).executeWith(Executors.newFixedThreadPool(2)).run(i -> {
+init(shell, testTables, temp, executionEngine);
+HiveConf.setBoolVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, isVectorized);
+HiveConf.setVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVEFETCHTASKCONVERSION, "none");
+HiveConf.setVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_QUERY_REEXECUTION_STRATEGIES,
+RETRY_STRATEGIES_WITHOUT_WRITE_CONFLICT);
+shell.executeStatement(singleFilterQuery[i]);
+shell.closeSession();
+ });
+} catch (Throwable ex) {
+ // If retry succeeds then it should not throw an ValidationException.
+ Throwable cause = Throwables.getRootCause(ex);
+ if (cause instanceof ValidationException &&
cause.getMessage().matches("^Found.*conflicting.*files(.*)")) {
+Assert.fail();
+ }
+}
+
+List objects =
+shell.executeStatement("SELECT * FROM customers ORDER BY customer_id,
last_name, first_name");
+Assert.assertEquals(12, objects.size());
+List expected =
TestHelper.RecordsBuilder.newInstance(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA)
+.add(1L, "Joanna", "Pierce")
+.add(1L, "Changed", "Taylor")
+.add(2L, "Changed", "Donnel")
+.add(2L, "Susan", "Morrison")
+.add(2L, "Bob", "Silver")
+.add(2L, "Joanna", "Silver")
+.add(3L, "Marci", "Barna")
+.add(3L, "Blake", "Burr")
+.add(3L, "Trudy", "Henderson")
+.add(3L, "Trudy", "Johnson")
+.add(4L, "Laci", "Zold")
+.add(5L, "Peti", "Rozsaszin").build();
+HiveIcebergTestUtils.validateData(expected,
+
HiveIcebergTestUtils.valueForRow(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
objects), 0);
+
+ }
+
+ @Test
+ public void testMultiFiltersUpdate() {
+
+PartitionSpec spec =
+
PartitionSpec.builderFor(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA).identity("last_name")
+.bucket("customer_id", 16).build();
+
+// create and insert an initial batch of records
+testTables.createTable(shell, "customers",
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1768723936 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1768040802 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
sonarcloud[bot] commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1767242473 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4761) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4761=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4761=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4761=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4761=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on PR #4761: URL: https://github.com/apache/hive/pull/4761#issuecomment-1766742007 Thanks again @ayushtkn for the review -- 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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
Re: [PR] HIVE-27653: Iceberg: Add conflictDetectionFilter to validate concurre… [hive]
simhadri-g commented on code in PR #4761:
URL: https://github.com/apache/hive/pull/4761#discussion_r1362395908
##
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##
@@ -382,9 +383,42 @@ public DecomposedPredicate decomposePredicate(JobConf
jobConf, Deserializer dese
}
}
predicate.pushedPredicate = (ExprNodeGenericFuncDesc) pushedPredicate;
+setFilterExpression(predicate);
return predicate;
}
+ /**
+ * Sets the filter expression in the configuration.
+ *
+ * @param predicate The predicate to create the filter expression from.
+ */
+ public void setFilterExpression(DecomposedPredicate predicate) {
+Expression expr = Expressions.alwaysTrue();
+try {
+ if (predicate.pushedPredicate != null) {
+SearchArgument sarg = ConvertAstToSearchArg.create(conf,
predicate.pushedPredicate);
+expr = Expressions.and(expr,
HiveIcebergFilterFactory.generateFilterExpression(sarg));
Review Comment:
Fixed
##
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestConflictingDataFiles.java:
##
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.apache.iceberg.mr.hive;
+
+import java.util.List;
+import java.util.concurrent.Executors;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.mr.TestHelper;
+import org.apache.iceberg.relocated.com.google.common.base.Throwables;
+import org.apache.iceberg.util.Tasks;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static
org.apache.iceberg.mr.hive.HiveIcebergStorageHandlerTestUtils.init;
+
+public class TestConflictingDataFiles extends
HiveIcebergStorageHandlerWithEngineBase {
+
+ @Test
+ public void testSingleFilterUpdate() {
+
+PartitionSpec spec =
+
PartitionSpec.builderFor(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA).identity("last_name")
+.bucket("customer_id", 16).build();
+
+// create and insert an initial batch of records
+testTables.createTable(shell, "customers",
HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA, spec, fileFormat,
+HiveIcebergStorageHandlerTestUtils.OTHER_CUSTOMER_RECORDS_2, 2);
+// insert one more batch so that we have multiple data files within the
same partition
+
shell.executeStatement(testTables.getInsertQuery(HiveIcebergStorageHandlerTestUtils.OTHER_CUSTOMER_RECORDS_1,
+TableIdentifier.of("default", "customers"), false));
+
+String[] singleFilterQuery = new String[] { "UPDATE customers SET
first_name='Changed' WHERE last_name='Taylor'",
+"UPDATE customers SET first_name='Changed' WHERE last_name='Donnel'"
};
+
+try {
+ Tasks.range(2).executeWith(Executors.newFixedThreadPool(2)).run(i -> {
+init(shell, testTables, temp, executionEngine);
+HiveConf.setBoolVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, isVectorized);
+HiveConf.setVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVEFETCHTASKCONVERSION, "none");
+HiveConf.setVar(shell.getHiveConf(),
HiveConf.ConfVars.HIVE_QUERY_REEXECUTION_STRATEGIES,
+RETRY_STRATEGIES_WITHOUT_WRITE_CONFLICT);
+shell.executeStatement(singleFilterQuery[i]);
+shell.closeSession();
+ });
+} catch (Throwable ex) {
+ // If retry succeeds then it should not throw an ValidationException.
+ Throwable cause = Throwables.getRootCause(ex);
+ if (cause instanceof ValidationException &&
cause.getMessage().matches("^Found.*conflicting.*files(.*)")) {
+Assert.fail();
+ }
Review Comment:
Fixed
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
