Skip to content

Commit db971b6

Browse files
jkosh44maddyblue
jkosh44
and
maddyblue
committed
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>
1 parent f242208 commit db971b6

File tree

6 files changed

+321
-22
lines changed

6 files changed

+321
-22
lines changed

‎src/adapter/src/catalog.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5383,6 +5383,10 @@ impl Catalog {
53835383
self.state.resolve_builtin_cluster(cluster)
53845384
}
53855385

5386+
pub fn get_mz_introspections_cluster_id(&self) -> &ClusterId {
5387+
&self.resolve_builtin_cluster(&MZ_INTROSPECTION_CLUSTER).id
5388+
}
5389+
53865390
/// Resolves a [`Cluster`] for a TargetCluster.
53875391
pub fn resolve_target_cluster(
53885392
&self,

‎src/adapter/src/coord/timeline.rs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -805,27 +805,27 @@ impl Coordinator {
805805
schemas.insert((&name.qualifiers.database_spec, &name.qualifiers.schema_spec));
806806
}
807807

808-
// If any of the system schemas is specified, add the rest of the
809-
// system schemas.
808+
// Always include all system schemas, if schemas is non-empty. System schemas are sometimes
809+
// used by applications in followup queries.
810810
let system_schemas = [
811811
(
812812
&ResolvedDatabaseSpecifier::Ambient,
813813
&SchemaSpecifier::Id(self.catalog().get_mz_catalog_schema_id().clone()),
814814
),
815815
(
816816
&ResolvedDatabaseSpecifier::Ambient,
817-
&SchemaSpecifier::Id(self.catalog().get_pg_catalog_schema_id().clone()),
817+
&SchemaSpecifier::Id(self.catalog().get_mz_internal_schema_id().clone()),
818818
),
819819
(
820820
&ResolvedDatabaseSpecifier::Ambient,
821-
&SchemaSpecifier::Id(self.catalog().get_information_schema_id().clone()),
821+
&SchemaSpecifier::Id(self.catalog().get_pg_catalog_schema_id().clone()),
822822
),
823823
(
824824
&ResolvedDatabaseSpecifier::Ambient,
825-
&SchemaSpecifier::Id(self.catalog().get_mz_internal_schema_id().clone()),
825+
&SchemaSpecifier::Id(self.catalog().get_information_schema_id().clone()),
826826
),
827827
];
828-
if system_schemas.iter().any(|s| schemas.contains(s)) {
828+
if !schemas.is_empty() {
829829
schemas.extend(system_schemas);
830830
}
831831

@@ -840,6 +840,12 @@ impl Coordinator {
840840
let mut id_bundle: CollectionIdBundle = self
841841
.index_oracle(compute_instance)
842842
.sufficient_collections(item_ids.iter());
843+
// Queries may get auto-routed to the mz_introspection cluster, so we include indexes on
844+
// that cluster.
845+
let system_id_bundle = self
846+
.index_oracle(*self.catalog().get_mz_introspections_cluster_id())
847+
.sufficient_collections(item_ids.iter());
848+
id_bundle.extend(&system_id_bundle);
843849

844850
// Filter out ids from different timelines.
845851
for ids in [

‎src/environmentd/tests/sql.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,6 +1316,11 @@ fn test_transactional_explain_timestamps() {
13161316
.batch_execute("CREATE TABLE t1 (i1 int)")
13171317
.unwrap();
13181318

1319+
client_writes.batch_execute("CREATE SCHEMA s").unwrap();
1320+
client_writes
1321+
.batch_execute("CREATE TABLE s.t2 (i2 int)")
1322+
.unwrap();
1323+
13191324
// Verify execution during a write-only txn fails
13201325
client_writes.batch_execute("BEGIN").unwrap();
13211326
client_writes
@@ -1370,10 +1375,7 @@ fn test_transactional_explain_timestamps() {
13701375

13711376
// Errors when an object outside the chosen time domain is referenced
13721377
let error = client_reads
1373-
.query_one(
1374-
"EXPLAIN TIMESTAMP FOR SELECT * FROM mz_catalog.mz_views;",
1375-
&[],
1376-
)
1378+
.query_one("EXPLAIN TIMESTAMP FOR SELECT * FROM s.t2;", &[])
13771379
.unwrap_err();
13781380

13791381
assert!(format!("{}", error)

‎test/sqllogictest/github-8241.slt‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,138 @@ DETAIL: The following relations in the query are outside the transaction's time
3939
"materialize.public.i2"
4040
Only the following relations are available:
4141
"materialize.public.i1"
42+
"materialize.public.t1"
43+
"mz_catalog.mz_array_types"
44+
"mz_catalog.mz_audit_events"
45+
"mz_catalog.mz_aws_privatelink_connections"
46+
"mz_catalog.mz_base_types"
47+
"mz_catalog.mz_cluster_replicas"
48+
"mz_catalog.mz_clusters"
49+
"mz_catalog.mz_columns"
50+
"mz_catalog.mz_connections"
51+
"mz_catalog.mz_databases"
52+
"mz_catalog.mz_default_privileges"
53+
"mz_catalog.mz_egress_ips"
54+
"mz_catalog.mz_functions"
55+
"mz_catalog.mz_index_columns"
56+
"mz_catalog.mz_indexes"
57+
"mz_catalog.mz_kafka_connections"
58+
"mz_catalog.mz_kafka_sinks"
59+
"mz_catalog.mz_list_types"
60+
"mz_catalog.mz_map_types"
61+
"mz_catalog.mz_materialized_views"
62+
"mz_catalog.mz_operators"
63+
"mz_catalog.mz_pseudo_types"
64+
"mz_catalog.mz_role_members"
65+
"mz_catalog.mz_roles"
66+
"mz_catalog.mz_schemas"
67+
"mz_catalog.mz_secrets"
68+
"mz_catalog.mz_sinks"
69+
"mz_catalog.mz_sources"
70+
"mz_catalog.mz_ssh_tunnel_connections"
71+
"mz_catalog.mz_system_privileges"
72+
"mz_catalog.mz_tables"
73+
"mz_catalog.mz_types"
74+
"mz_catalog.mz_views"
75+
"mz_internal.mz_active_peeks_per_worker_s2_primary_idx"
76+
"mz_internal.mz_active_peeks_per_worker_u1_primary_idx"
77+
"mz_internal.mz_aggregates"
78+
"mz_internal.mz_arrangement_batches_raw_s2_primary_idx"
79+
"mz_internal.mz_arrangement_batches_raw_u1_primary_idx"
80+
"mz_internal.mz_arrangement_heap_allocations_raw_s2_primary_idx"
81+
"mz_internal.mz_arrangement_heap_allocations_raw_u1_primary_idx"
82+
"mz_internal.mz_arrangement_heap_capacity_raw_s2_primary_idx"
83+
"mz_internal.mz_arrangement_heap_capacity_raw_u1_primary_idx"
84+
"mz_internal.mz_arrangement_heap_size_raw_s2_primary_idx"
85+
"mz_internal.mz_arrangement_heap_size_raw_u1_primary_idx"
86+
"mz_internal.mz_arrangement_records_raw_s2_primary_idx"
87+
"mz_internal.mz_arrangement_records_raw_u1_primary_idx"
88+
"mz_internal.mz_arrangement_sharing_raw_s2_primary_idx"
89+
"mz_internal.mz_arrangement_sharing_raw_u1_primary_idx"
90+
"mz_internal.mz_cluster_links"
91+
"mz_internal.mz_cluster_replica_heartbeats"
92+
"mz_internal.mz_cluster_replica_metrics"
93+
"mz_internal.mz_cluster_replica_metrics_ind"
94+
"mz_internal.mz_cluster_replica_sizes"
95+
"mz_internal.mz_cluster_replica_sizes_ind"
96+
"mz_internal.mz_cluster_replica_statuses"
97+
"mz_internal.mz_cluster_replica_statuses_ind"
98+
"mz_internal.mz_cluster_replicas_ind"
99+
"mz_internal.mz_clusters_ind"
100+
"mz_internal.mz_comments"
101+
"mz_internal.mz_compute_delays_histogram_raw_s2_primary_idx"
102+
"mz_internal.mz_compute_delays_histogram_raw_u1_primary_idx"
103+
"mz_internal.mz_compute_dependencies"
104+
"mz_internal.mz_compute_exports_per_worker_s2_primary_idx"
105+
"mz_internal.mz_compute_exports_per_worker_u1_primary_idx"
106+
"mz_internal.mz_compute_frontiers_per_worker_s2_primary_idx"
107+
"mz_internal.mz_compute_frontiers_per_worker_u1_primary_idx"
108+
"mz_internal.mz_compute_import_frontiers_per_worker_s2_primary_idx"
109+
"mz_internal.mz_compute_import_frontiers_per_worker_u1_primary_idx"
110+
"mz_internal.mz_compute_operator_durations_histogram_raw_s2_primary_idx"
111+
"mz_internal.mz_compute_operator_durations_histogram_raw_u1_primary_idx"
112+
"mz_internal.mz_dataflow_addresses_per_worker_s2_primary_idx"
113+
"mz_internal.mz_dataflow_addresses_per_worker_u1_primary_idx"
114+
"mz_internal.mz_dataflow_channels_per_worker_s2_primary_idx"
115+
"mz_internal.mz_dataflow_channels_per_worker_u1_primary_idx"
116+
"mz_internal.mz_dataflow_operator_reachability_raw_s2_primary_idx"
117+
"mz_internal.mz_dataflow_operator_reachability_raw_u1_primary_idx"
118+
"mz_internal.mz_dataflow_operators_per_worker_s2_primary_idx"
119+
"mz_internal.mz_dataflow_operators_per_worker_u1_primary_idx"
120+
"mz_internal.mz_dataflow_shutdown_durations_histogram_raw_s2_primary_idx"
121+
"mz_internal.mz_dataflow_shutdown_durations_histogram_raw_u1_primary_idx"
122+
"mz_internal.mz_frontiers"
123+
"mz_internal.mz_kafka_sources"
124+
"mz_internal.mz_message_batch_counts_received_raw_s2_primary_idx"
125+
"mz_internal.mz_message_batch_counts_received_raw_u1_primary_idx"
126+
"mz_internal.mz_message_batch_counts_sent_raw_s2_primary_idx"
127+
"mz_internal.mz_message_batch_counts_sent_raw_u1_primary_idx"
128+
"mz_internal.mz_message_counts_received_raw_s2_primary_idx"
129+
"mz_internal.mz_message_counts_received_raw_u1_primary_idx"
130+
"mz_internal.mz_message_counts_sent_raw_s2_primary_idx"
131+
"mz_internal.mz_message_counts_sent_raw_u1_primary_idx"
132+
"mz_internal.mz_object_dependencies"
133+
"mz_internal.mz_object_lifetimes_ind"
134+
"mz_internal.mz_peek_durations_histogram_raw_s2_primary_idx"
135+
"mz_internal.mz_peek_durations_histogram_raw_u1_primary_idx"
136+
"mz_internal.mz_postgres_sources"
137+
"mz_internal.mz_prepared_statement_history"
138+
"mz_internal.mz_scheduling_elapsed_raw_s2_primary_idx"
139+
"mz_internal.mz_scheduling_elapsed_raw_u1_primary_idx"
140+
"mz_internal.mz_scheduling_parks_histogram_raw_s2_primary_idx"
141+
"mz_internal.mz_scheduling_parks_histogram_raw_u1_primary_idx"
142+
"mz_internal.mz_session_history"
143+
"mz_internal.mz_sessions"
144+
"mz_internal.mz_show_all_objects_ind"
145+
"mz_internal.mz_show_cluster_replicas_ind"
146+
"mz_internal.mz_show_clusters_ind"
147+
"mz_internal.mz_show_columns_ind"
148+
"mz_internal.mz_show_connections_ind"
149+
"mz_internal.mz_show_databases_ind"
150+
"mz_internal.mz_show_indexes_ind"
151+
"mz_internal.mz_show_materialized_views_ind"
152+
"mz_internal.mz_show_schemas_ind"
153+
"mz_internal.mz_show_secrets_ind"
154+
"mz_internal.mz_show_sinks_ind"
155+
"mz_internal.mz_show_sources_ind"
156+
"mz_internal.mz_show_tables_ind"
157+
"mz_internal.mz_show_types_ind"
158+
"mz_internal.mz_show_views_ind"
159+
"mz_internal.mz_sink_statistics"
160+
"mz_internal.mz_sink_statistics_ind"
161+
"mz_internal.mz_sink_status_history"
162+
"mz_internal.mz_sink_status_history_ind"
163+
"mz_internal.mz_sink_statuses_ind"
164+
"mz_internal.mz_source_statistics"
165+
"mz_internal.mz_source_statistics_ind"
166+
"mz_internal.mz_source_status_history"
167+
"mz_internal.mz_source_status_history_ind"
168+
"mz_internal.mz_source_statuses_ind"
169+
"mz_internal.mz_statement_execution_history"
170+
"mz_internal.mz_storage_shards"
171+
"mz_internal.mz_storage_usage_by_shard"
172+
"mz_internal.mz_subscriptions"
173+
"mz_internal.mz_type_pg_metadata"
174+
"mz_internal.mz_view_foreign_keys"
175+
"mz_internal.mz_view_keys"
176+
"mz_internal.mz_webhook_sources"

‎test/sqllogictest/timedomain.slt‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,6 @@
99

1010
mode cockroach
1111

12-
# Since mz_now() is a custom function, the postgres client will look it up in the catalog on
13-
# first use. If the first use happens to be in a transaction, then we can get unexpected time
14-
# domain errors. This is an annoying hack to load the information in the postgres client before
15-
# we start any transactions.
16-
query T rowsort
17-
SELECT mz_now() LIMIT 0
18-
----
19-
2012
statement ok
2113
CREATE TABLE t (i INT);
2214

@@ -74,8 +66,7 @@ SELECT * FROM t;
7466
statement ok
7567
ROLLBACK
7668

77-
# Test that user table and system tables cannot be mixed in a transaction because they
78-
# belong to different timedomains.
69+
# Test that user table and system tables can be mixed in a transaction.
7970

8071
statement ok
8172
BEGIN;
@@ -84,11 +75,11 @@ query I rowsort
8475
SELECT * FROM t
8576
----
8677

87-
query error Transactions can only reference objects in the same timedomain.
78+
statement ok
8879
SELECT * FROM mz_views
8980

9081
statement ok
91-
ROLLBACK
82+
COMMIT
9283

9384
# Test that timeline dependent queries can be included in transaction.
9485

@@ -173,3 +164,30 @@ SELECT 1 FROM pg_attribute LIMIT 1
173164

174165
statement ok
175166
COMMIT
167+
168+
# Verify that system tables are always included in read txns, even if not
169+
# mentioned in the first query.
170+
simple
171+
BEGIN;
172+
SELECT * FROM t;
173+
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;
174+
COMMIT;
175+
----
176+
COMPLETE 0
177+
COMPLETE 0
178+
t,pg_catalog,record
179+
COMPLETE 1
180+
COMPLETE 0
181+
182+
simple
183+
BEGIN;
184+
SELECT row(1, 2);
185+
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;
186+
COMMIT;
187+
----
188+
COMPLETE 0
189+
(1,2)
190+
COMPLETE 1
191+
t,pg_catalog,record
192+
COMPLETE 1
193+
COMPLETE 0

0 commit comments

Comments
 (0)