Change all references in user-facing error messages to point to discu…

…ssions instead of issues, or remove the issue reference if it wasn't useful anymore
MaterializeInc · Sep 17, 2024 · aad66ff · aad66ff
1 parent 11ea1a7
commit aad66ff
Show file tree

Hide file tree

Showing 20 changed files with 85 additions and 57 deletions.
diff --git a/src/adapter/src/coord/sequencer/inner.rs b/src/adapter/src/coord/sequencer/inner.rs
@@ -4442,7 +4442,7 @@ impl Coordinator {
     ) -> Result<ExecuteResponse, AdapterError> {
         Err(AdapterError::PlanError(plan::PlanError::Unsupported {
             feature: "ALTER TABLE ... ADD COLUMN ...".to_string(),
-            issue_no: Some(28082),
+            discussion_no: Some(29607),
         }))
     }
 }

diff --git a/src/adapter/src/error.rs b/src/adapter/src/error.rs
@@ -251,8 +251,7 @@ impl AdapterError {
     pub fn detail(&self) -> Option<String> {
         match self {
             AdapterError::AmbiguousSystemColumnReference => {
-                Some("This is a limitation in Materialize that will be lifted in a future release. \
-                See https://github.com/MaterializeInc/materialize/issues/16650 for details.".to_string())
+                Some("This is a current limitation in Materialize".into())
             },
             AdapterError::Catalog(c) => c.detail(),
             AdapterError::Eval(e) => e.detail(),

diff --git a/src/catalog/src/memory/objects.rs b/src/catalog/src/memory/objects.rs
@@ -390,7 +390,7 @@ impl Cluster {
                 // them before we have to implement this.
                 return Err(PlanError::Unsupported {
                     feature: "SHOW CREATE for unmanaged clusters".to_string(),
-                    issue_no: Some(15435),
+                    discussion_no: None,
                 });
             }
         };

diff --git a/src/environmentd/src/http/sql.rs b/src/environmentd/src/http/sql.rs
@@ -1295,27 +1295,46 @@ async fn execute_stmt<S: ResultSender>(
             )
             .into()
         }
-        ExecuteResponse::SendingRows { future: mut rows, instance_id, strategy } => {
+        ExecuteResponse::SendingRows {
+            future: mut rows,
+            instance_id,
+            strategy,
+        } => {
             let rows = match await_rows(sender, client, &mut rows).await? {
                 PeekResponseUnary::Rows(rows) => {
-                    RecordFirstRowStream::record(execute_started, client, Some(instance_id), Some(strategy));
+                    RecordFirstRowStream::record(
+                        execute_started,
+                        client,
+                        Some(instance_id),
+                        Some(strategy),
+                    );
                     rows
                 }
                 PeekResponseUnary::Error(e) => {
-                    return Ok(
-                        SqlResult::err(client, Error::Unstructured(anyhow!(e))).into(),
-                    );
+                    return Ok(SqlResult::err(client, Error::Unstructured(anyhow!(e))).into());
                 }
                 PeekResponseUnary::Canceled => {
                     return Ok(SqlResult::err(client, AdapterError::Canceled).into());
                 }
             };
-            SqlResult::rows(client, rows, &desc.relation_desc.expect("RelationDesc must exist")).into()
-        }
-        ExecuteResponse::SendingRowsImmediate { rows } => {
-            SqlResult::rows(client, rows, &desc.relation_desc.expect("RelationDesc must exist")).into()
+            SqlResult::rows(
+                client,
+                rows,
+                &desc.relation_desc.expect("RelationDesc must exist"),
+            )
+            .into()
         }
-        ExecuteResponse::Subscribing { rx, ctx_extra, instance_id } => StatementResult::Subscribe {
+        ExecuteResponse::SendingRowsImmediate { rows } => SqlResult::rows(
+            client,
+            rows,
+            &desc.relation_desc.expect("RelationDesc must exist"),
+        )
+        .into(),
+        ExecuteResponse::Subscribing {
+            rx,
+            ctx_extra,
+            instance_id,
+        } => StatementResult::Subscribe {
             tag: "SUBSCRIBE".into(),
             desc: desc.relation_desc.unwrap(),
             rx: RecordFirstRowStream::new(
@@ -1333,9 +1352,12 @@ async fn execute_stmt<S: ResultSender>(
         | ExecuteResponse::DeclaredCursor
         | ExecuteResponse::ClosedCursor) => SqlResult::err(
             client,
-            Error::Unstructured(anyhow!("internal error: encountered prohibited ExecuteResponse {:?}.\n\n
-            This is a bug. Can you please file an issue letting us know?\n
-            https://github.com/MaterializeInc/materialize/issues/new?assignees=&labels=C-bug%2CC-triage&template=01-bug.yml", ExecuteResponseKind::from(res))),
+            Error::Unstructured(anyhow!(
+                "internal error: encountered prohibited ExecuteResponse {:?}.\n\n
+            This is a bug. Can you please file an bug report letting us know?\n
+            https://github.com/MaterializeInc/materialize/discussions/new?category=bug-reports",
+                ExecuteResponseKind::from(res)
+            )),
         )
         .into(),
     })

diff --git a/src/expr/src/scalar.proto b/src/expr/src/scalar.proto
@@ -736,7 +736,8 @@ message ProtoDomainLimit {
 message ProtoEvalError {
     message ProtoUnsupported {
         string feature = 1;
-        optional uint64 issue_no = 2;
+        reserved 2; // issue_no
+        optional uint64 discussion_no = 3;
     };
     message ProtoInvalidLayer {
         uint64 max_layer = 1;

diff --git a/src/expr/src/scalar.rs b/src/expr/src/scalar.rs
@@ -2424,7 +2424,7 @@ pub enum EvalError {
     DivisionByZero,
     Unsupported {
         feature: String,
-        issue_no: Option<usize>,
+        discussion_no: Option<usize>,
     },
     FloatOverflow,
     FloatUnderflow,
@@ -2542,10 +2542,13 @@ impl fmt::Display for EvalError {
             }
             EvalError::DateBinOutOfRange(message) => f.write_str(message),
             EvalError::DivisionByZero => f.write_str("division by zero"),
-            EvalError::Unsupported { feature, issue_no } => {
+            EvalError::Unsupported {
+                feature,
+                discussion_no,
+            } => {
                 write!(f, "{} not yet supported", feature)?;
-                if let Some(issue_no) = issue_no {
-                    write!(f, ", see https://github.com/MaterializeInc/materialize/issues/{} for more details", issue_no)?;
+                if let Some(discussion_no) = discussion_no {
+                    write!(f, ", see https://github.com/MaterializeInc/materialize/discussions/{} for more details", discussion_no)?;
                 }
                 Ok(())
             }
@@ -2834,9 +2837,12 @@ impl RustType<ProtoEvalError> for EvalError {
             EvalError::CharacterTooLargeForEncoding(v) => CharacterTooLargeForEncoding(*v),
             EvalError::DateBinOutOfRange(v) => DateBinOutOfRange(v.clone()),
             EvalError::DivisionByZero => DivisionByZero(()),
-            EvalError::Unsupported { feature, issue_no } => Unsupported(ProtoUnsupported {
+            EvalError::Unsupported {
+                feature,
+                discussion_no,
+            } => Unsupported(ProtoUnsupported {
                 feature: feature.clone(),
-                issue_no: issue_no.into_proto(),
+                discussion_no: discussion_no.into_proto(),
             }),
             EvalError::FloatOverflow => FloatOverflow(()),
             EvalError::FloatUnderflow => FloatUnderflow(()),
@@ -2997,7 +3003,7 @@ impl RustType<ProtoEvalError> for EvalError {
                 DivisionByZero(()) => Ok(EvalError::DivisionByZero),
                 Unsupported(v) => Ok(EvalError::Unsupported {
                     feature: v.feature,
-                    issue_no: v.issue_no.into_rust()?,
+                    discussion_no: v.discussion_no.into_rust()?,
                 }),
                 FloatOverflow(()) => Ok(EvalError::FloatOverflow),
                 FloatUnderflow(()) => Ok(EvalError::FloatUnderflow),

diff --git a/src/expr/src/scalar/func.rs b/src/expr/src/scalar/func.rs
@@ -7605,7 +7605,7 @@ fn make_acl_item<'a>(datums: &[Datum<'a>]) -> Result<Datum<'a>, EvalError> {
     if is_grantable {
         return Err(EvalError::Unsupported {
             feature: "GRANT OPTION".to_string(),
-            issue_no: None,
+            discussion_no: None,
         });
     }
 
@@ -7653,7 +7653,7 @@ fn array_fill<'a>(
     if matches!(fill, Datum::Array(_)) {
         return Err(EvalError::Unsupported {
             feature: "array_fill with arrays".to_string(),
-            issue_no: None,
+            discussion_no: None,
         });
     }
 

diff --git a/src/expr/src/scalar/func/impls/array.rs b/src/expr/src/scalar/func/impls/array.rs
@@ -43,7 +43,7 @@ impl LazyUnaryFunc for CastArrayToListOneDim {
                     "casting multi-dimensional array to list; got array with {} dimensions",
                     ndims
                 ),
-                issue_no: None,
+                discussion_no: None,
             });
         }
 

diff --git a/src/expr/src/scalar/func/impls/date.rs b/src/expr/src/scalar/func/impls/date.rs
@@ -142,7 +142,7 @@ pub fn extract_date_inner(units: DateTimeUnits, date: NaiveDate) -> Result<Numer
         | DateTimeUnits::TimezoneMinute
         | DateTimeUnits::IsoDayOfYear => Err(EvalError::Unsupported {
             feature: format!("'{}' timestamp units", units),
-            issue_no: None,
+            discussion_no: None,
         }),
     }
 }

diff --git a/src/expr/src/scalar/func/impls/time.rs b/src/expr/src/scalar/func/impls/time.rs
@@ -81,7 +81,7 @@ where
         DateTimeUnits::Timezone | DateTimeUnits::TimezoneHour | DateTimeUnits::TimezoneMinute => {
             Err(EvalError::Unsupported {
                 feature: format!("'{}' timestamp units", units),
-                issue_no: None,
+                discussion_no: None,
             })
         }
     }

diff --git a/src/expr/src/scalar/func/impls/timestamp.rs b/src/expr/src/scalar/func/impls/timestamp.rs
@@ -314,7 +314,7 @@ where
         | DateTimeUnits::IsoDayOfWeek
         | DateTimeUnits::IsoDayOfYear => Err(EvalError::Unsupported {
             feature: format!("'{}' timestamp units", units),
-            issue_no: None,
+            discussion_no: None,
         }),
     }
 }
@@ -395,7 +395,7 @@ where
         | DateTimeUnits::TimezoneMinute
         | DateTimeUnits::IsoDayOfYear => Err(EvalError::Unsupported {
             feature: format!("'{}' timestamp units", units),
-            issue_no: None,
+            discussion_no: None,
         }),
     }
 }
@@ -544,7 +544,7 @@ pub fn date_trunc_inner<T: TimestampLike>(units: DateTimeUnits, ts: &T) -> Resul
         | DateTimeUnits::IsoDayOfWeek
         | DateTimeUnits::IsoDayOfYear => Err(EvalError::Unsupported {
             feature: format!("'{}' timestamp units", units),
-            issue_no: None,
+            discussion_no: None,
         }),
     }
 }

diff --git a/src/interchange/src/confluent.rs b/src/interchange/src/confluent.rs
@@ -59,8 +59,7 @@ pub fn extract_protobuf_header(buf: &[u8]) -> Result<(i32, &[u8])> {
         Some(0) => Ok((schema_id, &buf[1..])),
         Some(message_id) => bail!(
             "unsupported Confluent-style protobuf message descriptor id: \
-                expected 0, but found: {}. \
-                See https://github.com/MaterializeInc/materialize/issues/9250",
+                expected 0, but found: {}.",
             message_id
         ),
         None => bail!(

diff --git a/src/repr/src/explain.rs b/src/repr/src/explain.rs
@@ -117,9 +117,7 @@ impl fmt::Display for ExplainError {
             ExplainError::LinearChainsPlusRecursive => {
                 write!(
                     f,
-                    "The linear_chains option is not supported with WITH MUTUALLY RECURSIVE. \
-                If you would like to see added support, then please comment at \
-                https://github.com/MaterializeInc/materialize/issues/19012."
+                    "The linear_chains option is not supported with WITH MUTUALLY RECURSIVE."
                 )
             }
             ExplainError::UnknownError(error) => {

diff --git a/src/sql/src/lib.rs b/src/sql/src/lib.rs
@@ -69,14 +69,14 @@ macro_rules! bail_unsupported {
     ($feature:expr) => {
         return Err(crate::plan::error::PlanError::Unsupported {
             feature: $feature.to_string(),
-            issue_no: None,
+            discussion_no: None,
         }
         .into())
     };
-    ($issue:expr, $feature:expr) => {
+    ($discussion_no:expr, $feature:expr) => {
         return Err(crate::plan::error::PlanError::Unsupported {
             feature: $feature.to_string(),
-            issue_no: Some($issue),
+            discussion_no: Some($discussion_no),
         }
         .into())
     };

diff --git a/src/sql/src/plan/error.rs b/src/sql/src/plan/error.rs
@@ -51,7 +51,7 @@ pub enum PlanError {
     /// This feature is not yet supported, but may be supported at some point in the future.
     Unsupported {
         feature: String,
-        issue_no: Option<usize>,
+        discussion_no: Option<usize>,
     },
     /// This feature is not supported, and will likely never be supported.
     NeverSupported {
@@ -450,10 +450,10 @@ impl PlanError {
 impl fmt::Display for PlanError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Self::Unsupported { feature, issue_no } => {
+            Self::Unsupported { feature, discussion_no } => {
                 write!(f, "{} not yet supported", feature)?;
-                if let Some(issue_no) = issue_no {
-                    write!(f, ", see https://github.com/MaterializeInc/materialize/issues/{} for more details", issue_no)?;
+                if let Some(discussion_no) = discussion_no {
+                    write!(f, ", see https://github.com/MaterializeInc/materialize/discussions/{} for more details", discussion_no)?;
                 }
                 Ok(())
             }

diff --git a/src/sql/src/plan/query.rs b/src/sql/src/plan/query.rs
@@ -3499,7 +3499,7 @@ fn plan_using_constraint(
                     "column name {} appears more than once in USING clause",
                     c.as_str().quoted()
                 ),
-                issue_no: None,
+                discussion_no: None,
             });
         }
     }
@@ -5341,7 +5341,7 @@ fn plan_literal<'a>(l: &'a Value) -> Result<CoercibleScalarExpr, PlanError> {
                 (Datum::Numeric(d), ScalarType::Numeric { max_scale: None })
             }
         }
-        Value::HexString(_) => bail_unsupported!(3114, "hex string literals"),
+        Value::HexString(_) => bail_unsupported!("hex string literals"),
         Value::Boolean(b) => match b {
             false => (Datum::False, ScalarType::Bool),
             true => (Datum::True, ScalarType::Bool),

diff --git a/src/sql/src/plan/statement/ddl.rs b/src/sql/src/plan/statement/ddl.rs
@@ -504,7 +504,7 @@ pub fn plan_create_webhook_source(
         ty => {
             return Err(PlanError::Unsupported {
                 feature: format!("{ty} is not a valid BODY FORMAT for a WEBHOOK source"),
-                issue_no: None,
+                discussion_no: None,
             })
         }
     };
@@ -4123,7 +4123,7 @@ pub fn unplan_create_cluster(
             })
         }
         CreateClusterVariant::Unmanaged(_) => {
-            bail_unsupported!(15435, "SHOW CREATE for unmanaged clusters")
+            bail_unsupported!("SHOW CREATE for unmanaged clusters")
         }
     }
 }
@@ -5460,7 +5460,7 @@ pub fn plan_alter_item_set_cluster(
     match object_type {
         ObjectType::MaterializedView => {}
         ObjectType::Index | ObjectType::Sink | ObjectType::Source => {
-            bail_unsupported!(20841, format!("ALTER {object_type} SET CLUSTER"))
+            bail_unsupported!(29606, format!("ALTER {object_type} SET CLUSTER"))
         }
         _ => {
             bail_never_supported!(
@@ -5833,7 +5833,7 @@ pub fn plan_alter_object_swap(
         }
         (object_type, _, _) => Err(PlanError::Unsupported {
             feature: format!("ALTER {object_type} .. SWAP WITH ..."),
-            issue_no: Some(12972),
+            discussion_no: None,
         }),
     }
 }
@@ -6561,7 +6561,7 @@ pub fn plan_comment(
                 r => {
                     return Err(PlanError::Unsupported {
                         feature: format!("Specifying comments on a column of {r}"),
-                        issue_no: Some(21465),
+                        discussion_no: None,
                     });
                 }
             }

diff --git a/src/sql/src/pure.rs b/src/sql/src/pure.rs
@@ -2120,8 +2120,8 @@ async fn compile_proto(
     let primary_fd = fds.file(0);
     let message_name = match primary_fd.message_type_size() {
         1 => String::from_utf8_lossy(primary_fd.message_type(0).name()).into_owned(),
-        0 => bail_unsupported!(9598, "Protobuf schemas with no messages"),
-        _ => bail_unsupported!(9598, "Protobuf schemas with multiple messages"),
+        0 => bail_unsupported!(29603, "Protobuf schemas with no messages"),
+        _ => bail_unsupported!(29603, "Protobuf schemas with multiple messages"),
     };
 
     // Encode the file descriptor set into a SQL byte string.

diff --git a/src/storage-types/src/errors.rs b/src/storage-types/src/errors.rs
@@ -566,9 +566,12 @@ mod columnation {
                         | e @ EvalError::LengthTooLarge
                         | e @ EvalError::AclArrayNullElement
                         | e @ EvalError::MzAclArrayNullElement => e.clone(),
-                        EvalError::Unsupported { feature, issue_no } => EvalError::Unsupported {
+                        EvalError::Unsupported {
+                            feature,
+                            discussion_no,
+                        } => EvalError::Unsupported {
                             feature: self.string_region.copy(feature),
-                            issue_no: *issue_no,
+                            discussion_no: *discussion_no,
                         },
                         EvalError::Float32OutOfRange(string) => {
                             EvalError::Float32OutOfRange(self.string_region.copy(string))

diff --git a/test/sqllogictest/alter.slt b/test/sqllogictest/alter.slt
@@ -97,7 +97,7 @@ ALTER VIEW mv SET CLUSTER quickstart
 query error db error: ERROR: v is a view not a materialized view
 ALTER MATERIALIZED VIEW v SET CLUSTER quickstart
 
-query error db error: ERROR: ALTER SINK SET CLUSTER not yet supported, see https://github\.com/MaterializeInc/materialize/issues/20841 for more details
+query error db error: ERROR: ALTER SINK SET CLUSTER not yet supported, see https://github\.com/MaterializeInc/materialize/discussions/29606 for more details
 ALTER SINK v SET CLUSTER quickstart
 
 statement ok