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

   ## Which issue does this PR close?
   
   N/A - minor cleanup
   
   ## Rationale for this change
   
   The `-- ConfigMatrix: parquet.enable.dictionary=false,true` directive was 
added as boilerplate to all 136 SQL expression test files. This causes each 
test to run twice (once with dictionary encoding enabled, once disabled), but 
is redundant because the tests generally do not create tables with duplicate 
string data that would actually be dictionary-encoded by Parquet. Removing it 
halves the number of SQL file test cases without reducing coverage.
   
   ## What changes are included in this PR?
   
   - Remove the `-- ConfigMatrix: parquet.enable.dictionary=false,true` 
directive from all 136 SQL test files
   - Update the contributor guide (`sql-file-tests.md`) to stop recommending 
this directive for new tests and use a more meaningful ConfigMatrix example
   - Update the expression guide (`adding_a_new_expression.md`) to remove the 
directive from the example SQL test file
   - Update the PR review skill to match
   
   ## How are these changes tested?
   
   The SQL file tests still run, just without the redundant dictionary encoding 
matrix. Existing test coverage is preserved since the dictionary encoding 
setting has no meaningful effect on these tests.


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