Allow testing records with sibling whitespace in SLT tests and add more string tests #13197
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
findepi
force-pushed
the
findepi/add-empty-string-to-string-test-data-5c1dbd
branch
2 times, most recently
from
7d41bee
to
e64d6d0
Compare
findepi
changed the title
findepi
changed the title
findepi
force-pushed
the
findepi/add-empty-string-to-string-test-data-5c1dbd
branch
2 times, most recently
from
d4ea9ef
to
1a3ac85
Compare
findepi
force-pushed
the
findepi/add-empty-string-to-string-test-data-5c1dbd
branch
from
1a3ac85
to
fa96eb3
Compare
| .collect::<Vec<_>>(); | ||
| let actual = actual | ||
| .iter() | ||
| .map(|strs| strs.iter().join(" ")) |
There was a problem hiding this comment.
we prob can do this in a sinlge map?
There was a problem hiding this comment.
I am not quite sure what you are suggesting here
There was a problem hiding this comment.
I'm suggesting to have a single collection iteration instead of two
.map(|strs| strs.iter().join(" ").trim_end().to_owned())
There was a problem hiding this comment.
yes, we can do this in single map.
i placed this into two map() steps so that the common part of actual and expected processing looks the same and so that i can write a nice comment
| .collect::<Vec<_>>(); | ||
| let actual = actual | ||
| .iter() | ||
| .map(|strs| strs.iter().join(" ")) |
There was a problem hiding this comment.
I am not quite sure what you are suggesting here
| let expected = expected | ||
| .iter() | ||
| // Trailing whitespace from lines in SLT will typically be removed, but do not fail if it is not | ||
| // If particular test wants to cover trailing whitespace on a value, |
There was a problem hiding this comment.
This is probably a good thing to add in the user documentation (aka https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/README.md)
I think it would be hard to find here for someone writing sqllogictests
There was a problem hiding this comment.
it feels so edge-case'y. but yes, we can add
There was a problem hiding this comment.
- Follow on PR for documentation: Minor: Document how to test for trailing whitespace in
slt/ sqllogictests #13215
@comphead please see https://github.com/apache/datafusion/pull/13197/commits |
findepi
deleted the
findepi/add-empty-string-to-string-test-data-5c1dbd
branch
EXPR LIKE 'constant'toexpr = 'constant'#13061 (comment)