Merge #86055 #86193 #86336 #86349 #86397

86055: sql: add index recommendation to execution_insights table r=j82w a=j82w

Adds index recommendation to execution_insights table
part of #81024

Release justification: Category 2: Bug fixes and
low-risk updates to new functionality

Release note (sql change): Adds index recommendation
to execution_insights

86193: opt: use only required columns in provided ordering for project r=DrewKimball a=DrewKimball

Project operators can only pass through their input ordering.
However, the provided ordering may have to be remapped in order to
ensure it only refers to output columns, since the `Project` can add
and remove columns. The `Project` uses its `internalFDs` field to
accomplish the remapping; these are constructed when the `Project`
is added to the memo by combining the functional dependencies of the
input and the projections.

The problem occurs when transformation rules cause the input of the
`Project` to "reveal" additional functional dependencies. For example,
one side of a union may be eliminated and the FDs of the remaining side
used in the result. This can cause the `Project` to output an ordering
that is equivalent to the required ordering according to its own FDs,
but which a parent operator cannot tell is equivalent because its FDs
were calculated before the tranformation rule fired. This can cause
panics later down the line when the provided ordering does not match
up with the required ordering.

In the following example, an exploration rule transforms the join into
two joins unioned together, one over each disjunct. After the
transformation, a normalization rule fires that removes the
`t0.rowid IS NULL` side because rowids are non-null. This reveals the
`t1.rowid = t0.rowid` FD, which later causes `t0.rowid` to be used in
a provided ordering rather than `t1.rowid`. For the reasons mentioned
above, this later causes a panic when a `Project` attempts to remap to
the required `t1.rowid` ordering.
```
CREATE TABLE t0 (c0 INT);
CREATE TABLE t1 (c0 INT);

SELECT * FROM t0 CROSS JOIN t1
WHERE (t0.rowid IS NULL) OR (t1.rowid IN (t0.rowid))
ORDER BY t1.rowid;
```

This commit prevents the panic by making `Project` operators remap the
input provided ordering to use columns from the required ordering
(which are a subset of the output columns). This prevents the disparity
between required and provided orderings that can cause panics down the
line. In the example given above, the `t1.rowid` column would be chosen
for the provided ordering because it is in the required ordering.

Fixes #85393

Release note (bug fix): fixed a vulnerability in the optimizer that could
cause a panic in rare cases when planning complex queries with `ORDER BY`.

Release justification: fixes release blocker

86336: externalconn: grant `ALL` on `CREATE EXTERNAL CONNECTION` r=adityamaru a=adityamaru

Previously, the user that created the external connection
was not granted any privileges as the creator/owner of the
object. This diff changes this by granting the user `ALL`
privileges.

When synthetic privileges have a concept of owners, we should
also set the owner of the External Connection object to the
creator. This can happen once #86181
is addressed.

There was also a small bug in the grant/revoke code where
external connection names that required to be wrapped in
`""` egs: `"foo-kms"` were being incorrectly stringified when
creating the synthetic privilege path. This resulted in:

```
CREATE EXTERNAL CONNECTION "foo-kms" AS ...
GRANT USAGE ON EXTERNAL CONNECTION "foo-kms" TO baz
```

To fail with a "foo-kms" cannot be found, even though it was
infact created before attempting the grant. Tests have been added to
test this case.

Fixes: #86261

Release note (bug fix): Users that create an external connection
are now granted `ALL` privileges on the object.

Release justification: bug fix for new functionality

86349: ttl: replace SPLIT AT with SplitTable in tests r=otan a=ecwall

refs #85800
fixes #86376

SPLIT AT was causing occasional test failures if it did not asynchronously
split the range before the TTL job started. SplitTable synchronously splits
the range before the test starts the TTL job.

Release justification: Fix test flake.

Release note: None

86397: cluster-ui: added internal prefix to transaction workload insights view r=ericharmeling a=ericharmeling

Previously, the connected component for transaction workload insights was
using the selector for the sessions data slice to load the internal app
name prefix. This isn't necessary; we can just use a constant, as is done
on other pages.

Fixes #86250.

https://www.loom.com/share/18bf4edf46fb4b1dad2fe32f9d285dcd

Release justification: bug fixes and low-risk updates to new functionality

Release note: None

Co-authored-by: j82w <[email protected]>
Co-authored-by: DrewKimball <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>

pkg/ccl/backupccl/testdata/backup-restore/external-connections-privileges

@@ -12,7 +12,7 @@ CREATE EXTERNAL CONNECTION root AS 'nodelocal://1/root'
 ----
 
 exec-sql user=testuser
-CREATE EXTERNAL CONNECTION fails AS 'userfile:///noprivs'
+CREATE EXTERNAL CONNECTION "testuser-ec" AS 'userfile:///noprivs'
 ----
 pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
 
@@ -21,7 +21,7 @@ GRANT SYSTEM EXTERNALCONNECTION TO testuser;
 ----
 
 exec-sql user=testuser
-CREATE EXTERNAL CONNECTION fails AS 'nodelocal://1/privs'
+CREATE EXTERNAL CONNECTION "testuser-ec" AS 'nodelocal://1/privs'
 ----
 
 exec-sql
@@ -32,17 +32,17 @@ exec-sql
 GRANT SELECT ON TABLE foo TO testuser
 ----
 
+# Since testuser created the External Connection they have `ALL` privileges on the object.
 exec-sql user=testuser
-BACKUP TABLE foo INTO 'external://fails'
+BACKUP TABLE foo INTO 'external://testuser-ec'
 ----
-pq: user testuser does not have USAGE privilege on external_connection fails
 
 exec-sql
-GRANT USAGE ON EXTERNAL CONNECTION fails TO testuser;
+GRANT USAGE ON EXTERNAL CONNECTION "testuser-ec" TO testuser;
 ----
 
 exec-sql user=testuser
-BACKUP TABLE foo INTO 'external://fails'
+BACKUP TABLE foo INTO LATEST IN 'external://testuser-ec'
 ----
 
 # Sanity check that the user can't write to any other external connection.
@@ -51,18 +51,34 @@ BACKUP TABLE foo INTO 'external://root'
 ----
 pq: user testuser does not have USAGE privilege on external_connection root
 
-# Revoke the USAGE privilege to test that restore also requires it.
+query-sql
+SELECT * FROM system.privileges
+----
+root /externalconn/root {ALL} {}
+testuser /externalconn/testuser-ec {ALL} {}
+testuser /global/ {EXTERNALCONNECTION} {}
+
+# Revoke the USAGE privilege. Note testuser had ALL privileges since they
+# created the External Connection, but revoking USAGE means that they will now
+# only have DROP privileges. Thus, they shouldn't be able to restore.
 exec-sql
-REVOKE USAGE ON EXTERNAL CONNECTION fails FROM testuser;
+REVOKE USAGE ON EXTERNAL CONNECTION "testuser-ec" FROM testuser;
+----
+
+query-sql
+SELECT * FROM system.privileges
 ----
+root /externalconn/root {ALL} {}
+testuser /externalconn/testuser-ec {DROP} {}
+testuser /global/ {EXTERNALCONNECTION} {}
 
 exec-sql user=testuser
-RESTORE TABLE foo FROM LATEST IN 'external://fails'
+RESTORE TABLE foo FROM LATEST IN 'external://testuser-ec'
 ----
-pq: user testuser does not have USAGE privilege on external_connection fails
+pq: user testuser does not have USAGE privilege on external_connection testuser-ec
 
 exec-sql
-GRANT USAGE ON EXTERNAL CONNECTION fails TO testuser;
+GRANT USAGE ON EXTERNAL CONNECTION "testuser-ec" TO testuser;
 ----
 
 exec-sql
@@ -71,7 +87,7 @@ GRANT CREATE ON DATABASE failsdb TO testuser;
 ----
 
 exec-sql user=testuser
