[njnu-seafish]

Was this PR generated or assisted by AI?

YES.

Purpose of the pull request

close #18714 .

Improve the SQL task's query processing in two aspects:
emit the display-row logs while the result set is being fetched instead of after the whole result is buffered,
and build the size-limited query-result alert without re-serializing the JSON on every dropped row.

Brief change log

1. Log result rows incrementally while fetching (SqlTask.resultProcess)

   • Each displayed row is logged as it is read from the result set (display the first {N} rows of sql result as follows: header, per-row row {k} : ... lines, and a trailing displayed {X} of {Y} rows summary), replacing the previous behavior where all rows were buffered first and then logged in a second pass. This gives real-time visibility for long-running / high-volume queries and removes the extra traversal of the result array.
   • The full result array is still retained as the task return value and the alert payload; only the log output is limited to displayRows.

2. Avoid redundant serialization when limiting the alert content (SqlTask.limitAlertContent)

   • Previously the kept rows were re-serialized and re-encoded to a UTF-8 byte array on every loop iteration while dropping tail rows to fit the byte budget. Now each row is serialized and measured once, the budget is enforced with running byte
     arithmetic, and the final content is assembled once. The cost drops from O(dropped-rows × rows) to O(rows), with the serialized output unchanged (still a valid JSON array respecting the configured max-rows / value-length / content-bytes budgets).

3. Test plumbing (dolphinscheduler-task-sql/pom.xml)

   • Excluded slf4j-reload4j from the module's dolphinscheduler-datasource-api declaration so logback stays the only SLF4J binding on the test classpath, making the new log-output assertions capturable through the logback API. Scoped to this module only; no runtime impact (provided scope). See the dedicated section below.

Verify this pull request

add unit test cases.

• SqlTaskTest: 38 tests pass, including new contract tests for the incremental log output and exact byte-boundary of the alert content (mutation-verified).
• The task result value and the alert content are unchanged in behavior.

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

[njnu-seafish]

@SbloodyS Thanks for the feedback, agreed — the byte arithmetic effectively duplicated the assembly logic, and readers had to keep both in sync to trust it. That's real complexity for a rarely-hit path.

Simplified in 5d78eab8: dropped the serializedContentSize formula and the running rowsBytes accounting. The budget check now simply assembles the candidate content via buildAlertContent and measures its UTF-8 bytes, dropping tail rows until it fits — the measured string is exactly what gets returned, so there is no separate size formula left to maintain.

The core improvement is kept: each row is still serialized only once (which is what #18714 is about); the retry loop just re-joins already-serialized strings, so it stays well ahead of dev, where every dropped row re-serialized the whole array (measured 1.1–5.5× faster across plain / wide / CJK / overflow scenarios).

Tests pass unchanged (38/38), including the exact byte-boundary cases — the output stays byte-for-byte identical.

[SbloodyS]

Each candidate row is now serialized once, but every truncation iteration still rebuilds the remaining JSON content and UTF-8 encodes it to check the byte budget. The overall worst-case complexity therefore remains quadratic in the number of candidate rows, rather than O(rows).

[njnu-seafish]

Each candidate row is now serialized once, but every truncation iteration still rebuilds the remaining JSON content and UTF-8 encodes it to check the byte budget. The overall worst-case complexity therefore remains quadratic in the number of candidate rows, rather than O(rows).

You're right — the assemble-and-measure version in d0e4c23 was O(rows²) in the worst case, since every dropped row rebuilt and re-encoded the remaining content just to check its byte length.

Addressed in a78f26a: the byte budget is now enforced with running byte arithmetic. Each row is still serialized only once; the drop loop additionally tracks the cumulative UTF-8 size, subtracting one row's bytes per drop (O(1)/row — the omission marker stays a tiny ~30-byte string), and the final content is assembled only once it fits. Overall complexity is back to O(rows): one serialization per row plus one final assembly.

Output is unchanged — all 38 tests pass, including the exact byte-boundary and UTF-8 byte-counting cases, so the alert content stays byte-for-byte identical.

[SbloodyS]

LGTM

[ruanwenjun]

LGTM