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

Cockroachdb v21.2 Support #229

Merged
merged 15 commits into from
Nov 30, 2021

Conversation

keithdoggett
Copy link
Contributor

@keithdoggett keithdoggett commented Oct 13, 2021

Add support for CockroachDB v21.2.

current version: 21.2.0

Known issues:

TODOs (so far):

  • Add version identification in adapter for v21.2.
  • Add interval style statement back to adapter

@keithdoggett
Copy link
Contributor Author

The intervalstyle statement has been added and the interval tests have been reverted back to ActiveRecord with the exception of the schema dump with default value test. (cockroachdb/cockroach#71776) needs to be resolved before it will work properly.

If a fix to that change won't be rolled into this release then we will probably have to add our custom interval implementation back to handle this case.

@keithdoggett
Copy link
Contributor Author

Added the Interval implementation back because this does still need to support older versions that do not allow intervalstyle. Once all supported versions allow intervalstyle to be set then our implementation can be removed.

@keithdoggett
Copy link
Contributor Author

@rafiss do you know if any of the fixes mentioned in the first comment will be merged into the 21.2 release?

@keithdoggett
Copy link
Contributor Author

@rafiss do you have any update on those fixes will be included in this release or will we need to come up with workarounds for this version?

@rafiss
Copy link
Contributor

rafiss commented Nov 12, 2021

Sorry I missed this. cockroachdb/cockroach#71440 and cockroachdb/cockroach#72041 will both be available in v21.2.1 (not in v21.2.0)

@keithdoggett
Copy link
Contributor Author

Ok sounds good.

  • I can add an additional check for this version to determine if the database doesn't exist.
  • I can replicate the constraints tests within the Cockroach adapter and disable transactions for the failing tests.
  • For the expression indexes we have a few options:
    • Disable expression indexes until 21.2.1 is released
    • Skip the failing test and just note that it won't work until 21.2.1
    • Try to override the column definition method to identify expression indexes where indkey isn't 0 then revert back to the upstream version once 21.2.1 is released.

I can go with whatever you think is best for the third issue.

@rafiss
Copy link
Contributor

rafiss commented Nov 12, 2021

"Disable expression indexes until 21.2.1 is released" sounds like the best option to me. Is there a way we can add a version gate to only allow the feature/test if the version is >= v21.2.1?

…ck for NoDatabaseError, add sig fig to crdb_versions so that patches can be accounted for, enbable expression indexes in 21.2.1
@keithdoggett
Copy link
Contributor Author

Yes, I added a sig fig to the crdb version attribute so we can account for patches.

@keithdoggett
Copy link
Contributor Author

@rafiss I added a helper to retry tests that fail because of the random foreign key error. This should help development until the root cause gets fixed.

@rafiss
Copy link
Contributor

rafiss commented Nov 23, 2021

The latest failure is from https://github.com/cockroachdb/cockroach/issues/71652 so retrying CI again

Also, unfortunately there was a change in our release schedule, so the expression indexes fix won't go out until 21.2.2

Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! feel free to merge when you'd like

@keithdoggett
Copy link
Contributor Author

Thanks! I changed the version in supports_expression_index? and will merge once the CI finishes.

@keithdoggett keithdoggett merged commit cb609fd into cockroachdb:master Nov 30, 2021
@keithdoggett keithdoggett deleted the cockroachdb-212 branch November 30, 2021 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants