Repository navigation
fix(admin): order Oracle operation logs before applying the row limit (#7739) - #7758
Merged
Merged
Conversation
Aias00
approved these changes
Oct 17, 2026
Aias00
left a comment
Aias00
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Verified the claim, the rewrite and the surrounding statements.
Evidence
- The bug is real in the current statement (
operation-record-log-sqlmap.xml:58-67):where rownum <= #{limit}sits in the same query block asorder by operation_time desc. Oracle assigns ROWNUM as rows are produced, so the row limit is applied to the unsorted scan and the ordering happens afterwards on those N rows — i.e. "the N oldest rows, then sorted", which is exactly the reported symptom. - The generic statement (
:45-56) already orders and thenlimits, so this change makes Oracle consistent with the existing contract instead of introducing new semantics. SELECT * FROM (SELECT ... ORDER BY ...) WHERE ROWNUM <= ?is Oracle's documented top-N form and needs no derived-table alias; the innerBase_Column_Listkeeps the result map intact throughSELECT *.- Scope is clean:
ROWNUM <=appears exactly once in the wholeshenyu-adminmapper set (the other 40 Oracle occurrences areROWNUM = 1single-row existence lookups, where ordering is irrelevant), so there is no second statement left with the same defect. <where>with a single optional<if>renders nothing whenusernameis null, which matches the previous optional predicate.
Non-blocking
parameterType="java.lang.Integer"while the statement binds#{username}is pre-existing and only works because the mapper method passes aMapthrough@Param. Since you are in the file, a comment (or the accurateparameterType) would stop the next reader from "fixing" it into a real bug. The same applies to the statement you copied the shape from.- The test pins statement shape, which is the reasonable unit-level assertion here. If an Oracle container ever lands in CI, an insert-three-take-two assertion is the only thing that would also catch a semantic regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7739.
Summary
The Oracle
selectLimitstatement limits rows withWHERE ROWNUM <= #{limit}in the same query block asORDER BY operation_time desc. Oracle may apply the row limit before sorting, so the recent-operation list can return older rows and omit the newest N; the generic statement orders first and then appliesLIMIT.Modifications
WHERE ROWNUM <= #{limit}in the outer query (Oracle's documented top-N form).databaseId="oracle".Verifying this change
New
OperationRecordLogSelectLimitSqlTest: underdatabaseId="oracle", asserts the SQL starts with the wrapped subquery,ORDER BYprecedesROWNUM, andROWNUM <= ?plus the operator filter are present. Fails on the previous statement (which showsrownumbeforeorder by), passes with the fix.Targeted test passes:
./mvnw -B -ntp -pl shenyu-admin test -Dtest=OperationRecordLogSelectLimitSqlTest1/1 green.
Checkstyle passes:
clean.
Notes
The configured limit and the per-operator filter are unchanged; only the evaluation order (filter/sort, then row limit) is corrected. The test pins MyBatis dialect selection locally; execution against a real Oracle instance is exercised by the repository's SQL matrix. This PR touches the same mapper file as the separate Oracle
CONCATfix, but a different statement.