Skip to content

Commit

Permalink
Merge pull request #29611 from chaas/gh-issues-to-discussions
Browse files Browse the repository at this point in the history
Change all references in user-facing error messages to point to discussions instead of issues
  • Loading branch information
benesch authored Sep 20, 2024
2 parents 86e8d85 + a945f4e commit 26741be
Show file tree
Hide file tree
Showing 21 changed files with 85 additions and 58 deletions.
2 changes: 1 addition & 1 deletion src/adapter/src/coord/sequencer/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4404,7 +4404,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),
}))
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/adapter/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion src/catalog/src/memory/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
};
Expand Down
48 changes: 35 additions & 13 deletions src/environmentd/src/http/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,27 +1296,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(
Expand All @@ -1334,9 +1353,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(),
})
Expand Down
2 changes: 1 addition & 1 deletion src/expr/src/scalar.proto
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ message ProtoDomainLimit {
message ProtoEvalError {
message ProtoUnsupported {
string feature = 1;
optional uint64 issue_no = 2;
optional uint64 discussion_no = 2;
};
message ProtoInvalidLayer {
uint64 max_layer = 1;
Expand Down
20 changes: 13 additions & 7 deletions src/expr/src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2424,7 +2424,7 @@ pub enum EvalError {
DivisionByZero,
Unsupported {
feature: String,
issue_no: Option<usize>,
discussion_no: Option<usize>,
},
FloatOverflow,
FloatUnderflow,
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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(()),
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions src/expr/src/scalar/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7613,7 +7613,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,
});
}

Expand Down Expand Up @@ -7661,7 +7661,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,
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/expr/src/scalar/func/impls/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/expr/src/scalar/func/impls/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/expr/src/scalar/func/impls/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/expr/src/scalar/func/impls/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ where
| DateTimeUnits::IsoDayOfWeek
| DateTimeUnits::IsoDayOfYear => Err(EvalError::Unsupported {
feature: format!("'{}' timestamp units", units),
issue_no: None,
discussion_no: None,
}),
}
}
Expand Down Expand Up @@ -395,7 +395,7 @@ where
| DateTimeUnits::TimezoneMinute
| DateTimeUnits::IsoDayOfYear => Err(EvalError::Unsupported {
feature: format!("'{}' timestamp units", units),
issue_no: None,
discussion_no: None,
}),
}
}
Expand Down Expand Up @@ -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,
}),
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/interchange/src/confluent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
4 changes: 1 addition & 3 deletions src/repr/src/explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
6 changes: 3 additions & 3 deletions src/sql/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
};
Expand Down
8 changes: 4 additions & 4 deletions src/sql/src/plan/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
}
Expand Down
4 changes: 2 additions & 2 deletions src/sql/src/plan/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3554,7 +3554,7 @@ fn plan_using_constraint(
"column name {} appears more than once in USING clause",
c.as_str().quoted()
),
issue_no: None,
discussion_no: None,
});
}
}
Expand Down Expand Up @@ -5396,7 +5396,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),
Expand Down
10 changes: 5 additions & 5 deletions src/sql/src/plan/statement/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,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,
})
}
};
Expand Down Expand Up @@ -4313,7 +4313,7 @@ pub fn unplan_create_cluster(
})
}
CreateClusterVariant::Unmanaged(_) => {
bail_unsupported!(15435, "SHOW CREATE for unmanaged clusters")
bail_unsupported!("SHOW CREATE for unmanaged clusters")
}
}
}
Expand Down Expand Up @@ -5665,7 +5665,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!(
Expand Down Expand Up @@ -6038,7 +6038,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,
}),
}
}
Expand Down Expand Up @@ -6766,7 +6766,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,
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/sql/src/pure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2122,8 +2122,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.
Expand Down
7 changes: 5 additions & 2 deletions src/storage-types/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion test/sqllogictest/alter.slt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/testdrive/protobuf-import.td
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,4 @@ $ kafka-ingest topic=import-csr format=protobuf descriptor-file=import.pb messag
{"importee1": {"b": false}, "importee2": {"ts": "1970-01-01T00:20:34.000005678Z"}}

! SELECT importee1::text, importee2::text FROM import_csr
contains:Decode error: protobuf deserialization error: unsupported Confluent-style protobuf message descriptor id: expected 0, but found: 123. See https://github.com/MaterializeInc/materialize/issues/9250
contains:Decode error: protobuf deserialization error: unsupported Confluent-style protobuf message descriptor id: expected 0, but found: 123

0 comments on commit 26741be

Please sign in to comment.