From c9d7737fbcb81c0a96d7686bdc570b1e9990fbcf Mon Sep 17 00:00:00 2001
From: Rafi Shamim <rafi@cockroachlabs.com>
Date: Thu, 20 Jul 2023 22:39:09 -0400
Subject: [PATCH] sql: small fixes to nameconcatoid

There was an edge case if the result was exactly 64 characters where the
input would not be truncated correctly. This also makes the
information_schema.role_routine_grants table use the same logic for
computing the specific_name.

Release note: None
---
 pkg/sql/information_schema.go                  | 18 ++++++++++++++++--
 .../logictest/testdata/logic_test/pg_builtins  |  5 +++++
 pkg/sql/sem/builtins/pg_builtins.go            |  2 +-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/pkg/sql/information_schema.go b/pkg/sql/information_schema.go
index d09d2bf528d6..7e8e7f1973c7 100644
--- a/pkg/sql/information_schema.go
+++ b/pkg/sql/information_schema.go
@@ -1774,7 +1774,7 @@ var informationSchemaRoleRoutineGrantsTable = virtualSchemaTable{
 
 				_, overloads := builtinsregistry.GetBuiltinProperties(name)
 				for _, o := range overloads {
-					fnSpecificName := tree.NewDString(fmt.Sprintf("%s_%d", fnNameStr, o.Oid))
+					fnSpecificName := tree.NewDString(nameConcatOid(fnNameStr, o.Oid))
 					for _, grantee := range roleNameForBuiltins {
 						if err := addRow(
 							tree.DNull, // grantor
@@ -1816,7 +1816,8 @@ var informationSchemaRoleRoutineGrantsTable = virtualSchemaTable{
 						return err
 					}
 					scNameStr := tree.NewDString(sc.GetName())
-					fnSpecificName := tree.NewDString(fmt.Sprintf("%s_%d", fn.GetName(), catid.FuncIDToOID(fn.GetID())))
+
+					fnSpecificName := tree.NewDString(nameConcatOid(fn.GetName(), catid.FuncIDToOID(fn.GetID())))
 					fnName := tree.NewDString(fn.GetName())
 					for _, u := range privs {
 						userNameStr := tree.NewDString(u.User.Normalized())
@@ -2955,3 +2956,16 @@ func userCanSeeDescriptor(
 func descriptorIsVisible(desc catalog.Descriptor, allowAdding bool) bool {
 	return desc.Public() || (allowAdding && desc.Adding())
 }
+
+// nameConcatOid is a Go version of the nameconcatoid builtin function. The
+// result is the same as fmt.Sprintf("%s_%d", s, o) except that, if it would not
+// fit in 63 characters, we make it do so by truncating the name input (not the
+// oid).
+func nameConcatOid(s string, o oid.Oid) string {
+	const maxLen = 63
+	oidStr := strconv.Itoa(int(o))
+	if len(s)+1+len(oidStr) <= maxLen {
+		return s + "_" + oidStr
+	}
+	return s[:maxLen-1-len(oidStr)] + "_" + oidStr
+}
diff --git a/pkg/sql/logictest/testdata/logic_test/pg_builtins b/pkg/sql/logictest/testdata/logic_test/pg_builtins
index 9177db06ec4f..74d1658205e9 100644
--- a/pkg/sql/logictest/testdata/logic_test/pg_builtins
+++ b/pkg/sql/logictest/testdata/logic_test/pg_builtins
@@ -882,4 +882,9 @@ select nameconcatoid(repeat('a', 58) || 'bbbbbbbbbb', 2);
 ----
 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbb_2
 
+query TI
+select nameconcatoid(repeat('a', 62), 2), length(nameconcatoid(repeat('a', 62), 2))
+----
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_2  63
+
 subtest end
diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go
index 0b54d1d1f101..8462694f78c4 100644
--- a/pkg/sql/sem/builtins/pg_builtins.go
+++ b/pkg/sql/sem/builtins/pg_builtins.go
@@ -2191,7 +2191,7 @@ var pgBuiltins = map[string]builtinDefinition{
 			ReturnType: tree.FixedReturnType(types.Name),
 			Body: `
 SELECT
-  CASE WHEN length($1::text || '_' || $2::text) > 64
+  CASE WHEN length($1::text || '_' || $2::text) > 63
 	THEN (substring($1 from 1 for 63 - length($2::text) - 1) || '_' || $2::text)::name
 	ELSE ($1::text || '_' || $2::text)::name
 	END