release-22.2: schema comment fixes

pkg/sql/BUILD.bazel

@@ -537,6 +537,7 @@ go_test(
         "comment_on_constraint_test.go",
         "comment_on_database_test.go",
         "comment_on_index_test.go",
+        "comment_on_schema_test.go",
         "comment_on_table_test.go",
         "conn_executor_internal_test.go",
         "conn_executor_savepoints_test.go",

pkg/sql/comment_on_column_test.go

@@ -181,7 +181,7 @@ func TestCommentOnColumnWhenDropColumn(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		t.Fatal("comment remain")
+		t.Fatal("comment remaining in system.comments despite drop")
 	}
 }
 

pkg/sql/comment_on_database_test.go

@@ -104,6 +104,6 @@ func TestCommentOnDatabaseWhenDrop(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		t.Fatal("dropped comment remain comment")
+		t.Fatal("comment remaining in system.comments despite drop")
 	}
 }

pkg/sql/comment_on_schema_test.go

@@ -0,0 +1,109 @@
+// Copyright 2022 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package sql_test
+
+import (
+	"context"
+	gosql "database/sql"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/sql/tests"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/errors"
+)
+
+func TestCommentOnSchema(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	params, _ := tests.CreateTestServerParams()
+	s, db, _ := serverutils.StartServer(t, params)
+	defer s.Stopper().Stop(context.Background())
+
+	if _, err := db.Exec(`
+		CREATE SCHEMA d;
+	`); err != nil {
+		t.Fatal(err)
+	}
+
+	testCases := []struct {
+		exec   string
+		query  string
+		expect gosql.NullString
+	}{
+		{
+			`COMMENT ON SCHEMA d IS 'foo'`,
+			`SELECT obj_description(oid, 'pg_namespace') FROM pg_namespace WHERE nspname = 'd'`,
+			gosql.NullString{String: `foo`, Valid: true},
+		},
+		{
+			`ALTER SCHEMA d RENAME TO d2`,
+			`SELECT obj_description(oid, 'pg_namespace') FROM pg_namespace WHERE nspname = 'd2'`,
+			gosql.NullString{String: `foo`, Valid: true},
+		},
+		{
+			`COMMENT ON SCHEMA d2 IS NULL`,
+			`SELECT obj_description(oid, 'pg_namespace') FROM pg_namespace WHERE nspname = 'd2'`,
+			gosql.NullString{Valid: false},
+		},
+	}
+
+	for _, tc := range testCases {
+		if _, err := db.Exec(tc.exec); err != nil {
+			t.Fatal(err)
+		}
+
+		row := db.QueryRow(tc.query)
+		var comment gosql.NullString
+		if err := row.Scan(&comment); err != nil {
+			t.Fatal(err)
+		}
+		if tc.expect != comment {
+			t.Fatalf("expected comment %v, got %v", tc.expect, comment)
+		}
+	}
+}
+
+func TestCommentOnSchemaWhenDrop(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	params, _ := tests.CreateTestServerParams()
+	s, db, _ := serverutils.StartServer(t, params)
+	defer s.Stopper().Stop(context.Background())
+
+	if _, err := db.Exec(`
+		CREATE SCHEMA d;
+	`); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := db.Exec(`COMMENT ON SCHEMA d IS 'foo'`); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := db.Exec(`DROP SCHEMA d`); err != nil {
+		t.Fatal(err)
+	}
+
+	row := db.QueryRow(`SELECT comment FROM system.comments LIMIT 1`)
+	var comment string
+	err := row.Scan(&comment)
+	if !errors.Is(err, gosql.ErrNoRows) {
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		t.Fatal("comment remaining in system.comments despite drop")
+	}
+}

pkg/sql/comment_on_table_test.go

@@ -108,6 +108,6 @@ func TestCommentOnTableWhenDrop(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		t.Fatal("dropped comment remain comment")
+		t.Fatal("comment remaining in system.comments despite drop")
 	}
 }

pkg/sql/information_schema.go

@@ -344,7 +344,9 @@ https://www.postgresql.org/docs/9.5/infoschema-check-constraints.html`,
 				// uses the format <namespace_oid>_<table_oid>_<col_idx>_not_null.
 				// We might as well do the same.
 				conNameStr := tree.NewDString(fmt.Sprintf(
-					"%s_%s_%d_not_null", h.NamespaceOid(db.GetID(), scName), tableOid(table.GetID()), column.Ordinal()+1,
+					"%s_%s_%d_not_null",
+					h.NamespaceOid(db, scName),
+					tableOid(table.GetID()), column.Ordinal()+1,
 				))
 				chkExprStr := tree.NewDString(fmt.Sprintf(
 					"%s IS NOT NULL", column.GetName(),
@@ -1308,7 +1310,9 @@ https://www.postgresql.org/docs/9.5/infoschema-table-constraints.html`,
 					}
 					// NOT NULL column constraints are implemented as a CHECK in postgres.
 					conNameStr := tree.NewDString(fmt.Sprintf(
-						"%s_%s_%d_not_null", h.NamespaceOid(db.GetID(), scName), tableOid(table.GetID()), col.Ordinal()+1,
+						"%s_%s_%d_not_null",
+						h.NamespaceOid(db, scName),
+						tableOid(table.GetID()), col.Ordinal()+1,
 					))
 					if err := addRow(
 						dbNameStr,                // constraint_catalog

pkg/sql/logictest/testdata/logic_test/information_schema

@@ -1983,25 +1983,25 @@ SELECT *
 FROM information_schema.table_constraints
 ORDER BY TABLE_NAME, CONSTRAINT_TYPE, CONSTRAINT_NAME
 ----
-constraint_catalog  constraint_schema  constraint_name            table_catalog  table_schema  table_name  constraint_type  is_deferrable  initially_deferred
-constraint_db       public             3864823197_118_1_not_null  constraint_db  public        t1          CHECK            NO             NO
-constraint_db       public             c2                         constraint_db  public        t1          CHECK            NO             NO
-constraint_db       public             check_a                    constraint_db  public        t1          CHECK            NO             NO
-constraint_db       public             t1_pkey                    constraint_db  public        t1          PRIMARY KEY      NO             NO
-constraint_db       public             t1_a_key                   constraint_db  public        t1          UNIQUE           NO             NO
-constraint_db       public             3864823197_119_2_not_null  constraint_db  public        t2          CHECK            NO             NO
-constraint_db       public             fk                         constraint_db  public        t2          FOREIGN KEY      NO             NO
-constraint_db       public             t2_pkey                    constraint_db  public        t2          PRIMARY KEY      NO             NO
+constraint_catalog  constraint_schema  constraint_name     table_catalog  table_schema  table_name  constraint_type  is_deferrable  initially_deferred
+constraint_db       public             117_118_1_not_null  constraint_db  public        t1          CHECK            NO             NO
+constraint_db       public             c2                  constraint_db  public        t1          CHECK            NO             NO
+constraint_db       public             check_a             constraint_db  public        t1          CHECK            NO             NO
+constraint_db       public             t1_pkey             constraint_db  public        t1          PRIMARY KEY      NO             NO
+constraint_db       public             t1_a_key            constraint_db  public        t1          UNIQUE           NO             NO
+constraint_db       public             117_119_2_not_null  constraint_db  public        t2          CHECK            NO             NO
+constraint_db       public             fk                  constraint_db  public        t2          FOREIGN KEY      NO             NO
+constraint_db       public             t2_pkey             constraint_db  public        t2          PRIMARY KEY      NO             NO
 
 query TTTT colnames
 SELECT *
 FROM information_schema.check_constraints
 ORDER BY CONSTRAINT_CATALOG, CONSTRAINT_NAME
 ----
-constraint_catalog  constraint_schema  constraint_name            check_clause
-constraint_db       public             3864823197_118_1_not_null  p IS NOT NULL
-constraint_db       public             c2                         ((a < 99:::INT8))
-constraint_db       public             check_a                    ((a > 4:::INT8))
+constraint_catalog  constraint_schema  constraint_name     check_clause
+constraint_db       public             117_118_1_not_null  p IS NOT NULL
+constraint_db       public             c2                  ((a < 99:::INT8))
+constraint_db       public             check_a             ((a > 4:::INT8))
 
 query TTTTTTT colnames
 SELECT *
@@ -2026,10 +2026,10 @@ USING (constraint_catalog, constraint_schema, constraint_name)
 WHERE tc.table_schema in ('public')
 ORDER BY tc.table_schema, tc.table_name, cc.constraint_name
 ----
-table_schema  table_name  constraint_name            check_clause
-public        t1          3864823197_118_1_not_null  p IS NOT NULL
-public        t1          c2                         ((a < 99:::INT8))
-public        t1          check_a                    ((a > 4:::INT8))
+table_schema  table_name  constraint_name     check_clause
+public        t1          117_118_1_not_null  p IS NOT NULL
+public        t1          c2                  ((a < 99:::INT8))
+public        t1          check_a             ((a > 4:::INT8))
 
 statement ok
 DROP DATABASE constraint_db CASCADE

pkg/sql/logictest/testdata/logic_test/orms

@@ -358,7 +358,7 @@ SELECT
   typdefaultbin
 FROM pg_type WHERE typname = 'regression_66576'
 ----
-regression_66576  4101115737  c  C  false  0  -1  0  -1  NULL
+regression_66576  105  c  C  false  0  -1  0  -1  NULL
 
 query T
 SELECT reltype FROM pg_class WHERE relname = 'regression_65576'