kosiew commented on code in PR #24493:
URL: https://github.com/apache/datafusion/pull/24493#discussion_r3818829889


##########
datafusion/sqllogictest/bin/sqllogictests.rs:
##########


Review Comment:
   It looks like `--substrait-round-trip` still dispatches directly to 
`run_test_file_substrait_round_trip`, so this path never parses or applies 
`configMatrix` combinations.
   
   That means an `.slt` file with a matrix can silently run only once in this 
supported mode, even though the directive suggests the file will be exercised 
across all combinations. Could we either route this path through the same 
per-combination setup, or explicitly reject `configMatrix` when Substrait 
round-trip mode is used?
   
   It would also be good to add a regression test that verifies each matrix 
combination is actually executed.



##########
datafusion/sqllogictest/README.md:
##########
@@ -184,6 +184,42 @@ EXPLAIN ANALYZE SELECT * FROM generate_series(100);
 Plan with Metrics LazyMemoryExec: partitions=1, 
batch_generators=[generate_series: start=0, end=100, batch_size=8192], 
metrics=[output_rows=101, elapsed_compute=<slt:ignore>, 
output_bytes=<slt:ignore>]
 ```
 
+## Cookbook: Sweeping config with `configMatrix`
+
+Runs the same `.slt` once per combination of config values. Each directive is 
a comment:
+
+```text
+# configMatrix: <key>=<v1>,<v2>[,...]
+```
+
+- Repeat the directive to nest keys. Values are the cartesian product.
+- Whitespace-trimmed and deduped; repeated keys merge value lists.
+- Unknown key or invalid value fails fast, naming the file, key, and value.
+- Test failures include `[configMatrix: k=v, ...]` in the `N errors in file …` 
banner.

Review Comment:
   Small documentation nit: this says the matrix tag is included in the `N 
errors in file ...` banner, but `run_test_file_once` currently appends the tag 
to the fully formatted error after the per-record errors.
   
   Could we either move the tag into that banner or adjust the wording here to 
describe where it actually appears?



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