FINERACT-2622: Preserve top-level ORDER BY when wrapping report SQL

[heissenberg06]

Description

Fixes FINERACT-2622.

Problem

Report SQL is wrapped as select x.* from (<sql>) x in GenericDataServiceImpl#wrapSQL. A derived table's ORDER BY is not guaranteed to be honored by the outer query, so a user report ending in e.g. ORDER BY account_no DESC returns rows in an arbitrary order.

Fix

Detect a top-level trailing ORDER BY (with any trailing LIMIT/OFFSET) and re-apply it on the outer wrapper, where it resolves against the columns exposed by x.*. The wrapping itself is kept (the CachedRowSetImpl column-label workaround).

The scan ignores ORDER BY that appears inside parentheses (subqueries / window functions), string literals, and SQL comments. As a safeguard against regressions, lifting is skipped when the trailing clause contains a dot-qualified reference (alias.column) that would not resolve in the outer scope, leaving today's behavior for that case.

Scope check: wrapSQL has three call sites in ReadReportingServiceImpl — buildPreparedQuery (user report SQL, the target of this fix) and getSql / getReportType (fixed internal queries with no ORDER BY, so their behavior is unchanged).

Tests

Unit tests on wrapSQL cover: ORDER BY preserved, no-ORDER-BY unchanged, subquery ORDER BY not lifted, window-function ORDER BY not lifted, dot-qualified skipped, string-literal ORDER BY ignored, trailing LIMIT carried.

Note on pagination

When pagination is enabled (IS_PAGINATION_ALLOWED=true), the wrapped SQL is passed to paginationHelper.fetchPage. If a user report SQL ends with a top-level ORDER BY ... LIMIT, the carried LIMIT could interact with pagination's own limiting. Flagging for reviewer confirmation on whether PaginationHelper appends or re-wraps; happy to skip LIMIT-carrying on that path if preferred. Also happy to add a DB-backed integration test if desired.

---

Checklist

• Write the commit message as per our guidelines
• Acknowledge that we will not review PRs that are not passing the build ("green") — it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
• Create/update unit or integration tests for verifying the changes made.
• Follow our coding conventions.
• Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
• This PR must not be a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)

[heissenberg06]

The failing run-sonarqube and publish-dockerhub checks are due to
fork PRs not having access to the SonarQube/DockerHub credentials
(unauthorized / missing server URL), not related to this change. The
unit tests, integration tests, and regression-safety checks pass.

[adamsaghy]

I wonder whether instead of this, we should revisit whether the org.apache.fineract.infrastructure.dataqueries.service.GenericDataServiceImpl#wrapSQL is still needed:

• is the mentioned bug still a problem?
• is there a better way to "prevent JDBC sql errors"?
• is there a better way to "prevent malicious sql"?

[heissenberg06]

Thanks @adamsaghy , that's a good challenge — I dug into the three justifications in the wrapSQL comment:

1. "prevent malicious sql": this no longer holds. The wrapping copies the inner SQL verbatim into the derived table, so it provides no injection protection by itself. Injection is handled separately by SQLInjectionValidator / the parameterized-query work in FINERACT-2081 and the column validators, not by wrapSQL.

2. "CachedRowSetImpl column-label bug" (Sun bug 7046875): the code reaches the result set via JdbcTemplate.queryForRowSet → SqlRowSetResultSetExtractor, which on JDK 21 obtains the CachedRowSet via javax.sql.rowset.RowSetProvider rather than the old com.sun.rowset.CachedRowSetImpl directly. So the original bug likely no longer applies, though this needs to be confirmed with a test (a query selecting AS-aliased columns returning correct labels without wrapping).

3. "prevent JDBC sql errors": there's no concrete reference for this in the comment; I'd want to find what concretely breaks (if anything) when the wrapping is removed.

So I agree it's worth testing whether wrapSQL can simply be dropped. If it can, that removes the root cause and this ORDER BY issue disappears without the SQL-parsing logic in this PR.

Proposed plan: I'll run the reporting tests with wrapSQL turned into a no-op to see what (if anything) fails — column-label handling and any dialect-specific behavior in particular. If it's clean, I'll repurpose this PR to remove the wrapping (and simplify the call sites) instead of adding the ORDER BY lifter. Does that direction work for you? Any specific cases (DB dialects, label edge cases) you'd want covered before removing it?