Skip to content

[Improvement-18714][Task-Sql] Avoid redundant serialization in alert content limiting and log result rows incrementally in query result processing in SqlTask - #18717

Open
njnu-seafish wants to merge 2 commits into
apache:devfrom
njnu-seafish:Improvement-18714
Open

njnu-seafish wants to merge 2 commits into
apache:devfrom
njnu-seafish:Improvement-18714

Conversation

@njnu-seafish

@njnu-seafish njnu-seafish commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

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

SbloodyS

This comment was marked as off-topic.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement][Task-Sql] Avoid redundant serialization in alert content limiting and log result rows incrementally in query result processing in SqlTask

2 participants