check and streamline parameter passing/override, fixes #8084 - #8101
Merged
Conversation
Contributor
Author
|
@dsanderbi , I think this will cover everything |
mattcasters
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8084.
The bug
A pipeline that declares its own named parameter with a default could have that parameter
silently overwritten by an unrelated variable of the same name that happened to exist in the
calling workflow's scope — even with Pass parameter values to sub pipeline unchecked and no
entry for it on the Parameters tab. Two independently written pipelines that both use a common
name (
my-param,input_file,date) could contaminate each other.Root cause
NamedParameters.activateParametersgained "prefer an already-set variable over the parameter'sdefault" in #7517, narrowed to non-empty defaults in #7521. Neither change was about parameter
semantics: both were patching the single-JVM integration-test runner, where a nested
main-*.hwfdeclaring
HOSTNAMEwith defaultlocalhoststopped picking up the project environment'sHOSTNAME. The workaround went into core, so it changed parameter precedence for every pipelineand workflow in the product.
Two further band-aids had grown on top of it:
PipelineExecutorcleared "sticky" values beforeactivation, and
ActionWorkflowcarried a comment pointing at the core hack. Both are removed here.The original integration-test problem is fixed properly rather than worked around — see
Behaviour changes below. No changes were needed in
integration-tests/scripts/.One contract, six executors
Everything that starts a child pipeline or workflow now resolves parameters through one shared
implementation,
org.apache.hop.core.parameters.SubExecutionParameters:parameter or variable with that name.
A row that names a parameter the child does not declare is declared on the child and set. A row
that configures nothing — no value, no stream field — is treated as if it were not there, so the
option and the child's default decide. A row whose stream field is empty on this row stays
authoritative and falls back to the child's default, which is what stops a value from a previous
executor iteration sticking.
TransformWithMappingMeta.activateParamsis kept and delegates, so plugins calling it keep working.// subParams.activateParameters(); TODOand a dead inheritance loopNeither control has ever governed variable inheritance — the engine factories call
initializeFrom(parent)unconditionally either way — so the six differently worded labels werereplaced by one shared
System.Parameters.PassParentValues.Label:"Pass parent values to matching parameters", with a tooltip stating that variables are
inherited regardless. The six obsolete keys are removed from all locale files so stale
translations cannot resurface.
PipelineExecutorMeta.setDefault()now sets the option on for newly created transforms, matchingevery other executor and its own documentation.
Behaviour changes
Measured against a rebuilt 2.18-semantics client, not against the 2.19 bug.
vs 2.19.0 — the fix itself. A declared parameter with a non-empty default no longer adopts an
unrelated same-named variable when the option is off. The option is on by default everywhere, so
most existing solutions are unaffected.
vs 2.18 — three deliberate changes, all in the direction of the option meaning something:
parameters; it now also receives a same-named variable, exactly as sub-pipelines always
have. IT
main-0004'sTEST4_PARAMETER3assertion is updated accordingly — this is the oneassertion changed on purpose, and it is why issue #7507 : restore empty-parameter defaults after single-JVM runner fix #7521 had to revert half of issue #7516 : fix integration tests broken by single-JVM runner #7517.
inherit_all_vars=Y, so theirchildren will now be seeded.
caller's variable whether the box was ticked or not. Untick it now and the child keeps its
default — which is exactly what [Bug]: Regression (2.18 → 2.19): "Pass parameter values to sub pipeline" no longer isolates parameters #8084 asks for.
(1) and (2) mean more values reach sub-workflows; (3) means fewer, and only when someone has
deliberately unticked the box.
Documentation
New reference page Passing parameters to a child pipeline or workflow
(
variables/parameter-passing.adoc), linked fromvariables.adoc,fields-parameters-variables.adoc,nav.adocand all seven executor pages. It states the rules,what counts as "supplies a value", the full outcome table, and the cases that surprise people.
Two pre-existing documentation errors are corrected:
fields-parameters-variables.adocadvised giving a child "a non-empty default so an already-setvariable of the same name is kept". That sentence was written a month after issue #7507 : restore empty-parameter defaults after single-JVM runner fix #7521 and described
the regression, not the contract.
has always meant "nothing passed", so the child's default applies.
The executor pages no longer claim the checkbox controls variable inheritance.