kbendick commented on a change in pull request #2081:
URL: https://github.com/apache/iceberg/pull/2081#discussion_r556385303
##########
File path:
spark3/src/test/java/org/apache/iceberg/spark/source/TestFilteredScan.java
##########
@@ -529,7 +529,7 @@ private Table buildPartitionedTable(String desc,
PartitionSpec spec, String udf,
return Lists.newArrayList(
record(schema, 0L, parse("2017-12-22T09:20:44.294658+00:00"),
"junction"),
record(schema, 1L, parse("2017-12-22T07:15:34.582910+00:00"),
"alligator"),
- record(schema, 2L, parse("2017-12-22T06:02:09.243857+00:00"),
"forrest"),
+ record(schema, 2L, parse("2017-12-22T06:02:09.243857+00:00"), ""),
Review comment:
Also, for reference, I did make several separate tests and they all
passed. But it just seems cleaner to me to introduce more common edge cases
into the existing test cases, especially when it doesn't require us to change
the existing tests. I'm surprised that the tests that run on random data hasn't
caught this before. But most of the random data tests use a predefined seed, so
I'm not that surprised.
That said, the tests in `TestFilteredScan` all go over the code paths that
deal with this. And after we merge this, I can add in two small / inexpensive
tests in the `NOT_STARTS_WITH` pr that will more explicitly cover this edge
case - by partitioning a DF using a filter of `data like '%'` and `data not
like '%'`, which would throw an exception due to the truncation length without
this change (though hopefully there aren't too many queries like the first one,
but it's not my place to tell people how to write queries).
If there is any desire for a more explicit test or anything, please let me
know 🙂
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
