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/catalog: rework validation and hydration to use cached descriptors #72402

Closed
ajwerner opened this issue Nov 3, 2021 · 3 comments
Closed
Labels
A-schema-descriptors Relating to SQL table/db descriptor handling. 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
Copy link
Contributor

ajwerner commented Nov 3, 2021

Is your feature request related to a problem? Please describe.
Today validation, including cross descriptor validation, occurs when reading a descriptor from the store in the catalogkv package. This is a low-level layer. The cross-descriptor validation not only validates the references from the descriptor being retrieved to what it references, it also fetches the dependencies one layer deeper so that that all references of descriptors which should back-reference this descriptor are validated.

This can cause real efficiency problems when, say, resolving schemas. Each schema will resolve its database which will, in turn, resolve all of the schemas. This is an O(N^2) proposition.

Describe the solution you'd like

To me, a key problem is the layering between retrieval of "raw" descriptors from the database and its relationship to validation. Instead of fetching and validating in one call, we should fetch and return a builder and then validate using already fetched descriptors in the descriptor collection. This can avoid the need to fetch any additional descriptors from the store most of the time.

Describe alternatives you've considered
We could do less cross-descriptor validation.

Additional context
Relates to #64388 and #64089.

Epic: CRDB-11534

Jira issue: CRDB-11135

@ajwerner ajwerner added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-schema-descriptors Relating to SQL table/db descriptor handling. labels Nov 3, 2021
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Nov 3, 2021
@ajwerner
Copy link
Contributor Author

ajwerner commented Mar 3, 2022

@postamar if I'm not mistaken, you've now fixed this 🥳

@postamar
Copy link
Contributor

postamar commented Mar 3, 2022

I got part of the way there only. There are still direct descriptor reads which avoid the collection for validation-related lookups, see readValidationDereferencer in catkv. That's the thing we need to get rid of, ultimately.

@postamar
Copy link
Contributor

I have fixed this completely with #77630

@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-descriptors Relating to SQL table/db descriptor handling. 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)
Projects
None yet
Development

No branches or pull requests

2 participants