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

sql: support virtual table indexes with generators #80422

Merged
merged 1 commit into from
May 6, 2022

Conversation

AlexTalks
Copy link
Contributor

@AlexTalks AlexTalks commented Apr 23, 2022

While previously virtual schema tables had support for defining tables
with a populate function, which eagerly loads all rows, or a
generator function, which lazy loads each row when called (possibly
running in a worker Goroutine), and also had support for virtual indexes
which would have their own populate functions, there was a subtle lack
of support for using virtual indexes with virtual tables that used a
generator, since the virtual index constraint logic would fall back to
a (possibly undefined) populate function in several cases. This change
fixes the nil pointer exception that could occur if using virtual
indexes with a table using a generator, and validates that the virtual
index is supported prior to use.

Release Note: None

Release Justification: Bug fix

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@AlexTalks AlexTalks changed the title Virtual index w generator sql: support virtual table indexes with generators Apr 23, 2022
While previously virtual schema tables had support for defining tables
with a `populate` function, which eagerly loads all rows, or a
`generator` function, which lazy loads each row when called (possibly
running in a worker Goroutine), and also had support for virtual indexes
which would have their own `populate` functions, there was a subtle lack
of support for using virtual indexes with virtual tables that used a
`generator`, since the virtual index constraint logic would fall back to
a (possibly undefined) `populate` function in several cases. This change
fixes the nil pointer exception that could occur if using virtual
indexes with a table using a `generator`, and validates that the virtual
index is supported prior to use.

Release note: None
Copy link
Member

@jordanlewis jordanlewis left a comment

Choose a reason for hiding this comment

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

:lgtm:

Thanks for the fix, and I appreciate your patience on the review!

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)

@AlexTalks
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented May 6, 2022

Build succeeded:

@craig craig bot merged commit 44c2fd5 into cockroachdb:master May 6, 2022
@AlexTalks AlexTalks deleted the virtual_index_w_generator branch May 13, 2022 23:08
craig bot pushed a commit that referenced this pull request May 14, 2022
79623: sql: add virtual indexes to crdb_internal.cluster_locks virtual table  r=AlexTalks a=AlexTalks

This change adds virtual indexes on the `table_id`, `database_name`, and
`table_name` columns of the `crdb_internal.cluster_locks` virtual table,
so that when queried, the `kv.Batch`es with `QueryLocksRequest`s can be
constrained to query only specific tables or databases. This allows the
requests to be much more limited, rather than needing to request all of
the ranges that comprise the key spans of all tables accessible by the
user.

Release note (sql change): Improved query performance for
`crdb_internal.cluster_locks` when issued with constraints in the WHERE
clause on `table_id`, `database_name`, or `table_name` columns.

Depends on #77876, #80422

80832: kvserver: IsCompleteTransaction might panic with certain batch sequences r=shralex a=shralex

It's unclear how this panic happened. One possibility is that EntTxn had a negative sequence number. Another hypothesis is that ba.Requests was concurrently mutated due to a data race. This happened once, so for now adding more info to the panic.

Release note: None

Jira issue: https://cockroachlabs.atlassian.net/browse/CRDB-14627

81190: roachtest: update activerecord adapter to v6.1.10 r=rafiss a=ecwall

refs #67893
refs #80777

This version correctly disables supports_expression_index to
prevent `ON CONFLICT expression` from appearing in generated
SQL statements.

Release note: None

81193: storage: upgrade to pebblev2 table format r=jbowens,nicktrav a=erikgrinaker

Resubmit of #76780, which was partially reverted for 22.1.

---

The `Pebblev2` SSTable format adds support for range keys. Add two new
cluster versions to provide the upgrade path - the first version for
bumping the store, the second for use as a feature gate.

Release note: None

81207: ttljob: fix a range edge case r=rafiss a=otan

See individual commits for details.

Refs: #81208

Co-authored-by: Alex Sarkesian <sarkesian@cockroachlabs.com>
Co-authored-by: shralex <shralex@gmail.com>
Co-authored-by: Evan Wall <wall@cockroachlabs.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
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.

3 participants