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: update search_path semantics to match pg now that we support UDS #54538

Merged
merged 1 commit into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions pkg/sql/catalog/catconstants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,7 @@

package catconstants

import (
"math"

"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
)

// DefaultSearchPath is the search path used by virgin sessions.
var DefaultSearchPath = sessiondata.MakeSearchPath([]string{"public"})
import "math"

// ReportableAppNamePrefix indicates that the application name can be
// reported in telemetry without scrubbing. (Note this only applies to
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/database"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -545,7 +544,7 @@ func (s *Server) populateMinimalSessionData(sd *sessiondata.SessionData) {
}
}
if len(sd.SearchPath.GetPathArray()) == 0 {
sd.SearchPath = catconstants.DefaultSearchPath
sd.SearchPath = sessiondata.DefaultSearchPathForUser(sd.User)
}
}

Expand Down
8 changes: 6 additions & 2 deletions pkg/sql/distsql/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,12 @@ func (ds *ServerImpl) setupFlow(
ApplicationName: req.EvalContext.ApplicationName,
Database: req.EvalContext.Database,
User: req.EvalContext.User,
SearchPath: sessiondata.MakeSearchPath(req.EvalContext.SearchPath).WithTemporarySchemaName(req.EvalContext.TemporarySchemaName),
SequenceState: sessiondata.NewSequenceState(),
SearchPath: sessiondata.MakeSearchPath(
req.EvalContext.SearchPath,
).WithTemporarySchemaName(
req.EvalContext.TemporarySchemaName,
).WithUserSchemaName(req.EvalContext.User),
SequenceState: sessiondata.NewSequenceState(),
DataConversion: sessiondata.DataConversionConfig{
Location: location,
BytesEncodeFormat: be,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/discard
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ DISCARD ALL
query T
SHOW SEARCH_PATH
----
public
$user,public

query T
SET timezone = 'Europe/Amsterdam'; SHOW TIMEZONE
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog
Original file line number Diff line number Diff line change
Expand Up @@ -1854,7 +1854,7 @@ reorder_joins_limit 8 NULL NUL
require_explicit_primary_keys off NULL NULL NULL string
results_buffer_size 16384 NULL NULL NULL string
row_security off NULL NULL NULL string
search_path public NULL NULL NULL string
search_path $user,public NULL NULL NULL string
serial_normalization rowid NULL NULL NULL string
server_encoding UTF8 NULL NULL NULL string
server_version 9.5.0 NULL NULL NULL string
Expand Down Expand Up @@ -1924,7 +1924,7 @@ reorder_joins_limit 8 NULL user
require_explicit_primary_keys off NULL user NULL off off
results_buffer_size 16384 NULL user NULL 16384 16384
row_security off NULL user NULL off off
search_path public NULL user NULL public public
search_path $user,public NULL user NULL $user,public $user,public
serial_normalization rowid NULL user NULL rowid rowid
server_encoding UTF8 NULL user NULL UTF8 UTF8
server_version 9.5.0 NULL user NULL 9.5.0 9.5.0
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/reset
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ RESET SEARCH_PATH
query T
SHOW SEARCH_PATH
----
public
$user,public

statement error parameter "server_version" cannot be changed
RESET SERVER_VERSION
Expand All @@ -39,7 +39,7 @@ RESET search_path
query T
SHOW search_path
----
public
$user,public

statement ok
RESET client_encoding; RESET NAMES
Expand Down
74 changes: 74 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/schema
Original file line number Diff line number Diff line change
Expand Up @@ -459,3 +459,77 @@ CREATE TABLE new_db.public.bar()

statement ok
CREATE TABLE new_db.testuser.bar()

# cleanup the testuser schema created as part of the CREATE SCHEMA AUTHORIZATION
# command above
statement ok
DROP SCHEMA testuser CASCADE

# If a schema with a username exists, then that should be the first entry in
# the search path.
subtest user_schema_search_path

# Test setup
user root

statement ok
CREATE SCHEMA testuser

statement ok
GRANT ALL ON SCHEMA testuser TO testuser

statement ok
CREATE TABLE public.public_table(a INT)

statement ok
GRANT SELECT ON public.public_table TO testuser

user testuser

statement ok
CREATE TABLE test_table(a INT);

statement error pq: relation "public.test_table" does not exist
SELECT * FROM public.test_table

statement ok
SELECT * FROM testuser.test_table

# Only root has privs to create inside public
user root

statement ok
CREATE TABLE public.test_table(a INT, b INT)

statement ok
GRANT SELECT ON public.test_table TO testuser

user testuser

query I colnames
SELECT * FROM test_table
----
a

query II colnames
SELECT * FROM public.test_table
----
a b

query I colnames
SELECT * FROM public_table
----
a

# The search path is configured to be user specific.
user root

query II colnames
SELECT * FROM test_table
----
a b

query I colnames
SELECT * FROM testuser.test_table
----
a
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/show_source
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ reorder_joins_limit 8
require_explicit_primary_keys off
results_buffer_size 16384
row_security off
search_path public
search_path $user,public
serial_normalization rowid
server_encoding UTF8
server_version 9.5.0
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
Expand Down Expand Up @@ -263,7 +262,7 @@ func newInternalPlanner(
ctx := logtags.AddTag(context.Background(), opName, "")

sd := &sessiondata.SessionData{
SearchPath: catconstants.DefaultSearchPath,
SearchPath: sessiondata.DefaultSearchPathForUser(user),
User: user,
Database: "system",
SequenceState: sessiondata.NewSequenceState(),
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/schema_changer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
Expand Down Expand Up @@ -1972,7 +1971,7 @@ func createSchemaChangeEvalCtx(

func newFakeSessionData() *sessiondata.SessionData {
sd := &sessiondata.SessionData{
SearchPath: catconstants.DefaultSearchPath,
SearchPath: sessiondata.DefaultSearchPathForUser(security.NodeUser),
// The database is not supposed to be needed in schema changes, as there
// shouldn't be unqualified identifiers in backfills, and the pure functions
// that need it should have already been evaluated.
Expand Down
46 changes: 43 additions & 3 deletions pkg/sql/sessiondata/search_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const PgCatalogName = "pg_catalog"
// PublicSchemaName is the name of the pg_catalog system schema.
const PublicSchemaName = "public"

// UserSchemaName is the alias for schema names for users.
const UserSchemaName = "$user"

// InformationSchemaName is the name of the information_schema system schema.
const InformationSchemaName = "information_schema"

Expand All @@ -40,6 +43,11 @@ const PgTempSchemaName = "pg_temp"
// when installing an extension, but must be stored as a separate schema in CRDB.
const PgExtensionSchemaName = "pg_extension"

// DefaultSearchPath is the search path used by virgin sessions.
var DefaultSearchPath = MakeSearchPath(
[]string{UserSchemaName, PublicSchemaName},
)

// SearchPath represents a list of namespaces to search builtins in.
// The names must be normalized (as per Name.Normalize) already.
type SearchPath struct {
Expand All @@ -48,11 +56,18 @@ type SearchPath struct {
containsPgExtension bool
containsPgTempSchema bool
tempSchemaName string
userSchemaName string
}

// EmptySearchPath is a SearchPath with no schema names in it.
var EmptySearchPath = SearchPath{}

// DefaultSearchPathForUser returns the default search path with the user
// specific schema name set so that it can be expanded during resolution.
func DefaultSearchPathForUser(username string) SearchPath {
return DefaultSearchPath.WithUserSchemaName(username)
}

// MakeSearchPath returns a new immutable SearchPath struct. The paths slice
// must not be modified after hand-off to MakeSearchPath.
func MakeSearchPath(paths []string) SearchPath {
Expand Down Expand Up @@ -87,14 +102,28 @@ func (s SearchPath) WithTemporarySchemaName(tempSchemaName string) SearchPath {
containsPgCatalog: s.containsPgCatalog,
containsPgTempSchema: s.containsPgTempSchema,
containsPgExtension: s.containsPgExtension,
userSchemaName: s.userSchemaName,
tempSchemaName: tempSchemaName,
}
}

// WithUserSchemaName returns a new immutable SearchPath struct with the
// userSchemaName populated and the same values for all other fields as before.
func (s SearchPath) WithUserSchemaName(userSchemaName string) SearchPath {
return SearchPath{
paths: s.paths,
containsPgCatalog: s.containsPgCatalog,
containsPgTempSchema: s.containsPgTempSchema,
containsPgExtension: s.containsPgExtension,
userSchemaName: userSchemaName,
tempSchemaName: s.tempSchemaName,
}
}

// UpdatePaths returns a new immutable SearchPath struct with the paths supplied
// and the same tempSchemaName as before.
// and the same tempSchemaName and userSchemaName as before.
func (s SearchPath) UpdatePaths(paths []string) SearchPath {
return MakeSearchPath(paths).WithTemporarySchemaName(s.tempSchemaName)
return MakeSearchPath(paths).WithTemporarySchemaName(s.tempSchemaName).WithUserSchemaName(s.userSchemaName)
}

// MaybeResolveTemporarySchema returns the session specific temporary schema
Expand Down Expand Up @@ -135,6 +164,7 @@ func (s SearchPath) Iter() SearchPathIter {
implicitPgExtension: !s.containsPgExtension,
implicitPgTempSchema: implicitPgTempSchema,
tempSchemaName: s.tempSchemaName,
userSchemaName: s.userSchemaName,
}
return sp
}
Expand All @@ -147,6 +177,7 @@ func (s SearchPath) IterWithoutImplicitPGSchemas() SearchPathIter {
implicitPgCatalog: false,
implicitPgTempSchema: false,
tempSchemaName: s.tempSchemaName,
userSchemaName: s.userSchemaName,
}
return sp
}
Expand Down Expand Up @@ -202,7 +233,7 @@ func (s SearchPath) Equals(other *SearchPath) bool {
}

func (s SearchPath) String() string {
return strings.Join(s.paths, ", ")
return strings.Join(s.paths, ",")
}

// SearchPathIter enables iteration over the search paths without triggering an
Expand All @@ -216,6 +247,7 @@ type SearchPathIter struct {
implicitPgExtension bool
implicitPgTempSchema bool
tempSchemaName string
userSchemaName string
i int
}

Expand Down Expand Up @@ -245,6 +277,14 @@ func (iter *SearchPathIter) Next() (path string, ok bool) {
}
return iter.tempSchemaName, true
}
if iter.paths[iter.i-1] == UserSchemaName {
// In case the user schema name is unset, we simply iterate to the next
// entry.
if iter.userSchemaName == "" {
return iter.Next()
}
return iter.userSchemaName, true
}
// pg_extension should be read before delving into the schema.
if iter.paths[iter.i-1] == PublicSchemaName && iter.implicitPgExtension {
iter.implicitPgExtension = false
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/temporary_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
Expand Down Expand Up @@ -229,7 +228,7 @@ func cleanupSchemaObjects(
}
a := catalogkv.UncachedPhysicalAccessor{}

searchPath := catconstants.DefaultSearchPath.WithTemporarySchemaName(schemaName)
searchPath := sessiondata.DefaultSearchPathForUser(security.RootUser).WithTemporarySchemaName(schemaName)
override := sessiondata.InternalExecutorOverride{
SearchPath: &searchPath,
User: security.RootUser,
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/delegate"
"github.com/cockroachdb/cockroach/pkg/sql/lex"
"github.com/cockroachdb/cockroach/pkg/sql/paramparse"
Expand Down Expand Up @@ -827,7 +826,7 @@ var varGen = map[string]sessionVar{
return evalCtx.SessionData.SearchPath.String()
},
GlobalDefault: func(sv *settings.Values) string {
return catconstants.DefaultSearchPath.String()
return sessiondata.DefaultSearchPath.String()
},
},

Expand Down