FrankChen021 commented on code in PR #19882:
URL: https://github.com/apache/druid/pull/19882#discussion_r3720541702
##########
processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerFactoryTest.java:
##########
@@ -351,30 +345,30 @@ public void testMergeRunnersGuardrailsExceeded()
.collect(Collectors.toList())
);
- expectedException.expect(ResourceLimitExceededException.class);
- expectedException.expectMessage(
- "Time ordering is not supported for a Scan query with 5 segments per
time chunk and a row limit of 10,001. "
- + "Try reducing your query limit below maxRowsQueuedForOrdering
(currently 10,000), or using compaction to "
- + "reduce the number of segments per time chunk, or raising
maxSegmentPartitionsOrderedInMemory "
- + "(currently 4) above the number of segments you have per time
chunk."
- );
-
- runner.run(
- QueryPlus.wrap(
- Druids.newScanQueryBuilder()
- .dataSource("foo")
- .limit(CONFIG.getMaxRowsQueuedForOrdering() + 1)
- .intervals(
- new MultipleSpecificSegmentSpec(
- IntStream.range(0,
CONFIG.getMaxSegmentPartitionsOrderedInMemory() + 1)
- .mapToObj(i -> new
SegmentDescriptor(Intervals.ETERNITY, "v0", i))
- .collect(Collectors.toList())
+ ResourceLimitExceededException ex = Assertions.assertThrows(
+ ResourceLimitExceededException.class,
+ () -> runner.run(
+ QueryPlus.wrap(
+ Druids.newScanQueryBuilder()
+ .dataSource("foo")
+ .limit(CONFIG.getMaxRowsQueuedForOrdering() + 1)
+ .intervals(
+ new MultipleSpecificSegmentSpec(
+ IntStream.range(0,
CONFIG.getMaxSegmentPartitionsOrderedInMemory() + 1)
+ .mapToObj(i -> new
SegmentDescriptor(Intervals.ETERNITY, "v0", i))
+ .collect(Collectors.toList())
+ )
)
- )
- .order(Order.ASCENDING)
- .build()
- ),
- ResponseContext.createEmpty()
+ .order(Order.ASCENDING)
+ .build()
+ ),
+ ResponseContext.createEmpty()
+ )
+ );
+ Assertions.assertTrue(
+ ex.getMessage().contains(
+ "Time ordering is not supported for a Scan query with 5 segments
per time chunk and a row limit of 10,001."
Review Comment:
[P3] Preserve full guardrail message coverage
Before this migration, `ExpectedException.expectMessage` checked the
complete user-facing remediation message, including both configuration limits.
The replacement checks only the first sentence, so regressions that remove or
corrupt the actionable guidance now pass. Please retain the original
full-string assertion, or separately assert the remaining guidance and values
`10,000` and `4`.
--
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]
