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: internalLookupCtx should be constructed using a collection #83919

Closed
postamar opened this issue Jul 6, 2022 · 3 comments
Closed

sql: internalLookupCtx should be constructed using a collection #83919

postamar opened this issue Jul 6, 2022 · 3 comments
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@postamar
Copy link
Contributor

postamar commented Jul 6, 2022

This requires a fairly simple rewrite of the newInternalLookupCtx definition and all its call sites except one:

lCtx, lErr := newInternalLookupCtxFromDescriptorProtos(
ctx, allDescs, nil, /* want all tables */
)

In this case, we don't rely on descriptors provided by descs.Collection, instead these come from a backup manifest. To deal with this, try inserting these descriptors as synthetic descriptors into the descs.Collection. That way, newInternalLookupCtx can become oblivious to where the descriptors are actually sourced from.

Jira issue: CRDB-17352

@postamar postamar added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jul 6, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Jul 6, 2022
@postamar
Copy link
Contributor Author

postamar commented Jul 6, 2022

Relates to #64673
cc @ajwerner @chengxiong-ruan

@ajwerner
Copy link
Contributor

ajwerner commented Jul 6, 2022

This is an approach. I've been thinking instead that we get rid of the internalLookupCtx altogether.

@postamar
Copy link
Contributor Author

postamar commented Jul 7, 2022

Baby steps! Jason is looking for a way to get started in this direction.

@postamar postamar added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. and removed C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) labels Nov 10, 2022
@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
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

3 participants