sql/sem/tree: break dependencies on catalog, sessiondata, security#80687
Merged
craig[bot] merged 11 commits intocockroachdb:masterfrom May 2, 2022
Merged
sql/sem/tree: break dependencies on catalog, sessiondata, security#80687craig[bot] merged 11 commits intocockroachdb:masterfrom
craig[bot] merged 11 commits intocockroachdb:masterfrom
Conversation
Member
|
This change is |
92cbcea to
f4a77d3
Compare
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
| "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" |
Contributor
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 |
Contributor
There was a problem hiding this comment.
maybe it belongs to pkg/base??? 
f4a77d3 to
86be457
Compare
Contributor
|
Merge away if CI is green |
d23a7c9 to
d800029
Compare
d800029 to
863891e
Compare
863891e to
06a1bd0
Compare
5fd0fc9 to
761e0b7
Compare
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
da24e5d to
a53fad6
Compare
Contributor
Author
|
TFTR! bors r+ |
Contributor
|
Build succeeded: |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.