sql: Include system indexes in read transactions

cf1271c claimed to have already
completed this, however, due to an unfortunate git typo the original
contents of that commit were replaced with a small refactor. This
commit applies the original contents of
cf1271c. The original commit message
is copied below.

System views from pg_catalog and mz_catalog are sometimes used by
applications (for example Metabase if it doesn't recognize a data type)
after the first query of a transaction. This previously would cause a
timedomain error. One solution to this is to always include system indexes
in read transactions. The cost is mostly that system indexes can now be
held back from compaction more than previously. This seems ok because
they don't change quickly, so should hopefully not cause memory issues.

Fixes #9375
Fixes #9374
Fixes #21815

Co-authored-by: Matt Jibson <matt.jibson@gmail.com>

src/adapter/src/catalog.rs

@@ -5383,6 +5383,10 @@ impl Catalog {
         self.state.resolve_builtin_cluster(cluster)
     }
 
+    pub fn get_mz_introspections_cluster_id(&self) -> &ClusterId {
+        &self.resolve_builtin_cluster(&MZ_INTROSPECTION_CLUSTER).id
+    }
+
     /// Resolves a [`Cluster`] for a TargetCluster.
     pub fn resolve_target_cluster(
         &self,

src/adapter/src/coord/timeline.rs

@@ -805,27 +805,27 @@ impl Coordinator {
             schemas.insert((&name.qualifiers.database_spec, &name.qualifiers.schema_spec));
         }
 
-        // If any of the system schemas is specified, add the rest of the
-        // system schemas.
+        // Always include all system schemas, if schemas is non-empty. System schemas are sometimes
+        // used by applications in followup queries.
         let system_schemas = [
             (
                 &ResolvedDatabaseSpecifier::Ambient,
                 &SchemaSpecifier::Id(self.catalog().get_mz_catalog_schema_id().clone()),
             ),
             (
                 &ResolvedDatabaseSpecifier::Ambient,
-                &SchemaSpecifier::Id(self.catalog().get_pg_catalog_schema_id().clone()),
+                &SchemaSpecifier::Id(self.catalog().get_mz_internal_schema_id().clone()),
             ),
             (
                 &ResolvedDatabaseSpecifier::Ambient,
-                &SchemaSpecifier::Id(self.catalog().get_information_schema_id().clone()),
+                &SchemaSpecifier::Id(self.catalog().get_pg_catalog_schema_id().clone()),
             ),
             (
                 &ResolvedDatabaseSpecifier::Ambient,
-                &SchemaSpecifier::Id(self.catalog().get_mz_internal_schema_id().clone()),
+                &SchemaSpecifier::Id(self.catalog().get_information_schema_id().clone()),
             ),
         ];
-        if system_schemas.iter().any(|s| schemas.contains(s)) {
+        if !schemas.is_empty() {
             schemas.extend(system_schemas);
         }
 
@@ -840,6 +840,12 @@ impl Coordinator {
         let mut id_bundle: CollectionIdBundle = self
             .index_oracle(compute_instance)
             .sufficient_collections(item_ids.iter());
+        // Queries may get auto-routed to the mz_introspection cluster, so we include indexes on
+        // that cluster.
+        let system_id_bundle = self
+            .index_oracle(*self.catalog().get_mz_introspections_cluster_id())
+            .sufficient_collections(item_ids.iter());
+        id_bundle.extend(&system_id_bundle);
 
         // Filter out ids from different timelines.
         for ids in [

test/sqllogictest/timedomain.slt

@@ -9,14 +9,6 @@
 
 mode cockroach
 
-# Since mz_now() is a custom function, the postgres client will look it up in the catalog on
-# first use. If the first use happens to be in a transaction, then we can get unexpected time
-# domain errors. This is an annoying hack to load the information in the postgres client before
-# we start any transactions.
-query T rowsort
-SELECT mz_now() LIMIT 0
-----
-
 statement ok
 CREATE TABLE t (i INT);
 
@@ -74,8 +66,7 @@ SELECT * FROM t;
 statement ok
 ROLLBACK
 
-# Test that user table and system tables cannot be mixed in a transaction because they
-# belong to different timedomains.
+# Test that user table and system tables can be mixed in a transaction.
 
 statement ok
 BEGIN;
@@ -84,11 +75,11 @@ query I rowsort
 SELECT * FROM t
 ----
 
-query error Transactions can only reference objects in the same timedomain.
+statement ok
 SELECT * FROM mz_views
 
 statement ok
-ROLLBACK
+COMMIT
 
 # Test that timeline dependent queries can be included in transaction.
 
@@ -173,3 +164,30 @@ SELECT 1 FROM pg_attribute LIMIT 1
 
 statement ok
 COMMIT
+
+# Verify that system tables are always included in read txns, even if not
+# mentioned in the first query.
+simple
+BEGIN;
+SELECT * FROM t;
+SELECT n.nspname = ANY(current_schemas(true)), n.nspname, t.typname FROM pg_catalog.pg_type t JOIN pg_catalog.pg_namespace n ON t.typnamespace = n.oid WHERE t.oid = 2249;
+COMMIT;
+----
+COMPLETE 0
+COMPLETE 0
+t,pg_catalog,record
+COMPLETE 1
+COMPLETE 0
+
+simple
+BEGIN;
+SELECT row(1, 2);
+SELECT n.nspname = ANY(current_schemas(true)), n.nspname, t.typname FROM pg_catalog.pg_type t JOIN pg_catalog.pg_namespace n ON t.typnamespace = n.oid WHERE t.oid = 2249;
+COMMIT;
+----
+COMPLETE 0
+(1,2)
+COMPLETE 1
+t,pg_catalog,record
+COMPLETE 1
+COMPLETE 0