You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/03/24 19:31:51 UTC
[GitHub] [arrow-datafusion] alamb opened a new issue, #5727: sqllogictests `--complete` does not escape `(` in error messages properly
alamb opened a new issue, #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727
### Describe the bug
sqllogictest automatically verifies error messages, including with regular expression matching
read more about sqllogictest here: https://github.com/apache/arrow-datafusion/tree/main/datafusion/core/tests/sqllogictests
Because it supports regular expression matching, to match an error like this (note `(` and `)`):
```
query error Error during planning: arrow_cast requires its second argument to be a constant string, got Int64(43)
```
You need to `\(` and `\)`:
```
query error Error during planning: arrow_cast requires its second argument to be a constant string, got Int64\(43\)
```
`sqllogictests --complete` mode is is awesome and will update the expected output, however for error messages that contain parenthesis it does not escape the parenthesis
This means that output captured with `--complete` that contains errors with `(` and `)` will not pass
### To Reproduce
Apply this diff (
```diff
--- a/datafusion/core/tests/sqllogictests/test_files/arrow_typeof.slt
+++ b/datafusion/core/tests/sqllogictests/test_files/arrow_typeof.slt
@@ -95,7 +95,7 @@ SELECT arrow_cast('1', 'Int16')
query error Error during planning: arrow_cast needs 2 arguments, 1 provided
SELECT arrow_cast('1')
-query error Error during planning: arrow_cast requires its second argument to be a constant string, got Int64\(43\)
+query error TBD
SELECT arrow_cast('1', 43)
query error Error unrecognized word: unknown
```
Then run the sqllogictest complete mode:
```
cargo test --test sqllogictests -- arrow_typeof --complete
```
Then run sqllogictest:
```shell
cargo test --test sqllogictests -- arrow_typeof
```
It fails with a very hard to understand error (the reported expected and actual are the same 🤯 ):
```shell
Error: query is expected to fail with error:
DataFusion error: Error during planning: arrow_cast requires its second argument to be a constant string, got Int64(43)
but got error:
DataFusion error: Error during planning: arrow_cast requires its second argument to be a constant string, got Int64(43)
[SQL] SELECT arrow_cast('1', 43)
at tests/sqllogictests/test_files/arrow_typeof.slt:98
```
### Expected behavior
I expect sqllogictest to pass after running `--complete` mode:
```shell
cargo test --test sqllogictests -- arrow_typeof
```
### Additional context
@tustvold hit this while working on https://github.com/apache/arrow-datafusion/pull/5685
The symptom was the following very non clear message
![Screenshot 2023-03-24 at 3 10 53 PM](https://user-images.githubusercontent.com/490673/227619287-0800509d-f99a-4bb2-bdc9-a024f317941e.png)
![image](https://user-images.githubusercontent.com/490673/227619403-17fbf395-c3bd-4aee-ad78-ed1a1b25e694.png)
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727#issuecomment-1483800172
I just tested locally (after running `cargo update` in datafusion to update to https://crates.io/crates/sqllogictest/0.13.2) and this works great. What a wonderful way to start a Saturady.
🙇 @melgenek and @xxchan
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727#issuecomment-1483313401
cc @melgenek
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb closed issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
URL: https://github.com/apache/arrow-datafusion/issues/5727
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] melgenek commented on issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "melgenek (via GitHub)" <gi...@apache.org>.
melgenek commented on issue #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727#issuecomment-1483339566
The completed error should probably be wrapped in the `regex::escape` [here](https://github.com/risinglightdb/sqllogictest-rs/blob/main/sqllogictest/src/parser.rs#L187) and [here](https://github.com/risinglightdb/sqllogictest-rs/blob/main/sqllogictest/src/parser.rs#L209).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727#issuecomment-1483312877
I strongly suspect this needs to be fixed upstream in https://github.com/risinglightdb/sqllogictest-rs @xxchan does this sound familiar to you at all?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] melgenek commented on issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "melgenek (via GitHub)" <gi...@apache.org>.
melgenek commented on issue #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727#issuecomment-1483410986
I've opened a pr in the sqllogictest-rs https://github.com/risinglightdb/sqllogictest-rs/pull/174.
It does effectively the same that I suggested above, but does the escaping when building a regex, rather than writing it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #5727: sqllogictests `--complete` does not escape `(` in error messages properly
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #5727:
URL: https://github.com/apache/arrow-datafusion/issues/5727#issuecomment-1483799032
> I've opened a pr in the sqllogictest-rs https://github.com/risinglightdb/sqllogictest-rs/pull/174.
Thank you so much @melgenek !
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org