-
Notifications
You must be signed in to change notification settings - Fork 51
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
verify column in option is the same as provided input #36
Comments
We need to refactor Another proposal is use |
I'm thinking about how should we handle the type.
Currently in runner we don't check the type-string. In parser, I prototyped something like this: #[derive(Debug, PartialEq, Eq, Clone, Copy)]
#[non_exhaustive]
pub enum ColumnType {
Text,
Integer,
FloatingPoint,
/// Do not check the type of the column.
Any,
Unknown(char),
}
impl Display for ColumnType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ColumnType::Text => write!(f, "T"),
ColumnType::Integer => write!(f, "I"),
ColumnType::FloatingPoint => write!(f, "R"),
ColumnType::Any => write!(f, "?"),
ColumnType::Unknown(c) => write!(f, "{}", c),
}
}
}
impl TryFrom<char> for ColumnType {
type Error = ParseErrorKind;
fn try_from(c: char) -> Result<Self, Self::Error> {
match c {
'T' => Ok(Self::Text),
'I' => Ok(Self::Integer),
'R' => Ok(Self::FloatingPoint),
'?' => Ok(Self::Any),
// FIXME:
// _ => Err(ParseErrorKind::InvalidType(c)),
_ => Ok(Self::Unknown(c)),
}
}
} |
I'm wondering whether slt is a good place to check types. One problem is that different dbs may have different type system. |
After #111, it would be easy to implement column type checking, but we need to decide the exact policy. It's trivial to check number of columns though. |
One solution may be providing a customizable type checker |
Hi! I see two ways:
The first approach seems to be quite complicated to achieve, as different users of this library could have different needs. Column types also depend on SQL dialects that library users support. Approach 2 seems a bit more flexible to me, allowing downstream projects to use I went ahead and implemented a way to define column types and a custom column validator in this pr #160. I would appreciate your feedback. |
We should give an error in this case, where we only specify
II
, but provide 3 column input.The text was updated successfully, but these errors were encountered: