You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/08/25 20:59:26 UTC

[GitHub] [arrow-datafusion] seddonm1 commented on pull request #947: Add support for trim variants

seddonm1 commented on pull request #947:
URL: https://github.com/apache/arrow-datafusion/pull/947#issuecomment-905869497


   hi @adsharma . this looks good and I think it is correct.
   
   I am not sure your tests are proving anything additional? This would benefit from tests proving the `trim ( [ LEADING | TRAILING | BOTH ] [ FROM ] string text [, characters text ] )` syntax. Maybe you could remove these tests and add some to `tests/sql.rs` which have ensure the SQL string -> function coverage like `trim(both from 'yxTomxx', 'xyz')`.


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