weiqingy commented on code in PR #2452:
URL: https://github.com/apache/auron/pull/2452#discussion_r3746365053


##########
thirdparty/auron-paimon/src/test/scala/org/apache/auron/paimon/AuronPaimonV2IntegrationSuite.scala:
##########
@@ -143,6 +143,24 @@ class AuronPaimonV2IntegrationSuite
     }
   }
 
+  test("paimon v2 COW primary-key table preserves latest value across 
commits") {
+    withTable("paimon.db.t_cow_multi_commit") {
+      sql("""
+            |create table paimon.db.t_cow_multi_commit (id int, v string)
+            |using paimon
+            |tblproperties (
+            |  'primary-key' = 'id',
+            |  'bucket' = '2',
+            |  'full-compaction.delta-commits' = '1'
+            |)
+            |""".stripMargin)
+      sql("insert into paimon.db.t_cow_multi_commit values (1, 'a'), (2, 'b')")
+      sql("insert into paimon.db.t_cow_multi_commit values (1, 'updated')")
+      val df = sql("select * from paimon.db.t_cow_multi_commit")
+      checkAnswer(df, Seq(Row(1, "updated"), Row(2, "b")))

Review Comment:
   The PR description says this test checks that the query still uses 
`NativePaimonV2TableScan`, but `checkAnswer` is the only assertion here.
   
   The catch is that the answer comes out the same either way. If the native 
scan bails out, Spark falls back to its own Paimon reader, which merges by 
primary key and returns those same two rows, so the test stays green even if it 
never touches the native path. `t_mor` at line 234 is exactly that combination: 
no native scan, correct answer.
   
   And the bail-out is a real possibility here. 
`PaimonScanSupport.scala:162-173` drops the native plan when a split isn't 
raw-readable, and `PaimonConvertProvider.scala:52` and `:95` then quietly leave 
Spark's own scan in place.
   
   Every other test in this suite that expects a native scan says so, directly 
(lines 49, 59, 73, 91, 105, 116, 142, 169, 408) or through 
`checkSparkAnswerAndNativePaimonScan` and `executedNativeScan`. Would it be 
worth doing the same here? It would also make this the first test pinning the 
native path for a COW primary-key table past its first commit.
   
   One line, in case it helps:
   
   ```suggestion
         checkAnswer(df, Seq(Row(1, "updated"), Row(2, "b")))
         assertNativePaimonScanApplied(df)
   ```
   
   And if the scan turns out not to stay native after a second commit, that 
feels worth knowing 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to