-
Notifications
You must be signed in to change notification settings - Fork 465
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
Active Record 7 Updates #14485
Active Record 7 Updates #14485
Conversation
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify site settings. |
@kernfeld-cockroach The sample app and connection doc still appear to be working. Do we want to remove mentions of v5 and just support v6 and v7? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the version on the "third party tools" page?
The sample app and connection doc still appear to be working.
Thanks for verifying!
Do we want to remove mentions of v5 and just support v6 and v7?
Yes
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gemma-shay and @kernfeld-cockroach)
v22.1/install-client-drivers.md
line 336 at r2 (raw file):
{{site.data.alerts.callout_info}} The exact command above will vary depending on the desired version of Active Record. Specifically, version 5.1.x of Active Record requires version 0.2.x of the adapter; version 5.2.x of Active Record requires version 5.2.x of the adapter; version 6.0.x of Active Record requires version 6.0.x of the adapter; version 7.0.x of Active Record requires version 7.0.x of the adapter.
No v5, as mentioned
v22.1/spatial-data.md
line 42 at r2 (raw file):
- [RGeo/RGeo-Active Record](https://github.com/rgeo/rgeo-activerecord) CockroachDB's Active Record adapter ([`activerecord-cockroachdb-adapter`](https://github.com/cockroachdb/activerecord-cockroachdb-adapter)) uses [RGeo](https://github.com/rgeo/rgeo) and [RGeo-Active Record](https://github.com/rgeo/rgeo-activerecord) for spatial support with Active Record v5.2.2+, v6.0.0+, and v7.0.0+. For information about using CockroachDB spatial features with Active Record, see the [`activerecord-cockroachdb-adapter` README](https://github.com/cockroachdb/activerecord-cockroachdb-adapter#working-with-spatial-data).
No v5, as mentioned
@kernfeld-cockroach the version on the third party tools page appears to be coming from here: https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/tests/activerecord.go |
@gemma-shay could you try the PR to change it and add Rafi as the reviewer? If the tests are green, we're probably good to go. If there are test failures, we'll need to hand it over to SQL Experience to pursue further. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, had a couple comments on the name of the Ruby geo lib
84208: Update activerecord.go v7 support r=rafiss a=gemma-shay cockroachdb/docs#14485 [DOC-4878](https://cockroachlabs.atlassian.net/browse/DOC-4878) cc `@kernfeld-cockroach` 84214: storage: handle range keys in readAsOfIterator r=erikgrinaker a=msbutler Previously, the readAsOfIterator used in RESTORE could not handle range keys. This PR implements the new SimpleMVCCIterator methods that handle range keys. Further, this patch ensures the readAsOfIterator skips over point keys shadowed by range keys at or below the caller's specified asOf timestamp. Next, Backup needs to be tought about RangeKeys. Informs #71155 Release note: none 84323: tracing: delete old field r=andreimatei a=andreimatei The RecordedSpan.RedactableLogs. This field was unused since 22.1. Release note: None 84742: sql/distsql: delete unused lazyInternalExecutor r=andreimatei a=andreimatei Release note: None 84784: sql/sqlinstance/instancestorage: use CommitInBatch to optimize round-… r=ajwerner a=ajwerner …trips By using CommitInBatch we can hit the 1PC optimization and avoid a round-trip to the leaseholder of the range in question. Release note: None 84849: opt: clarify plan gist comment r=mgartner a=mgartner Release note: None 84851: sql: fix recent leak of a context r=yuzefovich a=yuzefovich Fixes: #84801. Release note: None 84853: ccl/streamingccl/streamingest: skip TestTenantStreamingPauseResumeIngestion r=yuzefovich a=yuzefovich Refs: #84414 Reason: flaky test Generated by bin/skip-test. Release justification: non-production code changes Release note: None 84862: logcrash: fix test on arm r=dt a=dt Release note: none. Co-authored-by: Gemma Shay <[email protected]> Co-authored-by: Michael Butler <[email protected]> Co-authored-by: Andrei Matei <[email protected]> Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: David Taylor <[email protected]>
just nits, still works correctly
DOC-4878