jayzhan211 opened a new issue, #27239:
URL: https://github.com/apache/datafusion/issues/27239
### Is your feature request related to a problem or challenge?
326 error checks in 65 `.slt` files put the full error message on the
`statement error` / `query error` line as an escaped regex, which is hard to
read and review:
```text
statement error DataFusion error: This feature is not implemented: WITH
\[NO\] DATA not supported
CREATE TABLE foo AS SELECT 1 WITH NO DATA;
```
As plain text after `----`, the same check is exact and readable:
```text
statement error
CREATE TABLE foo AS SELECT 1 WITH NO DATA;
----
DataFusion error: This feature is not implemented: WITH [NO] DATA not
supported
```
#27217 documents this form in the sqllogictest README, and #27218 asks
`--complete` to write it for new records. Neither changes existing lines,
because `--complete` leaves a regex that still matches alone. This issue is
about converting the existing lines.
### Describe the solution you'd like
For each line listed by this command:
```shell
git grep -nE '^(statement|query) error DataFusion error:
.*\\[][(){}.+*?^$|]' -- 'datafusion/sqllogictest/test_files/*.slt'
```
1. Remove the message from the `statement error` / `query error` line, and
add `----` and two blank lines after the SQL. With only one blank line, the
next record becomes part of the expected error and `--complete` deletes it.
2. Run `cargo test --test sqllogictests -- <file> --complete`.
3. Keep the new block only if its message equals the old regex with the
escapes removed. If it differs, the regex matched part of the message on
purpose, so restore the old line. For example, `ddl.slt:704` uses `.*` for a
local path.
4. Check `git diff`: only the converted records should change. Start from a
file that passes, because `--complete` also rewrites any other failing record.
Several small PRs (about 5 to 10 files each) are easier to review and
conflict less: 36 open PRs touch 21 of these files today. Please comment here
with the files you're taking.
The 75 other escaped lines don't start with `DataFusion error:`, so they
match only part of a message. Leave them as they are.
### Describe alternatives you've considered
- One PR for all 65 files: a harder review, and more conflicts with open PRs.
- Waiting for #27218: it changes only what `--complete` writes for failing
records, so existing lines that pass stay as they are.
### Additional context
Follow-up to #27217.
<details>
<summary>The steps above, tried on <code>ddl.slt</code> (7 lines)</summary>
4 lines converted cleanly, for example:
```diff
-statement error DataFusion error: Execution error: Catalog 'cat' doesn't
exist\.
+statement error
drop catalog cat;
+----
+DataFusion error: Execution error: Catalog 'cat' doesn't exist.
+
```
The 3 `No files found at .*` lines were filled with the local path
(`file:///.../test_files/scratch/ddl/empty_dir/`), so step 3 restores them.
</details>
### Willingness to contribute
I'm not planning to work on this myself, so anyone is welcome to pick it up.
Feel free to ping me with questions or when you'd like a review.
--
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]
