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