Follow the error msg style guide

MaterializeInc · Jan 4, 2024 · 99e64b4 · 99e64b4
1 parent b49aef0
commit 99e64b4
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/src/adapter/src/coord/sequencer/inner/create_materialized_view.rs b/src/adapter/src/coord/sequencer/inner/create_materialized_view.rs
@@ -290,9 +290,12 @@ impl Coordinator {
             // `as_of` to the first refresh. This is because we'd like queries on the MV to
             // block until the first refresh (rather than to show an empty MV).
             if let Some(refresh_schedule) = &refresh_schedule {
-                if let Some(ts) = as_of.as_option() {
-                    let Some(rounded_up_ts) = refresh_schedule.round_up_timestamp(*ts) else {
-                        return Err(AdapterError::MaterializedViewWouldNeverRefresh(name.item));
+                if let Some(as_of_ts) = as_of.as_option() {
+                    let Some(rounded_up_ts) = refresh_schedule.round_up_timestamp(*as_of_ts) else {
+                        return Err(AdapterError::MaterializedViewWouldNeverRefresh(
+                            refresh_schedule.last_refresh().expect("if round_up_timestamp returned None, then there should be a last refresh"),
+                            *as_of_ts
+                        ));
                     };
                     as_of = Antichain::from_elem(rounded_up_ts);
                 } else {

diff --git a/src/adapter/src/error.rs b/src/adapter/src/error.rs
@@ -22,7 +22,7 @@ use mz_ore::str::StrExt;
 use mz_pgwire_common::{ErrorResponse, Severity};
 use mz_repr::adt::timestamp::TimestampError;
 use mz_repr::explain::ExplainError;
-use mz_repr::NotNullViolation;
+use mz_repr::{NotNullViolation, Timestamp};
 use mz_sql::plan::PlanError;
 use mz_sql::rbac;
 use mz_sql::session::vars::VarError;
@@ -218,7 +218,7 @@ pub enum AdapterError {
     /// We refuse to create the materialized view, because it would never be refreshed, so it would
     /// never be queryable. This can happen when the only specified refreshes are further back in
     /// the past than the initial compaction window of the materialized view.
-    MaterializedViewWouldNeverRefresh(String),
+    MaterializedViewWouldNeverRefresh(Timestamp, Timestamp),
 }
 
 impl AdapterError {
@@ -305,6 +305,14 @@ impl AdapterError {
             AdapterError::InvalidAlter(_, e) => e.detail(),
             AdapterError::Optimizer(e) => e.detail(),
             AdapterError::ConnectionValidation(e) => e.detail(),
+            AdapterError::MaterializedViewWouldNeverRefresh(last_refresh, earliest_possible) => {
+                Some(format!(
+                    "The specified last refresh is at {}, while the earliest possible time to compute the materialized \
+                    view is {}.",
+                    last_refresh,
+                    earliest_possible,
+                ))
+            }
             _ => None,
         }
     }
@@ -481,7 +489,7 @@ impl AdapterError {
             AdapterError::InvalidAlter(_, _) => SqlState::FEATURE_NOT_SUPPORTED,
             AdapterError::ConnectionValidation(_) => SqlState::SYSTEM_ERROR,
             // `DATA_EXCEPTION`, similarly to `AbsurdSubscribeBounds`.
-            AdapterError::MaterializedViewWouldNeverRefresh(_) => SqlState::DATA_EXCEPTION,
+            AdapterError::MaterializedViewWouldNeverRefresh(_, _) => SqlState::DATA_EXCEPTION,
         }
     }
 
@@ -695,12 +703,11 @@ impl fmt::Display for AdapterError {
                 write!(f, "invalid ALTER {t}: {e}")
             }
             AdapterError::ConnectionValidation(e) => e.fmt(f),
-            AdapterError::MaterializedViewWouldNeverRefresh(name) => {
+            AdapterError::MaterializedViewWouldNeverRefresh(_, _) => {
                 write!(
                     f,
-                    "Error when creating materialized view {}: All the specified refreshes \
-                    would be too far in the past, and thus they would never happen",
-                    name
+                    "all the specified refreshes of the materialized view would be too far in the past, and thus they \
+                    would never happen"
                 )
             }
         }

diff --git a/test/sqllogictest/materialized_views.slt b/test/sqllogictest/materialized_views.slt
@@ -962,7 +962,7 @@ SELECT * FROM const_mv2
 2
 
 ## MV that has refreshes only in the past
-query error db error: ERROR: Error when creating materialized view mv_no_refresh: All the specified refreshes would be too far in the past, and thus they would never happen
+query error db error: ERROR: all the specified refreshes of the materialized view would be too far in the past, and thus they would never happen
 CREATE MATERIALIZED VIEW mv_no_refresh
 WITH (REFRESH AT '2000-01-01 10:00')
 AS SELECT * FROM t2;