gianm commented on a change in pull request #8566: Upgrade Calcite to 1.21
URL: https://github.com/apache/incubator-druid/pull/8566#discussion_r327734512
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -757,16 +761,16 @@ public void
testSelectStarFromSelectSingleColumnWithLimitDescending() throws Exc
newScanQueryBuilder()
.dataSource(CalciteTests.DATASOURCE1)
.intervals(querySegmentSpec(Filtration.eternity()))
- .columns(ImmutableList.of("__time", "dim1"))
+ .columns(ImmutableList.of("dim1"))
.limit(2)
- .order(ScanQuery.Order.DESCENDING)
+ .order(ScanQuery.Order.NONE)
Review comment:
Technically correct (the standard allows it) but now this test misses the
point of what it was originally trying to validate (that the wrapping technique
used by the web console works).
I suggest keeping this test, but adding a comment like the one you have in
`testSelectProjectionFromSelectSingleColumnDescending`. And then adding another
test that uses the inner query here, plus the new `sqlOuterLimit`
functionality, to verify that the Druid query generated in _that_ case is a
good one.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
