You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "Xiangakun (via GitHub)" <gi...@apache.org> on 2023/12/03 12:11:27 UTC

[PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Xiangakun opened a new pull request, #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   Fix bug: #2425
   <!-- Linking this pull request to the issue -->
   Linked issue: close #2425
   
   <!-- What is the purpose of the change -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838261828

   @Xiangakun see https://github.com/apache/incubator-paimon/pull/2445


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Xiangakun (via GitHub)" <gi...@apache.org>.
Xiangakun commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838290846

   > @Xiangakun see #2445
   
   It seems there is some thing wrong with the fix, following code should be kept, since the writeBuilder in `bucketMode match { }` is not the same with the one after `collect`。And your test case does not cover the real compact action, it will not do a compaction since only one file in each bucket
   ![image](https://github.com/apache/incubator-paimon/assets/20922357/41c369df-350d-4cab-af43-cdbd53a31a0a)
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Xiangakun (via GitHub)" <gi...@apache.org>.
Xiangakun commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838824017

   > @Xiangakun
   > 
   > > since the writeBuilder in bucketMode match { } is not the same with the one after collect
   > 
   > Can you explain it in more detail?
   Sorry, you're correct, I made a mistake ~
   
   > > And your test case does not cover the real compact action, it will not do a compaction since only one file in each bucket
   > 
   > In fact, the trigger of this bug is that when inserting overwrite partition pk table, restoreFiles + new file can reach `num-sorted-run.compaction-trigger` (no need compaction to be done before), my test can just cover it.
   
   Yes, I agree with you, and I think it's necessary to test the compact action along with insert overwrite, and make sure , the data is still correct after compaction, thanks~
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838695985

   @Xiangakun
   
   > since the writeBuilder in bucketMode match { } is not the same with the one after collect
   
   Can you explain it in more detail?
   
   > And your test case does not cover the real compact action, it will not do a compaction since only one file in each bucket
   
   In fact, the trigger of this bug is that when inserting overwrite partition pk table, restoreFiles + new file can reach `num-sorted-run.compaction-trigger` (no need compaction to be done before), my test can just cover it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Xiangakun (via GitHub)" <gi...@apache.org>.
Xiangakun commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838489175

   Updated, thanks~


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1837811839

   Fix should be:
   ```
   val writeBuilder = table.newBatchWriteBuilder()
       if (overwritePartition != null) {
         writeBuilder.withOverwrite(overwritePartition.asJava)
       }
   ```
   
   And then:
   ```
   val tableCommit = writeBuilder.newCommit()
   ```
   
   You should add a test for this too.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1837809838

   @Xiangakun Thanks! It is true.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Xiangakun (via GitHub)" <gi...@apache.org>.
Xiangakun commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838410659

   And, when use parquet format, seems some jar conflicts:
   
   <img width="790" alt="image" src="https://github.com/apache/incubator-paimon/assets/20922357/ed10f341-9636-4a28-a313-acc9495741b3">
   
   Job aborted due to stage failure: Task 0 in stage 3.0 failed 1 times, most recent failure: Lost task 0.0 in stage 3.0 (TID 3) (10.10.128.57 executor driver): java.lang.NoSuchMethodError: org.apache.parquet.Preconditions.checkArgument(ZLjava/lang/String;Ljava/lang/Object;)V
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "YannByron (via GitHub)" <gi...@apache.org>.
YannByron commented on code in PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#discussion_r1413348170


##########
paimon-spark/paimon-spark-3.3/src/test/scala/org/apache/paimon/spark/sql/InsertOverwriteTest.scala:
##########
@@ -37,7 +37,8 @@ class InsertOverwriteTest extends PaimonSparkTestBase {
 
             spark.sql(s"""
                          |CREATE TABLE T (a INT, b INT, c STRING)
-                         |TBLPROPERTIES ($primaryKeysProp 'bucket'='$bucket')
+                         |TBLPROPERTIES ($primaryKeysProp 'bucket'='$bucket',
+                         |'num-sorted-run.compaction-trigger'='1')

Review Comment:
   Do not use existing test cases to test your cases. Add a new one for it.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Xiangakun (via GitHub)" <gi...@apache.org>.
Xiangakun commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1837800194

   @YannByron @JingsongLi 
   Hello, guys,could you please help take a look at this pr? It has some blocking in our case, thanks~


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [spark] Fix insert overwrite with compact error (#2425) [incubator-paimon]

Posted by "Xiangakun (via GitHub)" <gi...@apache.org>.
Xiangakun commented on PR #2441:
URL: https://github.com/apache/incubator-paimon/pull/2441#issuecomment-1838377425

   One question, it seems ORC file does not support to rolling file, correct? 
   
   <img width="1089" alt="image" src="https://github.com/apache/incubator-paimon/assets/20922357/c50c3463-a2d2-412f-b583-0e503b5c5ee6">
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org