Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message for unsupported types in sinks #17709

Open
xxchan opened this issue Jul 17, 2024 · 2 comments
Open

Improve error message for unsupported types in sinks #17709

xxchan opened this issue Jul 17, 2024 · 2 comments
Assignees
Milestone

Comments

@xxchan
Copy link
Member

xxchan commented Jul 17, 2024

Noticed this in #17690. Basically all changes under src/connector/src/sink/ have this problem

e.g.,

risingwave_common::types::DataType::Bytea => Err(SinkError::ClickHouse(
"clickhouse can not support Bytea".to_string(),
)),
risingwave_common::types::DataType::Jsonb => Err(SinkError::ClickHouse(
"clickhouse rust can not support Json".to_string(),
)),
risingwave_common::types::DataType::Serial => {
Ok(ck_column.r#type.contains("UInt64") | ck_column.r#type.contains("Int64"))
}
risingwave_common::types::DataType::Int256 => Err(SinkError::ClickHouse(
"clickhouse can not support Int256".to_string(),
)),

The language "can not support" is confusing: whether it's technically impossible, or it's actually "unsupported"?

If it's the latter, we should fix the sentence. Ideally use NotImplemented error instead, which will also encourage user to create a feature request.

impl Display for TrackingIssue {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self.0 {
Some(id) => write!(f, "Tracking issue: https://github.com/risingwavelabs/risingwave/issues/{id}"),
None => write!(f, "No tracking issue yet. Feel free to submit a feature request at https://github.com/risingwavelabs/risingwave/issues/new?labels=type%2Ffeature&template=feature_request.yml"),
}
}
}
#[derive(Error, Debug, Macro)]
#[error("Feature is not yet implemented: {feature}\n{issue}")]
#[thiserror_ext(macro(path = "crate::common"))]
pub struct NotImplemented {
#[message]
pub feature: String,
pub issue: TrackingIssue,
}

Besides, there are grammar issues like

"Don't support Float32 and Int256"

@github-actions github-actions bot added this to the release-1.11 milestone Jul 17, 2024
@xxchan
Copy link
Member Author

xxchan commented Jul 17, 2024

@xxhZs @hengm3467 Can you take a look? Thanks.

Copy link
Contributor

This issue has been open for 60 days with no activity.

If you think it is still relevant today, and needs to be done in the near future, you can comment to update the status, or just manually remove the no-issue-activity label.

You can also confidently close this issue as not planned to keep our backlog clean.
Don't worry if you think the issue is still valuable to continue in the future.
It's searchable and can be reopened when it's time. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants