sql: migrate has_sequence_privilege from evalPrivilegeCheck to ctx.Pl…

…anner.HasPrivilege refs #66173 Migrate has_sequence_privilege from evalPrivilegeCheck to ctx.Planner.HasPrivilege. Release note: None
cockroachdb · Jan 19, 2022 · 7afe472 · 7afe472
1 parent b917f1e
commit 7afe472
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 84 deletions.
diff --git a/pkg/sql/logictest/testdata/logic_test/privilege_builtins b/pkg/sql/logictest/testdata/logic_test/privilege_builtins
@@ -733,7 +733,7 @@ true  true  true
 query error pgcode 42P01 relation "does_not_exist" does not exist
 SELECT has_sequence_privilege('does_not_exist', 'SELECT')
 
-query error pgcode 42809 't' is not a sequence
+query error pgcode 42809 "t" is not a sequence
 SELECT has_sequence_privilege('t', 'SELECT')
 
 query BBB

diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go
@@ -406,11 +406,19 @@ func (p *planner) ResolveDescriptorForPrivilegeSpecifier(
 		if err != nil {
 			return nil, err
 		}
-		if err := validateColumnForHasPrivilegeSpecifier(
-			table,
-			specifier,
-		); err != nil {
-			return nil, err
+		if *specifier.IsSequence {
+			// has_table_privilege works with sequences, but has_sequence_privilege does not work with tables
+			if !table.IsSequence() {
+				return nil, pgerror.Newf(pgcode.WrongObjectType,
+					"\"%s\" is not a sequence", table.GetName())
+			}
+		} else {
+			if err := validateColumnForHasPrivilegeSpecifier(
+				table,
+				specifier,
+			); err != nil {
+				return nil, err
+			}
 		}
 		return table, nil
 	}

diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go
@@ -451,46 +451,6 @@ func getNameForArg(ctx *tree.EvalContext, arg tree.Datum, pgTable, pgCol string)
 	return string(tree.MustBeDString(r[0])), nil
 }
 
-// getTableNameForArg determines the qualified table name for the specified
-// argument, which should be either an unwrapped STRING or an OID. If the table
-// is not found, the returned pointer will be nil.
-func getTableNameForArg(ctx *tree.EvalContext, arg tree.Datum) (*tree.TableName, error) {
-	switch t := arg.(type) {
-	case *tree.DString:
-		tn, err := parser.ParseQualifiedTableName(string(*t))
-		if err != nil {
-			return nil, err
-		}
-		if _, err := ctx.Planner.ResolveTableName(ctx.Ctx(), tn); err != nil {
-			return nil, err
-		}
-		if ctx.SessionData().Database != "" && ctx.SessionData().Database != string(tn.CatalogName) {
-			// Postgres does not allow cross-database references in these
-			// functions, so we don't either.
-			return nil, pgerror.Newf(pgcode.FeatureNotSupported,
-				"cross-database references are not implemented: %s", tn)
-		}
-		return tn, nil
-	case *tree.DOid:
-		r, err := ctx.Planner.QueryRowEx(ctx.Ctx(), "get-table-name-for-arg",
-			ctx.Txn,
-			sessiondata.NoSessionDataOverride,
-			`SELECT n.nspname, c.relname FROM pg_class c
-			JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
-			WHERE c.oid = $1`, t)
-		if err != nil || r == nil {
-			return nil, err
-		}
-		db := tree.Name(ctx.SessionData().Database)
-		schema := tree.Name(tree.MustBeDString(r[0]))
-		table := tree.Name(tree.MustBeDString(r[1]))
-		tn := tree.MakeTableNameWithSchema(db, schema, table)
-		return &tn, nil
-	default:
-		return nil, errors.AssertionFailedf("unexpected arg type %T", t)
-	}
-}
-
 // privMap maps a privilege string to a Privilege.
 type privMap map[string]privilege.Privilege
 
@@ -1441,7 +1401,7 @@ SELECT description
 		argTypeOpts{{"table", strOrOidTypes}},
 		func(ctx *tree.EvalContext, args tree.Datums, user security.SQLUsername) (tree.Datum, error) {
 			tableArg := tree.UnwrapDatum(ctx, args[0])
-			specifier, err := tableHasPrivilegeSpecifier(tableArg)
+			specifier, err := tableHasPrivilegeSpecifier(tableArg, false /* isSequence */)
 			if err != nil {
 				return nil, err
 			}
@@ -1706,36 +1666,10 @@ SELECT description
 		argTypeOpts{{"sequence", strOrOidTypes}},
 		func(ctx *tree.EvalContext, args tree.Datums, user security.SQLUsername) (tree.Datum, error) {
 			seqArg := tree.UnwrapDatum(ctx, args[0])
-			tn, err := getTableNameForArg(ctx, seqArg)
+			specifier, err := tableHasPrivilegeSpecifier(seqArg, true /* isSequence */)
 			if err != nil {
 				return nil, err
 			}
-			pred := ""
-			retNull := false
-			if tn == nil {
-				// Postgres returns NULL if no matching table is found
-				// when given an OID.
-				retNull = true
-			} else {
-				// Verify that the table name is actually a sequence.
-				if r, err := ctx.Planner.QueryRowEx(
-					ctx.Ctx(), "has-sequence-privilege",
-					ctx.Txn,
-					sessiondata.NoSessionDataOverride,
-					`SELECT sequence_name FROM information_schema.sequences `+
-						`WHERE sequence_catalog = $1 AND sequence_schema = $2 AND sequence_name = $3`,
-					tn.CatalogName, tn.SchemaName, tn.ObjectName); err != nil {
-					return nil, err
-				} else if r == nil {
-					return nil, pgerror.Newf(pgcode.WrongObjectType,
-						"%s is not a sequence", seqArg)
-				}
-
-				pred = fmt.Sprintf(
-					"table_catalog = '%s' AND table_schema = '%s' AND table_name = '%s'",
-					tn.CatalogName, tn.SchemaName, tn.ObjectName)
-			}
-
 			privs, err := parsePrivilegeStr(args[1], privMap{
 				// Sequences and other table objects cannot be given a USAGE privilege,
 				// so we check for SELECT here instead. See privilege.TablePrivileges.
@@ -1749,12 +1683,9 @@ SELECT description
 			if err != nil {
 				return nil, err
 			}
-			if retNull {
-				return tree.DNull, nil
-			}
 			return runPrivilegeChecks(privs, func(priv privilege.Privilege) (tree.Datum, error) {
-				return evalPrivilegeCheck(ctx, "information_schema", "table_privileges",
-					user, pred, priv.Kind)
+				ret, err := ctx.Planner.HasPrivilege(ctx.Context, specifier, user, priv)
+				return handleTableHasPrivilegeError(specifier, ret, err)
 			})
 		},
 	),
@@ -1802,7 +1733,7 @@ SELECT description
 		argTypeOpts{{"table", strOrOidTypes}},
 		func(ctx *tree.EvalContext, args tree.Datums, user security.SQLUsername) (tree.Datum, error) {
 			tableArg := tree.UnwrapDatum(ctx, args[0])
-			specifier, err := tableHasPrivilegeSpecifier(tableArg)
+			specifier, err := tableHasPrivilegeSpecifier(tableArg, false /* isSequence */)
 			if err != nil {
 				return nil, err
 			}
@@ -2407,8 +2338,12 @@ func databaseHasPrivilegeSpecifier(databaseArg tree.Datum) (tree.HasPrivilegeSpe
 
 // tableHasPrivilegeSpecifier returns the HasPrivilegeSpecifier for
 // the given table.
-func tableHasPrivilegeSpecifier(tableArg tree.Datum) (tree.HasPrivilegeSpecifier, error) {
-	var specifier tree.HasPrivilegeSpecifier
+func tableHasPrivilegeSpecifier(
+	tableArg tree.Datum, isSequence bool,
+) (tree.HasPrivilegeSpecifier, error) {
+	specifier := tree.HasPrivilegeSpecifier{
+		IsSequence: &isSequence,
+	}
 	switch t := tableArg.(type) {
 	case *tree.DString:
 		s := string(*t)
@@ -2426,7 +2361,7 @@ func tableHasPrivilegeSpecifier(tableArg tree.Datum) (tree.HasPrivilegeSpecifier
 func columnHasPrivilegeSpecifier(
 	tableArg tree.Datum, colArg tree.Datum,
 ) (tree.HasPrivilegeSpecifier, error) {
-	specifier, err := tableHasPrivilegeSpecifier(tableArg)
+	specifier, err := tableHasPrivilegeSpecifier(tableArg, false /* isSequence */)
 	if err != nil {
 		return specifier, err
 	}

diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go
@@ -3120,7 +3120,7 @@ type EvalDatabase interface {
 }
 
 // HasPrivilegeSpecifier specifies an object to lookup privilege for.
-// Only one of DatabaseName, DatabaseOID, TableName, TableOID is filled.
+// Only one of { DatabaseName, DatabaseOID, TableName, TableOID } is filled.
 type HasPrivilegeSpecifier struct {
 
 	// Database privilege
@@ -3130,6 +3130,8 @@ type HasPrivilegeSpecifier struct {
 	// Table privilege
 	TableName *string
 	TableOID  *oid.Oid
+	// Sequences are stored internally as a table
+	IsSequence *bool
 
 	// Column privilege
 	// Requires TableName or TableOID.