alamb commented on code in PR #15152: URL: https://github.com/apache/datafusion/pull/15152#discussion_r1992230488
########## datafusion/sqllogictest/test_files/aggregates_topk.slt: ########## @@ -19,6 +19,12 @@ # Setup test data table ####### +# Setting to map varchar to utf8view, to test PR https://github.com/apache/datafusion/pull/15152 +# Before the PR, the test case would not work because the Utf8View will not be supported by the TopK aggregation Review Comment: this change now loses coverage for utf8 Instead perhaps we can make a table with stringview? Something like ```sql CREATE TABLE traces_utf8view AS SELECT arrow_cast(trace_id, 'Utf8View') as trace_id, timestamp, other ) ``` And then do an explain ```sql explain select trace_id, MAX(timestamp) from traces group by trace_id order by MAX(timestamp) desc limit 4; ``` ########## datafusion/sqllogictest/test_files/aggregates_topk.slt: ########## @@ -19,6 +19,12 @@ # Setup test data table ####### +# Setting to map varchar to utf8view, to test PR https://github.com/apache/datafusion/pull/15152 +# Before the PR, the test case would not work because the Utf8View will not be supported by the TopK aggregation Review Comment: Also it would be great to do the same fir `LargeUtf8` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