-RESTORE TABLE foo FROM LATEST IN 'external://fails' WITH into_db=failsdb;
+RESTORE TABLE foo FROM LATEST IN 'external://testuser-ec' WITH into_db=failsdb;
 ----
 
 subtest end

pkg/ccl/cloudccl/externalconn/BUILD.bazel

@@ -10,6 +10,7 @@ go_test(
     data = glob(["testdata/**"]),
     deps = [
         "//pkg/base",
+        "//pkg/ccl/backupccl",
         "//pkg/ccl/changefeedccl",
         "//pkg/ccl/kvccl/kvtenantccl",
         "//pkg/cloud/externalconn",

pkg/ccl/cloudccl/externalconn/datadriven_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	_ "github.com/cockroachdb/cockroach/pkg/ccl/backupccl"
 	_ "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl"
 	"github.com/cockroachdb/cockroach/pkg/cloud/externalconn"
 	_ "github.com/cockroachdb/cockroach/pkg/cloud/externalconn/providers" // register all the concrete External Connection implementations

pkg/ccl/cloudccl/externalconn/testdata/create_drop_external_connection

@@ -23,7 +23,7 @@ pq: failed to construct External Connection details: failed to create nodelocal
 exec-sql
 CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo';
 ----
-pq: external connection with connection name 'foo' already exists
+pq: failed to create external connection: external connection with connection name 'foo' already exists
 
 # Create another External Connection with a unique name.
 exec-sql
@@ -57,89 +57,6 @@ inspect-system-table
 
 subtest end
 
-subtest create-external-connection-global-privilege
-
-exec-sql
-CREATE USER testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-exec-sql
-GRANT SYSTEM EXTERNALCONNECTION TO testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION privileged;
-----
-
-exec-sql
-REVOKE SYSTEM EXTERNALCONNECTION FROM testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-subtest end
-
-subtest drop-external-storage-privilege
-
-exec-sql
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-# Create another External Connection.
-exec-sql
-CREATE EXTERNAL CONNECTION 'privileged-dup' AS 'nodelocal://1/foo'
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-pq: user testuser does not have DROP privilege on external_connection privileged
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-GRANT DROP ON EXTERNAL CONNECTION privileged TO testuser;
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-
-# Try to drop the second external connection, testuser should be disallowed.
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-pq: user testuser does not have DROP privilege on external_connection privileged-dup
-
-inspect-system-table
-----
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-
-subtest end
-
 subtest basic-gcp-kms
 
 disable-check-kms

pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/create_drop_external_connection

@@ -26,7 +26,7 @@ pq: failed to construct External Connection details: failed to create nodelocal
 exec-sql
 CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo';
 ----
-pq: external connection with connection name 'foo' already exists
+pq: failed to create external connection: external connection with connection name 'foo' already exists
 
 # Create another External Connection with a unique name.
 exec-sql
@@ -60,89 +60,6 @@ inspect-system-table
 
 subtest end
 
-subtest create-external-connection-global-privilege
-
-exec-sql
-CREATE USER testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-exec-sql
-GRANT SYSTEM EXTERNALCONNECTION TO testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION privileged;
-----
-
-exec-sql
-REVOKE SYSTEM EXTERNALCONNECTION FROM testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-subtest end
-
-subtest drop-external-storage-privilege
-
-exec-sql
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-# Create another External Connection.
-exec-sql
-CREATE EXTERNAL CONNECTION 'privileged-dup' AS 'nodelocal://1/foo'
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-pq: user testuser does not have DROP privilege on external_connection privileged
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-GRANT DROP ON EXTERNAL CONNECTION privileged TO testuser;
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-
-# Try to drop the second external connection, testuser should be disallowed.
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-pq: user testuser does not have DROP privilege on external_connection privileged-dup
-
-inspect-system-table
-----
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-
-subtest end
-
 subtest basic-gs-kms
 
 disable-check-kms