yyqdbngt opened a new pull request, #5343:
URL: https://github.com/apache/rocketmq-dashboard/pull/5343
### Which Issue(s) This PR Fixes
- Fixes #4974
### Brief Description
`MybatisPlusAiEventRepository.findByConversationIdAfterSeq`
(`server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/MybatisPlusAiEventRepository.java:63-72`)
generated `WHERE (conversation_id = ? AND seq > ?) LIMIT n` with no `ORDER BY`
and sorted the bounded slice in Java. SQL does not define *which* qualifying
rows a `LIMIT` without an `ORDER BY` keeps, so the ascending order
`AiConversationService.timeline` assumes — and the `nextAfter` cursor it
derives from the last retained row — rested on row order the statement never
asked for. A Java sort of whatever subset came back cannot repair the choice of
subset.
The fix puts the ordering into the statement (`ORDER BY seq ASC` immediately
before `LIMIT`) and drops the in-memory sort. `uk_ai_event_conversation_seq`
leads with the equality column and orders by `seq`, so the same range scan that
applies the `seq > ?` predicate already visits the qualifying rows in `seq`
order; MySQL satisfies the clause from that index instead of materialising the
MEDIUMTEXT `payload` into `sort_buffer_size`. The javadocs on
`AiEventRepository.findByConversationIdAfterSeq`,
`AiConversationService.timeline` and the events endpoint that documented the
opposite decision now document this contract. Nothing outside the ordering of a
timeline slice changes: the requested row count, the probe row and the cursor
arithmetic are untouched.
Files:
-
`server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/MybatisPlusAiEventRepository.java`
-
`server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/AiEventRepository.java`
(javadoc)
-
`server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/AiConversationService.java`
(javadoc)
-
`server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/AiConversationController.java`
(javadoc)
-
`server/src/test/java/org/apache/rocketmq/studio/ops/ai/conversation/AiTimelineRepositoryTest.java`
### How Did You Test This Change?
`AiTimelineRepositoryTest` is the SQL-shape test of these repositories. Its
slice fixture now carries the ordering contract (`SLICE_SQL = "(conversation_id
= ? AND seq > ?) ORDER BY seq ASC LIMIT "`),
`theSliceShouldNeverSortInTheDatabaseTest` became
`theSliceShouldOrderBySeqBeforeTheLimitTest` (ordering present, and positioned
before the limit), and `theSliceShouldBeSortedInMemoryBySeqThenIdTest` became
`theSliceShouldKeepTheDatabaseOrderTest` (the repository returns the
database-ordered slice unchanged).
Red — that test file against the unmodified repository (`git stash push --
server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/`, so the
tree was upstream plus only the test change):
```
$ cd server && mvn -B -ntp test -Dtest=AiTimelineRepositoryTest
[ERROR] Tests run: 35, Failures: 7, Errors: 0, Skipped: 0, Time elapsed:
3.979 s <<< FAILURE! -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest
[ERROR]
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest.theSliceShouldOrderBySeqBeforeTheLimitTest
-- Time elapsed: 0.006 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
expected: "(conversation_id = ? AND seq > ?) ORDER BY seq ASC LIMIT 201"
but was: "(conversation_id = ? AND seq > ?) LIMIT 201"
at
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest.theSliceShouldOrderBySeqBeforeTheLimitTest(AiTimelineRepositoryTest.java:145)
[ERROR]
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest.theSliceShouldKeepTheDatabaseOrderTest
-- Time elapsed: 0.019 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
Expecting actual:
[901L, 902L]
to contain exactly (and in same order):
[902L, 901L]
but there were differences at these indexes:
- element at index 0: expected "902" but was "901"
- element at index 1: expected "901" but was "902"
at
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest.theSliceShouldKeepTheDatabaseOrderTest(AiTimelineRepositoryTest.java:168)
[ERROR] Tests run: 35, Failures: 7, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE
```
Green — after the fix, same test file, no further test edits:
```
$ cd server && mvn -B -ntp test -Dtest=AiTimelineRepositoryTest
[INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed:
4.826 s -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest
[INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
The other five failures in the red run are the remaining slice-shape
assertions: they share the `SLICE_SQL` fixture, so all seven move with the
contract and all seven are green after the fix. Checkstyle runs at `validate`
in the same command and reported no violations.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits
- [x] Tests added or updated for non-trivial changes
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable: no UI text changed
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks) — the change touches no package boundary; `mvn test
-Dtest=AiTimelineRepositoryTest` ran the checkstyle gate at `validate`
- [x] New source files carry the ASF license header — no new source file
- [x] Documentation touched where behaviour changed (README / `docs/` /
in-app help) — the class and interface javadocs that stated the previous
ordering rule were updated together with the code
--
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]
