Merge #148056

148056: upgrades,systemschema: add new column for login time to system.users r=spilchen,pritesh-lahoti a=souravcrl

- adds an 'estimated_last_login_time' column with the `TIMESTAMPTZ` type. This new
column will be used to store the last login timestamp of authenticating users to
the database. We plan to populate this asynchronously and with absence of retry
mechanisms inorder to reduce latency for performing a single login attempt which
could result in updation of the column frequently. This work will be taken up
subsequently.

- validated the system.users table post migration on local cluster
```
root@localhost:26257/defaultdb> select * from system.users;                                                                                                                                              
  username | hashedPassword | isRole | user_id | estimated_last_login_time
-----------+----------------+--------+---------+----------------------------
  admin    | \x             |   t    |       2 | NULL
  root     | \x             |   f    |       1 | NULL
(2 rows)

Time: 3ms total (execution 2ms / network 1ms)
```

informs #147602
fixes #147600
Epic: CRDB-21590
Release note: None

Co-authored-by: souravcrl <[email protected]>

Author: craig[bot]

docs/generated/settings/settings-for-tenants.txt

@@ -417,4 +417,4 @@ trace.zipkin.collector	string		the address of a Zipkin instance to receive trace
 ui.database_locality_metadata.enabled	boolean	true	if enabled shows extended locality data about databases and tables in DB Console which can be expensive to compute	application
 ui.default_timezone	string		the default timezone used to format timestamps in the ui	application
 ui.display_timezone	enumeration	etc/utc	the timezone used to format timestamps in the ui. This setting is deprecatedand will be removed in a future version. Use the 'ui.default_timezone' setting instead. 'ui.default_timezone' takes precedence over this setting. [etc/utc = 0, america/new_york = 1]	application
-version	version	1000025.2-upgrading-to-1000025.3-step-004	set the active cluster version in the format '<major>.<minor>'	application
+version	version	1000025.2-upgrading-to-1000025.3-step-006	set the active cluster version in the format '<major>.<minor>'	application

docs/generated/settings/settings.html

@@ -375,6 +375,6 @@
 <tr><td><div id="setting-ui-database-locality-metadata-enabled" class="anchored"><code>ui.database_locality_metadata.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if enabled shows extended locality data about databases and tables in DB Console which can be expensive to compute</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-default-timezone" class="anchored"><code>ui.default_timezone</code></div></td><td>string</td><td><code></code></td><td>the default timezone used to format timestamps in the ui</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-display-timezone" class="anchored"><code>ui.display_timezone</code></div></td><td>enumeration</td><td><code>etc/utc</code></td><td>the timezone used to format timestamps in the ui. This setting is deprecatedand will be removed in a future version. Use the &#39;ui.default_timezone&#39; setting instead. &#39;ui.default_timezone&#39; takes precedence over this setting. [etc/utc = 0, america/new_york = 1]</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000025.2-upgrading-to-1000025.3-step-004</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000025.2-upgrading-to-1000025.3-step-006</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 </tbody>
 </table>

pkg/backup/full_cluster_backup_restore_test.go

@@ -1190,28 +1190,28 @@ func TestFullClusterRestoreWithUserIDs(t *testing.T) {
 	sqlDB.Exec(t, `BACKUP INTO $1`, localFoo)
 
 	sqlDB.CheckQueryResults(t, `SELECT * FROM system.users ORDER BY user_id`, [][]string{
-		{"root", "", "false", "1"},
-		{"admin", "", "true", "2"},
-		{"test1", "NULL", "false", "100"},
-		{"test2", "NULL", "false", "101"},
+		{"root", "", "false", "1", "NULL"},
+		{"admin", "", "true", "2", "NULL"},
+		{"test1", "NULL", "false", "100", "NULL"},
+		{"test2", "NULL", "false", "101", "NULL"},
 	})
 	// Ensure that the new backup succeeds.
 	sqlDBRestore.Exec(t, `RESTORE FROM LATEST IN $1`, localFoo)
 
 	sqlDBRestore.CheckQueryResults(t, `SELECT * FROM system.users ORDER BY user_id`, [][]string{
-		{"root", "", "false", "1"},
-		{"admin", "", "true", "2"},
-		{"test1", "NULL", "false", "100"},
-		{"test2", "NULL", "false", "101"},
+		{"root", "", "false", "1", "NULL"},
+		{"admin", "", "true", "2", "NULL"},
+		{"test1", "NULL", "false", "100", "NULL"},
+		{"test2", "NULL", "false", "101", "NULL"},
 	})
 
 	sqlDBRestore.Exec(t, `CREATE USER test3`)
 
 	sqlDBRestore.CheckQueryResults(t, `SELECT * FROM system.users ORDER BY user_id`, [][]string{
-		{"root", "", "false", "1"},
-		{"admin", "", "true", "2"},
-		{"test1", "NULL", "false", "100"},
-		{"test2", "NULL", "false", "101"},
-		{"test3", "NULL", "false", "102"},
+		{"root", "", "false", "1", "NULL"},
+		{"admin", "", "true", "2", "NULL"},
+		{"test1", "NULL", "false", "100", "NULL"},
+		{"test2", "NULL", "false", "101", "NULL"},
+		{"test3", "NULL", "false", "102", "NULL"},
 	})
 }

pkg/cli/doctor_test.go

@@ -157,7 +157,7 @@ func TestDoctorCluster(t *testing.T) {
 		"CREATE TABLE to_drop (id INT)",
 		"DROP TABLE to_drop",
 		"CREATE TABLE foo (id INT)",
-		"INSERT INTO system.users VALUES ('node', NULL, true, 3)",
+		"INSERT INTO system.users VALUES ('node', NULL, true, 3, NULL)",
 		"GRANT node TO root",
 		"DELETE FROM system.namespace WHERE name = 'foo'",
 		"SELECT pg_catalog.pg_sleep(1)",
@@ -222,7 +222,7 @@ func TestDoctorClusterDropped(t *testing.T) {
 	c.RunWithArgs([]string{"sql", "-e", strings.Join([]string{
 		"CREATE TABLE foo (i INT)",
 		desc,
-		"INSERT INTO system.users VALUES ('node', NULL, true, 3)",
+		"INSERT INTO system.users VALUES ('node', NULL, true, 3, NULL)",
 		"GRANT node TO root",
 		"DELETE FROM system.namespace WHERE name = 'foo'",
 		"SELECT pg_catalog.pg_sleep(1)",

pkg/clusterversion/cockroach_versions.go

@@ -216,6 +216,7 @@ const (
 
 	V25_3_AddEventLogColumnAndIndex
 
+	V25_3_AddEstimatedLastLoginTime
 	// *************************************************
 	// Step (1) Add new versions above this comment.
 	// Do not add new versions to a patch release.
@@ -272,6 +273,8 @@ var versionTable = [numKeys]roachpb.Version{
 
 	V25_3_AddEventLogColumnAndIndex: {Major: 25, Minor: 2, Internal: 4},
 
+	V25_3_AddEstimatedLastLoginTime: {Major: 25, Minor: 2, Internal: 6},
+
 	// *************************************************
 	// Step (2): Add new versions above this comment.
 	// Do not add new versions to a patch release.