[Improvement-18443][API&DAO] Optimize WorkflowInstanceMapper to exclude large text fields from list queries - #18444
[Improvement-18443][API&DAO] Optimize WorkflowInstanceMapper to exclude large text fields from list queries#18444njnu-seafish wants to merge 8 commits into
Conversation
@SbloodyS This error seems to be a frontend issue during the E2E test and is likely unrelated to my changes. Could you please help re-trigger the CI? Thanks a lot!
|
|
CI has been reruned. @njnu-seafish |
SbloodyS
left a comment
There was a problem hiding this comment.
public API response contract is unintentionally changed
queryWorkflowInstanceListPaging, queryTopNWorkflowInstance, and queryByTriggerCode now use listSql, but their results are returned directly by public API endpoints as WorkflowInstance objects.
Consequently, these previously populated response properties will become null or be omitted:
commandParamglobalParamshistoryCmdvarPoolstateHistory
This is therefore not only an internal DAO optimization—it changes the existing API response contract and may break API consumers.
Please either:
- Keep using
baseSqlfor queries whose entities are returned directly by public APIs; or - Introduce an explicit lightweight response DTO, document the incompatible API change, and add controller-level response regression tests.
The critical runtime queries reverted to baseSql look correct, but this public API compatibility issue remains blocking.
Exactly. The reviewer's points are incredibly thorough. I will go with Option 2. This allows us to retain the performance optimizations of listSql while introducing explicit, lightweight DTOs to lock down the API contract. We will no longer let entity fields implicitly dictate the API response. |
…ntentionally omit commandParam/globalParams/historyCmd/varPool/stateHistory
|
@SbloodyS Could you please help trigger the CI pipeline and review the code? Thanks! |
| instance.getHost(), | ||
| instance.getCommandType(), | ||
| instance.getTaskDependType(), | ||
| instance.getMaxTryTimes(), |
| instance.getTimeout(), | ||
| instance.getTenantCode(), | ||
| instance.getDryRun(), | ||
| instance.getNextWorkflowInstanceId(), |
| String executorName, | ||
| WorkflowExecutionStatus stateType, | ||
| String host, | ||
| String otherParamsJson, |



Was this PR generated or assisted by AI?
YES. Leverage LLMs to verify that the removed large text fields are indeed unused.
Purpose of the pull request
close #18443
Brief change log
The WorkflowInstanceMapper was returning all fields including large text/longtext fields (command_param, global_params, history_cmd, var_pool, state_history) for all queries,which caused unnecessary database I/O, network transfer, and memory usage. This commit adds a dedicated listSql SQL fragment that excludes these large fields, and updates the following list query methods to use them:
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
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