sql/catalog: build composable and well-defined catalog abstractions #64089
Labels
A-schema-catalog
Related to the schema descriptors collection and the catalog API in general.
A-schema-transactional
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Comments
ajwerner
added
C-enhancement
craig bot
pushed a commit
that referenced
this issue
Jun 4, 2021
65142: sql/*: remove catalog.ResolvedSchema, generalize SchemaDescriptor r=postamar a=ajwerner The `ResolvedSchema` was a blemish on the descriptor resolution APIs. It made it very hard to create a reasonable abstraction for descriptor resolution injection. This commit creates new implementations of `SchemaDescriptor` to represent virtual, public, and temporary schemas. There is likely more cleanup that could be done along the way to make the different kinds of schemas more uniform, but that is left for later or never. Relates to #64089. Release note: None 65780: sql: Add unit tests for planning inside the new schema changer r=ajwerner a=fqazi Previously, the new schema changer did not have any unit tests covering the planning capability. This was inadequate, because we had no way of detect if plans were regressing with changes or enhancements. To address this, this patch adds basic tests to see if the operators/dependencies for a given command are sane. Release note: None 66051: logcrash: update non-release Sentry URL r=mgartner a=mgartner The documentation for creating Sentry reports in non-release builds directed developers to set the COCKROACH_CRASH_REPORTS env var to an incorrect URL. Sentry reports were not created when using this URL. The documentation has been updated to refer to the correct URL. Release note: None 66070: sql: fix very slow TestSchemaChangeRetryOnVersionChange r=fqazi a=ajwerner See [here](https://teamcity.cockroachdb.com/project.html?projectId=Cockroach_UnitTests&testNameId=6532733632782929224&tab=testDetails). This test was taking 10 minutes every time. This was happening because of the change to wait for the version's leases to expire in #63725. After: ``` --- PASS: TestSchemaChangeRetryOnVersionChange (1.70s) ``` Release note: None 66074: sql: fix statement diagnostics for EXECUTE r=RaduBerinde a=RaduBerinde Previously, prepared statements ran through the EXECUTE statements could not trigger collection of statement diagnostics. This is because we consider the fingerprint of the EXECUTE statement instead of the one from the prepared statement. The fix is to move up the special handling code in the executor, and replace the AST and fingerprint before setting up the instrumentation helper. Release note (bug fix): queries ran through the EXECUTE statement can now generate statement diagnostic bundles as expected. Fixes #66048. Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Faizan Qazi <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Radu Berinde <[email protected]>
postamar
added
A-schema-catalog
|
@ajwerner correct me if I'm wrong but at this point, this boils down to:
right? |
|
Yes. With the resolver interface move would be a moving of privilege checking too I believe. |
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 13, 2022
This commit refactors the descs.Collection's GetAll* methods, which
return descriptors and other catalog metadata in bulk, to share a common
access path in which the descriptor's layers are properly aggregated.
This is non-trivial as there are numerous edge cases:
- virtual schemas and objects exist for all databases and their
descriptors don't reference the parent database at all,
- descriptorless public schemas and temporary schemas only exist as
namespace entries,
- functions don't have namespace entries,
- and so forth.
These new methods are delegated to by the existing methods which are now
deprecated.
Informs cockroachdb#64089.
Release note: None
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 14, 2022
This commit refactors the descs.Collection's GetAll* methods, which
return descriptors and other catalog metadata in bulk, to share a common
access path in which the descriptor's layers are properly aggregated.
This is non-trivial as there are numerous edge cases:
- virtual schemas and objects exist for all databases and their
descriptors don't reference the parent database at all,
- descriptorless public schemas and temporary schemas only exist as
namespace entries,
- functions don't have namespace entries,
- and so forth.
These new methods are delegated to by the existing methods which are now
deprecated.
Informs cockroachdb#64089.
Release note: None
craig bot
pushed a commit
that referenced
this issue
Dec 14, 2022
93364: diagnosticccl: minor test clean r=andreimatei a=andreimatei
This test was using a storage node's Stopper in conjunction with a tenant server's component (the Reporter). That's not kosher - we were creating a trace having a parent span created with the storage node's Tracer, and a child created with the tenant's Tracer. Combining Tracers like that in a single trace is not allowed. The test is getting away with it because the parent span happened to be created with the "sterile" option, and sterile spans were allowed to be combined with children created with other Tracers as an exception for arcane reasons, but that's going away.
Release note: None
Epic: None
93450: vendor: bump Pebble to 0893071d8a52 r=jbowens a=jbowens
```
6e8d3bb3 db: add NextPrefix to Iterator
6311f7a9 db: add RangeKeyIteratorStats
06047857 db: refactor merging iterator's prefix handling for TrySeekUsingNext
e0f0de89 bench: add --max-size flag
88c8fbcc *: use golang.org/x/sys package
```
Epic: None
Release note: None
93543: descs: refactor GetAll* methods r=postamar a=postamar
This commit refactors the descs.Collection's GetAll* methods, which
return descriptors and other catalog metadata in bulk, to share a common
access path in which the descriptor's layers are properly aggregated.
This is non-trivial as there are numerous edge cases:
- virtual schemas and objects exist for all databases and their
descriptors don't reference the parent database at all,
- descriptorless public schemas and temporary schemas only exist as
namespace entries,
- functions don't have namespace entries,
- and so forth.
These new methods are delegated to by the existing methods which are now
deprecated.
Informs #64089.
Release note: None
Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 14, 2022
Informs cockroachdb#64089. Release note: None
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 14, 2022
This gets rid of some deprecated descs.Collection method calls. Informs cockroachdb#64089. Release note: None
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 14, 2022
This commit replaces usages of that method with calls to GetAllObjectsInSchema. Informs cockroachdb#64089. Release note: None
craig bot
pushed a commit
that referenced
this issue
Dec 14, 2022
93337: server: implement health endpoint for app tenants r=knz a=dhartunian Previously, there was a separate implementation of the Health endpoint for tenants in `tenant_admin.go`. Now that we've unified the implementations, that needs to be added to `adminServer`. The implementation for the system tenant uses liveness, this implementation uses the sqlServer's readiness to accept connections. Epic: [CRDB-17356](https://cockroachlabs.atlassian.net/browse/CRDB-17356) Release note: None 93570: sql: replace deprecated descs.Collection method calls r=postamar a=postamar descs: remove GetObjectNamesAndIDs method This commit replaces usages of that method with calls to GetAllObjectsInSchema. Informs #64089. Release note: None ---- sql: amortize schema descriptor lookups in vtable generation This gets rid of some deprecated descs.Collection method calls. Informs #64089. Release note: None ---- scbuild,scdeps: replace deprecated descs.Collection method calls Informs #64089. Release note: None Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Marius Posta <[email protected]>
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 14, 2022
This can now be replaced by calls to descs.Collection methods. Informs cockroachdb#64089. Release note: None
postamar
pushed a commit
to postamar/cockroach
that referenced
this issue
Dec 15, 2022
This can now be replaced by calls to descs.Collection methods. Informs cockroachdb#64089. Release note: None
craig bot
pushed a commit
that referenced
this issue
Dec 15, 2022
93611: catkv,*: remove catkv.Direct interface r=postamar a=postamar Informs #64089. Release note: None Co-authored-by: Marius Posta <[email protected]>
exalate-issue-sync
bot
added
T-sql-foundations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently in cockroach the access to the catalog (mapping names and IDs to their meanings) is scattered around the code-base. There are a number of layers to this access which are confusing and hard to use. Access is generally a session-level concept, though there are a variety of needs to access the catalog in asynchronous and background jobs. In order to make Transactional Schema Changes work we'll need to be able to properly orchestrate the transaction used for lookups (#56588) as well as controlling deadlines. All of this hints at a need for more abstraction both above and below the
*descs.Collection.Tons of resolution functionality exists hanging off the
*sql.planneras it implements a variety of resolution-related interfaces.This is a meta-issue for tracking a relatively large set of work related to these catalog improvements. Other work to improve the testability and reliability will be performed along the way.
sqlpackage.Epic: CRDB-2454
Jira issue: CRDB-6887
The text was updated successfully, but these errors were encountered: