Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle null variable input with nullable=false #30330

Merged
merged 1 commit into from Jan 11, 2022

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Jan 10, 2022

v1.1 targeted fix for #30307. The variable handling has been overhauled in v1.2 already, but we want to head off any possible incorrect use of null variables slipping into validation when nullable=false in the variable configuration. The given test is already fixed in v1.2 by #29959.

Default values are currently being handled in two places, the graph
transformation where a synthetic default expression is created if there
was no input expression, and in the evaluator when a reference to the
variable is evaluated. Unfortunately neither of these covers the case
where a non-nullable variable default needs to be checked within a
validation statement

Rather than try and fix the overall variable handling here, which runs
the risk of encountering more unexpected behavior, we are going to fixup
this one case to ensure a null value doesn't continue to slip into
validation. A more extensive refactoring of variable handling has been
completed in v1.2, and this code will only be relevant for the v1.1
branch.

Fixes #30307

Default values are currently being handled in two places, the graph
transformation where a synthetic default expression is created if there
was no input expression, and in the evaluator when a reference to the
variable is evaluated. Unfortunately neither of these covers the case
where a non-nullable variable default needs to be checked within a
validation statement

Rather than try and fix the overall variable handling here, which runs
the risk of encountering more unexpected behavior, we are going to fixup
this one case to ensure a null value doesn't continue to slip into
validation. A more extensive refactoring of variable handling has been
completed in v1.2, and this code will only be relevant for the v1.1
branch.
@jbardin jbardin requested a review from a team January 10, 2022 21:56
@jbardin jbardin self-assigned this Jan 10, 2022
@apparentlymart apparentlymart added this to the v1.1.4 milestone Jan 11, 2022
@jbardin jbardin merged commit b3e7ec1 into v1.1 Jan 11, 2022
@jbardin jbardin deleted the jbardin/non-nullable-default branch January 11, 2022 15:43
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants