From f2bf9763e969de2aea1000552f0e490947d1f5d3 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Thu, 17 Sep 2026 15:37:26 +0200 Subject: [PATCH] Use safe in() lists for id collections in the variable queries The variable queries emitted a single flat in() clause for the id collections, which fails on databases that limit the number of entries in an in() clause (Oracle allows at most 1000). The id collections are now split into chunks of MAX_ENTRIES_IN_CLAUSE and combined with OR, the same way the historic variable query already did it for some of its filters. This covers taskIds, executionIds, scopeIds and subScopeIds on InternalVariableInstanceQuery, taskIds and executionIds on VariableInstanceQuery, and executionIds on HistoricVariableInstanceQuery, which was the last filter in that query still using a flat in() clause. The scopeTypes and names collections are not expected to exceed the limit and are left as they are. While there, the cached entity matcher now ignores empty scopeIds and subScopeIds collections, so it is consistent with the SQL, which skips the filter in that case. --- ...bleInstanceQueryLargeIdCollectionTest.java | 199 ++++++++++++++++++ .../HistoricVariableInstanceQueryImpl.java | 4 + .../InternalVariableInstanceQueryImpl.java | 27 ++- .../impl/VariableInstanceQueryImpl.java | 8 + .../entity/HistoricVariableInstance.xml | 10 +- .../db/mapping/entity/VariableInstance.xml | 64 ++++-- 6 files changed, 287 insertions(+), 25 deletions(-) create mode 100644 modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java new file mode 100644 index 00000000000..bcfa860e388 --- /dev/null +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/test/impl/VariableInstanceQueryLargeIdCollectionTest.java @@ -0,0 +1,199 @@ +/* Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.flowable.engine.test.impl; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; + +import org.flowable.common.engine.api.scope.ScopeTypes; +import org.flowable.common.engine.impl.db.AbstractDataManager; +import org.flowable.engine.impl.test.PluggableFlowableTestCase; +import org.flowable.engine.runtime.ProcessInstance; +import org.flowable.engine.test.Deployment; +import org.flowable.variable.api.history.HistoricVariableInstance; +import org.flowable.variable.api.persistence.entity.VariableInstance; +import org.flowable.variable.service.InternalVariableInstanceQuery; +import org.flowable.variable.service.VariableService; +import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * Tests that the variable queries split id collections which exceed {@link AbstractDataManager#MAX_ENTRIES_IN_CLAUSE} + * into multiple in() clauses, since not every database supports more entries in a single in() clause. + * + * @author Filip Hrisafov + */ +class VariableInstanceQueryLargeIdCollectionTest extends PluggableFlowableTestCase { + + protected final List variableIds = new ArrayList<>(); + + @AfterEach + void tearDown() { + managementService.executeCommand(commandContext -> { + VariableService variableService = processEngineConfiguration.getVariableServiceConfiguration().getVariableService(); + for (String variableId : variableIds) { + VariableInstanceEntity variable = variableService.createInternalVariableInstanceQuery().id(variableId).singleResult(); + if (variable != null) { + variableService.deleteVariableInstance(variable); + } + } + return null; + }); + variableIds.clear(); + } + + @Test + void internalQueryWithLargeTaskIdCollection() { + createVariable("taskVar", variable -> variable.setTaskId("task-1")); + createVariable("otherTaskVar", variable -> variable.setTaskId("task-2")); + createVariable("ignoredTaskVar", variable -> variable.setTaskId("task-3")); + + assertThat(findVariables(query -> query.taskIds(idsWith("task-1", "task-2")))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("taskVar", "otherTaskVar"); + } + + @Test + @Deployment(resources = "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml") + void internalQueryWithLargeExecutionIdCollection() { + // Variables are linked to an execution with a foreign key, so real process instances are needed here + ProcessInstance firstInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("executionVar", "first")); + ProcessInstance secondInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("otherExecutionVar", "second")); + runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("ignoredExecutionVar", "third")); + + assertThat(findVariables(query -> query.executionIds(idsWith(firstInstance.getId(), secondInstance.getId())))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("executionVar", "otherExecutionVar"); + } + + @Test + void internalQueryWithLargeScopeIdCollection() { + createVariable("scopeVar", variable -> { + variable.setScopeId("scope-1"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("otherScopeVar", variable -> { + variable.setScopeId("scope-2"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("ignoredScopeVar", variable -> { + variable.setScopeId("scope-3"); + variable.setScopeType(ScopeTypes.CMMN); + }); + + assertThat(findVariables(query -> query.scopeIds(idsWith("scope-1", "scope-2")).scopeType(ScopeTypes.CMMN))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("scopeVar", "otherScopeVar"); + } + + @Test + void internalQueryWithLargeSubScopeIdCollection() { + createVariable("subScopeVar", variable -> { + variable.setSubScopeId("subScope-1"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("otherSubScopeVar", variable -> { + variable.setSubScopeId("subScope-2"); + variable.setScopeType(ScopeTypes.CMMN); + }); + createVariable("ignoredSubScopeVar", variable -> { + variable.setSubScopeId("subScope-3"); + variable.setScopeType(ScopeTypes.CMMN); + }); + + assertThat(findVariables(query -> query.subScopeIds(idsWith("subScope-1", "subScope-2")).scopeType(ScopeTypes.CMMN))) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("subScopeVar", "otherSubScopeVar"); + } + + @Test + void variableInstanceQueryWithLargeTaskIdCollection() { + createVariable("taskVar", variable -> variable.setTaskId("task-1")); + createVariable("otherTaskVar", variable -> variable.setTaskId("task-2")); + createVariable("ignoredTaskVar", variable -> variable.setTaskId("task-3")); + + assertThat(runtimeService.createVariableInstanceQuery().taskIds(idsWith("task-1", "task-2")).list()) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("taskVar", "otherTaskVar"); + } + + @Test + @Deployment(resources = "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml") + void variableInstanceQueryWithLargeExecutionIdCollection() { + ProcessInstance firstInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("executionVar", "first")); + ProcessInstance secondInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("otherExecutionVar", "second")); + runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("ignoredExecutionVar", "third")); + + assertThat(runtimeService.createVariableInstanceQuery().executionIds(idsWith(firstInstance.getId(), secondInstance.getId())).list()) + .extracting(VariableInstance::getName) + .containsExactlyInAnyOrder("executionVar", "otherExecutionVar"); + } + + @Test + @Deployment(resources = "org/flowable/engine/test/api/oneTaskProcess.bpmn20.xml") + void historicVariableInstanceQueryWithLargeExecutionIdCollection() { + ProcessInstance firstInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("executionVar", "first")); + ProcessInstance secondInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("otherExecutionVar", "second")); + runtimeService.startProcessInstanceByKey("oneTaskProcess", Map.of("ignoredExecutionVar", "third")); + + assertThat(historyService.createHistoricVariableInstanceQuery().executionIds(idsWith(firstInstance.getId(), secondInstance.getId())).list()) + .extracting(HistoricVariableInstance::getVariableName) + .containsExactlyInAnyOrder("executionVar", "otherExecutionVar"); + } + + protected List findVariables(Consumer queryCustomizer) { + return managementService.executeCommand(commandContext -> { + InternalVariableInstanceQuery query = processEngineConfiguration.getVariableServiceConfiguration() + .getVariableService() + .createInternalVariableInstanceQuery(); + queryCustomizer.accept(query); + return query.list(); + }); + } + + protected void createVariable(String name, Consumer variableCustomizer) { + String variableId = managementService.executeCommand(commandContext -> { + VariableService variableService = processEngineConfiguration.getVariableServiceConfiguration().getVariableService(); + VariableInstanceEntity variable = variableService.createVariableInstance(name); + variableCustomizer.accept(variable); + variableService.insertVariableInstanceWithValue(variable, name + "-value", null); + return variable.getId(); + }); + variableIds.add(variableId); + } + + /** + * Returns a set which contains the given ids, padded with dummy ids so the total exceeds the maximum number of entries in a single in() clause. + */ + protected Set idsWith(String... ids) { + Set allIds = new LinkedHashSet<>(List.of(ids)); + allIds.addAll(generateIds("dummy", AbstractDataManager.MAX_ENTRIES_IN_CLAUSE + 500)); + return allIds; + } + + protected Collection generateIds(String prefix, int amount) { + List ids = new ArrayList<>(amount); + for (int i = 0; i < amount; i++) { + ids.add(prefix + "-generated-" + i); + } + return ids; + } +} diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java index d85ec647dcb..3621376c951 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/HistoricVariableInstanceQueryImpl.java @@ -422,6 +422,10 @@ public Set getExecutionIds() { return executionIds; } + public List> getSafeExecutionIds() { + return getSafeList(executionIds); + } + public boolean isExcludeTaskRelated() { return excludeTaskRelated; } diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java index 4e07a42e71c..14b967524cd 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/InternalVariableInstanceQueryImpl.java @@ -17,9 +17,11 @@ import org.apache.commons.lang3.StringUtils; import org.flowable.common.engine.api.FlowableIllegalArgumentException; +import org.flowable.common.engine.impl.db.AbstractDataManager; import org.flowable.common.engine.impl.db.SingleCachedEntityMatcher; import org.flowable.common.engine.impl.persistence.cache.CachedEntity; import org.flowable.common.engine.impl.persistence.cache.CachedEntityMatcher; +import org.flowable.common.engine.impl.util.CollectionUtil; import org.flowable.variable.service.InternalVariableInstanceQuery; import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity; import org.flowable.variable.service.impl.persistence.entity.data.VariableInstanceDataManager; @@ -225,6 +227,10 @@ public Collection getTaskIds() { return taskIds; } + public List> getSafeTaskIds() { + return getSafeList(taskIds); + } + public String getProcessInstanceId() { return processInstanceId; } @@ -237,6 +243,10 @@ public Collection getExecutionIds() { return executionIds; } + public List> getSafeExecutionIds() { + return getSafeList(executionIds); + } + public boolean isWithoutTaskId() { return withoutTaskId; } @@ -249,6 +259,10 @@ public Collection getScopeIds() { return scopeIds; } + public List> getSafeScopeIds() { + return getSafeList(scopeIds); + } + public String getSubScopeId() { return subScopeId; } @@ -257,6 +271,10 @@ public Collection getSubScopeIds() { return subScopeIds; } + public List> getSafeSubScopeIds() { + return getSafeList(subScopeIds); + } + public boolean isWithoutSubScopeId() { return withoutSubScopeId; } @@ -277,6 +295,11 @@ public Collection getNames() { return names; } + protected List> getSafeList(Collection values) { + // need to split into different parts due to some dbs not supporting more than MAX_ENTRIES_IN_CLAUSE for in() + return CollectionUtil.partition(values, AbstractDataManager.MAX_ENTRIES_IN_CLAUSE); + } + // This method is needed because we have a different way of querying list and single objects via MyBatis. // Querying lists wraps the object in a ListQueryParameterObject public InternalVariableInstanceQueryImpl getParameter() { @@ -304,7 +327,7 @@ public boolean isRetained(VariableInstanceEntity entity, InternalVariableInstanc return false; } - if (param.scopeIds != null && !param.scopeIds.contains(entity.getScopeId())) { + if (param.scopeIds != null && !param.scopeIds.isEmpty() && !param.scopeIds.contains(entity.getScopeId())) { return false; } @@ -324,7 +347,7 @@ public boolean isRetained(VariableInstanceEntity entity, InternalVariableInstanc return false; } - if (param.subScopeIds != null && !param.subScopeIds.contains(entity.getSubScopeId())) { + if (param.subScopeIds != null && !param.subScopeIds.isEmpty() && !param.subScopeIds.contains(entity.getSubScopeId())) { return false; } diff --git a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java index f580f2de1b1..18da8a20bb8 100644 --- a/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java +++ b/modules/flowable-variable-service/src/main/java/org/flowable/variable/service/impl/VariableInstanceQueryImpl.java @@ -372,6 +372,10 @@ public Set getTaskIds() { return taskIds; } + public List> getSafeTaskIds() { + return getSafeList(taskIds); + } + public String getExecutionId() { return executionId; } @@ -380,6 +384,10 @@ public Set getExecutionIds() { return executionIds; } + public List> getSafeExecutionIds() { + return getSafeList(executionIds); + } + public boolean isExcludeTaskRelated() { return excludeTaskRelated; } diff --git a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml index 6b80b43e7ad..de8372787ee 100644 --- a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml +++ b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/HistoricVariableInstance.xml @@ -421,10 +421,14 @@ and RES.EXECUTION_ID_ = #{executionId, jdbcType=NVARCHAR} - and RES.EXECUTION_ID_ in - - #{executionId, jdbcType=NVARCHAR} + and ( + + RES.EXECUTION_ID_ in + + #{executionId, jdbcType=NVARCHAR} + + ) and RES.TASK_ID_ = #{taskId, jdbcType=NVARCHAR} diff --git a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml index 72d6bc3b7f0..9ca9de4b886 100644 --- a/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml +++ b/modules/flowable-variable-service/src/main/resources/org/flowable/variable/service/db/mapping/entity/VariableInstance.xml @@ -196,11 +196,15 @@ AND TASK_ID_ = #{parameter.taskId, jdbcType=NVARCHAR} - - AND TASK_ID_ in - - #{taskId, jdbcType=NVARCHAR} + + AND ( + + TASK_ID_ in + + #{taskId, jdbcType=NVARCHAR} + + ) AND PROC_INST_ID_ = #{parameter.processInstanceId, jdbcType=NVARCHAR} @@ -208,11 +212,15 @@ AND EXECUTION_ID_ = #{parameter.executionId, jdbcType=NVARCHAR} - - AND EXECUTION_ID_ in - - #{executionId, jdbcType=NVARCHAR} + + AND ( + + EXECUTION_ID_ in + + #{executionId, jdbcType=NVARCHAR} + + ) AND TASK_ID_ is null @@ -221,19 +229,27 @@ AND SCOPE_ID_ = #{parameter.scopeId, jdbcType=NVARCHAR} - AND SCOPE_ID_ in - - #{scopeId, jdbcType=NVARCHAR} + AND ( + + SCOPE_ID_ in + + #{scopeId, jdbcType=NVARCHAR} + + ) AND SUB_SCOPE_ID_ = #{parameter.subScopeId, jdbcType=NVARCHAR} - AND SUB_SCOPE_ID_ in - - #{subScopeId, jdbcType=NVARCHAR} + AND ( + + SUB_SCOPE_ID_ in + + #{subScopeId, jdbcType=NVARCHAR} + + ) AND SUB_SCOPE_ID_ is null @@ -287,19 +303,27 @@ and RES.EXECUTION_ID_ = #{executionId, jdbcType=NVARCHAR} - and RES.EXECUTION_ID_ in - - #{executionId, jdbcType=NVARCHAR} + and ( + + RES.EXECUTION_ID_ in + + #{executionId, jdbcType=NVARCHAR} + + ) and RES.TASK_ID_ = #{taskId, jdbcType=NVARCHAR} - and RES.TASK_ID_ in - - #{taskId, jdbcType=NVARCHAR} + and ( + + RES.TASK_ID_ in + + #{taskId, jdbcType=NVARCHAR} + + ) and RES.TASK_ID_ is NULL