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

Fix SQL visibility root workflow fields in schema #6067

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

rodrigozhou
Copy link
Contributor

@rodrigozhou rodrigozhou commented Jun 5, 2024

What changed?

Fix SQL visibility root workflow fields in schema: they are supposed to be not null.

Why?

The schema set the root workflow fields as nullable, but the code was assuming to be not null.
GH Issue: #6050

How did you test it?

Start Temporal v1.23.1, create some workflows, update the schemas in v1.24.0, upgrade Temporal v1.24.0:

  • reproduced the error
  • applied this fix, and the error was gone.

Potential risks

Documentation

Is hotfix candidate?

@rodrigozhou rodrigozhou requested a review from a team as a code owner June 5, 2024 15:34
@rodrigozhou rodrigozhou force-pushed the rodrigozhou/fix-vis-schema-root-wf branch from 2caa9ba to a1d21aa Compare June 5, 2024 16:03
@rodrigozhou rodrigozhou merged commit 00e3981 into main Jun 5, 2024
42 checks passed
@rodrigozhou rodrigozhou deleted the rodrigozhou/fix-vis-schema-root-wf branch June 5, 2024 18:41
stephanos pushed a commit that referenced this pull request Jun 5, 2024
## What changed?
<!-- Describe what has changed in this PR -->
Fix SQL visibility root workflow fields in schema: they are supposed to
be not null.

## Why?
<!-- Tell your future self why have you made these changes -->
The schema set the root workflow fields as nullable, but the code was
assuming to be not null.
GH Issue: #6050

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Start Temporal v1.23.1, create some workflows, update the schemas in
v1.24.0, upgrade Temporal v1.24.0:
- reproduced the error
- applied this fix, and the error was gone.

## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->

## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->

## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants