-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
roachtest: schemachange/mixed-versions failed #70204
Comments
roachtest.schemachange/mixed-versions failed with artifacts on release-21.2 @ d3fc366bcba04ab19f4cb59844212e908e2a9aa8:
Same failure on other branches
|
roachtest.schemachange/mixed-versions failed with artifacts on release-21.2 @ 0babf97f52ed9e44036851b2a9868e17eeee95ed:
Same failure on other branches
|
(This was on release-21.2, so not caused by the fallout from #69887. Should be investigated separately.) |
Thanks and sorry for the noise. This seems totally legit.
|
@nvanbenschoten, @rafiss, @otan this seems to be related to |
This seems likely. There was no version gating added in #68877. How do we usually perform version gating on newly introduced SQL types? |
Agh, yeah it must be version gating, sorry for missing it in the review. I think what makes sense to add the version gating in CREATE and ALTER TABLE around these two areas: cockroach/pkg/sql/add_column.go Line 44 in 43f0d27
cockroach/pkg/sql/create_table.go Line 1636 in 43f0d27
|
Last time we did it, we did it here: cockroach/pkg/sql/create_table.go Lines 115 to 145 in 661fbbc
|
On a related note, how do we handle version gating of newly introduced builtin functions? Can't these be stored in table descriptors (e.g. in |
That's a really good point. We need to build something there. We haven't tested enough in that direction. I'll file an issue. |
This was unfortunately all removed in f3d95c5. It seems like the kind of infrastructure we'll want to keep in place across versions, even if there are times when all types are temporarily supported. @rafiss is this something someone on your team would be able to pick up? I can as well, but likely won't get to this until Wed or Thurs. |
I can probably get to this tomorrow. |
Thanks @RichardJCai ! |
Thanks Richard! |
Also, we don't do any version gating (that i know of) when we change/fix the implementation of existing builtins. |
roachtest.schemachange/mixed-versions failed with artifacts on release-21.2 @ 24021ba163e4ac438b169d575cf1527a4aae394d:
Same failure on other branches
|
roachtest.schemachange/mixed-versions failed with artifacts on release-21.2 @ 14411b999aae710ca0f4a6376d58e302b197281b:
Reproduce
See: roachtest README
Same failure on other branches
This test on roachdash | Improve this report!
The text was updated successfully, but these errors were encountered: