sql/sem/tree: break dependencies on catalog, sessiondata, security #80687
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
from
92cbcea
to
f4a77d3
Compare
ajwerner
requested review from
shermanCRL and
stevendanna
and removed request for
a team
otan
approved these changes
Apr 28, 2022
| iter := sp.Iter() | ||
| for schema, ok := iter.Next(); ok; schema, ok = iter.Next() { | ||
| if err := f(schema); err != nil { | ||
| if iterutil.Done(err) { |
pkg/sql/alter_database.go
Outdated
| @@ -35,6 +35,7 @@ import ( | |||
| "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" | |||
| "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" | |||
| "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" | |||
| "github.com/cockroachdb/cockroach/pkg/sql/username" | |||
There was a problem hiding this comment.
this import confused me when i reviewed this commit but got lost somewhere. i don't know if you want to fix it.
| // PublicSchemaID redefines keys.PublicSchemaID to avoid an import. It exists | ||
| // to deal with time-travel queries from moments before the time when all | ||
| // databases other than system were given a public schema. | ||
| const PublicSchemaID = 29 |
There was a problem hiding this comment.
maybe it belongs to pkg/base??? 
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
from
f4a77d3
to
86be457
Compare
|
Merge away if CI is green |
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
4 times, most recently
from
d23a7c9
to
d800029
Compare
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
from
d800029
to
863891e
Compare
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
from
863891e
to
06a1bd0
Compare
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
6 times, most recently
from
5fd0fc9
to
761e0b7
Compare
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
from
8991138
to
da24e5d
Compare
It was big and obscuring the rest of the logic. Release note: None
Release note: None
No reason to involve sessiondata down in tree. Note that this is all probably moot because we're removing these options, but I didn't want to be the one to do that right now. Release note: None
The security package depends on logging amongst other things. This is easy to break as the methods were really just about string conversions. Also we needed sessiondata here. Release note: None
Tree and protobuf packages like sessiondatapb imported security just for the SQLUsername symbol and its associated proto symbol. There's no need for that. This was a purely automated move. Release note: None
Release note: None
Before this if we disallowed a prefix for imports and the package was itself under that prefix, the test would fail. Now we remove the package itself. Release note: None
Release note: None
It should just be constants. Release note: None
This is just constants. We want tree to be free of catalog. Release note: None
Release note: None
ajwerner
force-pushed
the
ajwerner/remove-catalog-from-tree
branch
from
da24e5d
to
a53fad6
Compare
|
TFTR! bors r+ |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pile of commits moves stuff and makes some minor changes to break more dependencies of tree. Most of the commits are totally mechanical. The ones which aren't relate to abstracting
SearchPatha tad and moving cast options to a struct. This should be straightforward to review commit-by-commit.The end goal here will be to break the dependencies of the parser on
util/log. There's still a few more things remaining here and there, but this felt like a reasonable stopping point.