[PR] [Improvement-18714][Task-Sql] Log display rows incrementally while reading the query result set [dolphinscheduler]

["wank125 (via GitHub)" <gi...@apache.org>]

wank125 opened a new pull request, #18719:
URL: https://github.com/apache/dolphinscheduler/pull/18719

   ## Was this PR generated or assisted by AI?
   
   YES. The code change, tests and this description were drafted with an AI coding agent under my direction, and I reviewed every line.
   
   ## Purpose of the pull request
   
   Close #18714 (scoped to the log-timing part, per the maintainer feedback in the issue).
   
   In `SqlTask#resultProcess`, the `displayRows` rows are currently logged only after the whole result set has been buffered into `resultJSONArray`, in a second traversal over the array. For a long-running query with a large result set, no row output appears in the worker log until the entire result has been fetched.
   
   As discussed in the issue, this PR deliberately keeps the change to the log-timing improvement only: log rows inline as each row is read and drop the second traversal. The `limitAlertContent` serialization optimization is intentionally NOT included, to keep the implementation simple.
   
   ## Brief change log
   
   - `dolphinscheduler-task-sql`: `SqlTask#resultProcess` now logs each of the first `displayRows` rows inline while iterating the `ResultSet`. The header message now states the configured cap ("up to N rows") because the total row count is unknown before the result set is fully read.
   - The task return value and alert payload behavior is unchanged: the full result set is still buffered and returned, and the per-row log format ("row N : {...}") is unchanged.
   - `dolphinscheduler-task-sql` pom: exclude `slf4j-reload4j` (transitively pulled via `hadoop-client`) and declare `logback-classic` in test scope, so the module has a single deterministic SLF4J binding for the new log assertions.
   
   ## Verify this pull request
   
   This change added tests and can be verified as follows:
   
   - Added `SqlTaskTest#testResultProcess_LogsDisplayRowsWhileReadingResultSet`: captures `SqlTask` logs with a Logback `ListAppender` and asserts all rows are already logged at the moment the `ResultSet` is exhausted (i.e. logging is inline while reading). This test fails on the previous implementation (0 rows logged at exhaustion).
   - Added `SqlTaskTest#testResultProcess_DisplayRowsLimit_LogsOnlyFirstRows`: with `displayRows=2` and a 5-row result set, only rows 1-2 are logged while the returned result still contains all 5 rows.
   - Manually verified the change by running `mvn -pl dolphinscheduler-task-plugin/dolphinscheduler-task-sql -am test` locally (JDK 8): all 21 tests of the module pass, and `spotless:check` passes.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Improvement-18714][Task-Sql] Log display rows incrementally while reading the query result set [dolphinscheduler]

["SbloodyS (via GitHub)" <gi...@apache.org>]

SbloodyS closed pull request #18719: [Improvement-18714][Task-Sql] Log display rows incrementally while reading the query result set
URL: https://github.com/apache/dolphinscheduler/pull/18719


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org