namanjain24-sudo commented on issue #25100:
URL: https://github.com/apache/datafusion/issues/25100#issuecomment-5661070922

   I ran this directly instead of reasoning about it further. Setup: `main` at 
c2cf289, substrait-java / Isthmus 0.103.0 (the latest release), the 22 TPC-H 
queries, SF 0.01 data. @alexandrefimov's comment above already covers what the 
existing suites test, so this is only what a fresh run shows.
   
   **Java → DataFusion.** I used plans generated by Isthmus from its own TPC-H 
queries.
   
   - As generated, 0 of 22 load. The plans name tables in upper case and 
DataFusion registers them in lower case, so every one fails with `No table 
named 'LINEITEM'` (#14831).
   - With identifier normalization turned off so the names match, 7 of 22 run. 
All 7 return the same rows as DuckDB running the same queries (compared to 4 
decimal places).
   - Of the other 15:
     - 11 fail on `VarChar` literals. The consumer accepts `FixedChar` but has 
no `VarChar` case.
     - 3 are correlated subqueries. Isthmus now writes 
`OuterReference.rel_reference`, a field the `substrait` 0.63.0 crate we pin 
doesn't have, so the consumer sees `steps_out = 0`.
     - 1 has an enum function argument (#12493).
   
   The checked-in TPC-H plans from 2024 contain no `VarChar` literals and no 
`rel_reference`, so those tests can't see either gap.
   
   **DataFusion → Java.** DataFusion produces all 22 plans, and substrait-java 
0.103.0 fails on all 22 at the extension URN (#11545).
   
   **A correction to my earlier comment on this issue.** I wrote "Nothing is 
rejected." That is true of substrait-java's plan converter, but not end to end. 
I took an Isthmus plan for `SELECT COUNT(I), SUM(I), AVG(I)` over the values 1, 
2, 3 and changed only `AggregateFunction.phase`. With `INITIAL_TO_RESULT`, 
substrait-spark 0.103.0 on Spark 3.5.4 returns `[3, 6, 2.0]`. With 
`UNSPECIFIED`, it maps the aggregates to `Final`, and Spark rejects the plan 
during analysis with `MISSING_ATTRIBUTES`. So the phase bug is real, but in 
Spark it shows up as a failure, not as wrong results.
   
   The harness is small: one Java class for Isthmus and substrait-java, a Rust 
example for DataFusion, and DuckDB for the reference rows. If it's useful, I 
can turn it into a job that runs on a schedule rather than on every PR, either 
here or in consumer-testing, whichever you prefer. The `VarChar` literal and 
`rel_reference` gaps don't seem to have issues yet, and I can file them.


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to