ErikBPF opened a new pull request, #6897:
URL: https://github.com/apache/datafusion-comet/pull/6897

   ## Which issue does this PR close?
   
   Related to #6418. This fixes a Python benchmark registration issue 
discovered during query validation; it does not resolve the reported 
performance investigation.
   
   ## Rationale for this change
   
   The Python file-backed runner infers Parquet columns as STRING, losing the 
declared TPC-DS CHAR types. Spark's TPC-DS generator intentionally pads CHAR 
values. Without their logical types, predicates such as `i_category = 'Men'` do 
not use Spark's CHAR comparison semantics and can incorrectly eliminate 
qualifying data.
   
   [Spark's datatype 
documentation](https://spark.apache.org/docs/4.2.0/sql-ref-datatypes.html) 
specifies these semantics. Spark's own 
[`TPCDSQueryBenchmark`](https://github.com/apache/spark/blob/v4.2.0/sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/TPCDSQueryBenchmark.scala)
 supplies table schemas when registering files, and 
[`CharVarcharTestSuite`](https://github.com/apache/spark/blob/v4.2.0/sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala)
 covers external-table padding, comparisons, NULLs and overlength values.
   
   ## What changes are included in this PR?
   
   - Register file-backed TPC-DS Parquet using 24 pinned Spark table schemas in 
isolated external-table metadata, then expose temporary views and clean up 
registration metadata.
   - Preserve source files and query SQL while restoring logical CHAR/VARCHAR 
types; retain existing TPC-H, non-Parquet and catalog registration paths.
   - Bundle the schema module in the benchmark image and document the behavior 
and regression command.
   - Add a runnable regression through the production registration/query entry 
points, including comparison semantics, declarations, reader options, layouts, 
isolation and cleanup.
   
   ## How are these changes tested?
   
   - Before the fix, a regression through the existing production runner failed 
with one qualifying row instead of two. Typed registration restores the 
expected result without trimming values or rewriting the query.
   - Run `spark-submit --master 'local[8]' test_tpcbench.py` from 
`benchmarks/tpc`, using Spark 4.2.0 and then the Comet plugin configuration 
with an existing Comet 1.1.0 JAR. Tests compare real Parquet data, logical 
metadata and exact results; source fixture files are content-hashed 
before/after.
   - Verify all 24 declarations / 425 columns against the pinned upstream 
schema source.
   - Before the final reader-option normalization change, unchanged q54 
returned 35 rows with matching fingerprints on Spark and Comet. q54 was not 
rerun afterward; the full regression passed on both engines after that change. 
Source file sizes/mtimes remained unchanged; this smoke is not a byte-level 
rehash or a performance-improvement claim.
   - A boolean reader option reproduced a Boolean/String conversion failure 
before normalization; the final full regression passed on Spark and Comet 
afterward. A real catalog table-creation failure checks namespace cleanup and 
source-file preservation.
   - Python compilation, whitespace and benchmark image schema-module inclusion 
checks.
   
   Validation is limited to Spark 4.2.0 and the existing Comet 1.1.0 runtime. 
Older Spark versions, a fresh main-native build and genuine Iceberg execution 
were not exercised. Catalog routing is tested through real Spark catalog tables.
   


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