Skip to content

Commit

Permalink
Support MySQL's MEDIUMINT column type
Browse files Browse the repository at this point in the history
Since there is no `i24/u24` native to Rust, we use `u32` and do some
manual bounds checking when converting from other types.

We also introduce a MySQL-specific logictest subdirectory, for tests
(such as the one included here) which we never expect to run against
PostgreSQL. Likely some existing tests should be moved there.

Release-Note-Core: Added support for MySQL's `MEDIUMINT` column type.
Fixes: REA-4285
Change-Id: I4530093ea029957dc4c8b32ab6b56a47cce177ca
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/7461
Tested-by: Buildkite CI
Reviewed-by: Jason Brown <jason.b@readyset.io>
  • Loading branch information
mvzink committed May 9, 2024
1 parent a04f066 commit 6665713
Show file tree
Hide file tree
Showing 14 changed files with 275 additions and 5 deletions.
1 change: 1 addition & 0 deletions .buildkite/pipeline.public-common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ steps:
- export RUST_BACKTRACE=full
- cargo --locked run --bin readyset-logictest -- verify logictests
- cargo --locked run --bin readyset-logictest -- verify logictests/psql --database-type postgresql
- cargo --locked run --bin readyset-logictest -- verify logictests/mysql --database-type mysql
timeout_in_minutes: 60
depends_on:
- build-image
Expand Down
8 changes: 6 additions & 2 deletions data-generator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,9 @@ pub fn value_of_type(typ: &SqlType) -> DfValue {
// octets.
DfValue::ByteArray(Arc::new(vec![0u8]))
}
SqlType::Int(_) | SqlType::Int4 | SqlType::Serial => 1i32.into(),
SqlType::Int(_) | SqlType::MediumInt(_) | SqlType::Int4 | SqlType::Serial => 1i32.into(),
SqlType::BigInt(_) | SqlType::Int8 | SqlType::BigSerial => 1i64.into(),
SqlType::UnsignedInt(_) => 1u32.into(),
SqlType::UnsignedInt(_) | SqlType::UnsignedMediumInt(_) => 1u32.into(),
SqlType::UnsignedBigInt(_) => 1u64.into(),
SqlType::TinyInt(_) => 1i8.into(),
SqlType::UnsignedTinyInt(_) => 1u8.into(),
Expand Down Expand Up @@ -533,6 +533,8 @@ where
SqlType::UnsignedTinyInt(_) => rng.gen::<u8>().into(),
SqlType::SmallInt(_) | SqlType::Int2 => rng.gen::<i16>().into(),
SqlType::UnsignedSmallInt(_) => rng.gen::<u16>().into(),
SqlType::MediumInt(_) => rng.gen_range((-1i32 << 23)..(1i32 << 23)).into(),
SqlType::UnsignedMediumInt(_) => rng.gen_range(0..(1u32 << 24)).into(),
SqlType::Float | SqlType::Double => 1.5f64.try_into().unwrap(),
SqlType::Real => 1.5f32.try_into().unwrap(),
SqlType::Decimal(prec, scale) => {
Expand Down Expand Up @@ -660,6 +662,8 @@ pub fn unique_value_of_type(typ: &SqlType, idx: u32) -> DfValue {
SqlType::UnsignedTinyInt(_) => (idx).into(),
SqlType::SmallInt(_) | SqlType::Int2 => (idx as i16).into(),
SqlType::UnsignedSmallInt(_) => (idx as u16).into(),
SqlType::MediumInt(_) => (idx as i32).into(),
SqlType::UnsignedMediumInt(_) => (idx).into(),
SqlType::Float | SqlType::Double => (1.5 + idx as f64).try_into().unwrap(),
SqlType::Real => (1.5 + idx as f32).try_into().unwrap(),
SqlType::Decimal(prec, scale) => Decimal::new(clamp_digits(*prec as _), *scale as _).into(),
Expand Down
6 changes: 6 additions & 0 deletions dataflow-expression/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,8 @@ fn mysql_type_conversion(left_ty: &DfType, right_ty: &DfType) -> DfType {
| DfType::UnsignedTinyInt
| DfType::SmallInt
| DfType::UnsignedSmallInt
| DfType::MediumInt
| DfType::UnsignedMediumInt
| DfType::Int
| DfType::UnsignedInt
| DfType::BigInt
Expand All @@ -479,6 +481,8 @@ fn mysql_type_conversion(left_ty: &DfType, right_ty: &DfType) -> DfType {
| DfType::UnsignedTinyInt
| DfType::SmallInt
| DfType::UnsignedSmallInt
| DfType::MediumInt
| DfType::UnsignedMediumInt
| DfType::Int
| DfType::UnsignedInt
| DfType::BigInt
Expand All @@ -503,6 +507,8 @@ fn mysql_type_conversion(left_ty: &DfType, right_ty: &DfType) -> DfType {
| DfType::UnsignedTinyInt
| DfType::SmallInt
| DfType::UnsignedSmallInt
| DfType::MediumInt
| DfType::UnsignedMediumInt
| DfType::Int
| DfType::UnsignedInt
| DfType::BigInt
Expand Down
23 changes: 23 additions & 0 deletions logictests/mysql/mediumint.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
statement ok
create table t1 (mint mediumint, umint mediumint unsigned);

statement ok
insert into t1 values (-8388608, 0), (8388607, 16777215);

statement error
insert into t1 values (-8388609, 0);

statement error
insert into t1 values (8388608, 0);

statement error
insert into t1 values (0, -1);

statement error
insert into t1 values (0, 16777216);

query I
select umint from t1 where mint = ?;
? = 8388607
----
16777215
6 changes: 6 additions & 0 deletions nom-sql/src/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,12 @@ impl Literal {
SqlType::UnsignedSmallInt(_) => any::<u16>()
.prop_map(|i| Self::UnsignedInteger(i as _))
.boxed(),
SqlType::MediumInt(_) => ((-1i32 << 23)..(1i32 << 23))
.prop_map(|i| Self::Integer(i as _))
.boxed(),
SqlType::UnsignedMediumInt(_) => (0..(1u32 << 24))
.prop_map(|i| Self::UnsignedInteger(i as _))
.boxed(),
SqlType::Blob
| SqlType::ByteArray
| SqlType::LongBlob
Expand Down
61 changes: 61 additions & 0 deletions nom-sql/src/sql_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ pub enum SqlType {
UnsignedTinyInt(Option<u16>),
SmallInt(Option<u16>),
UnsignedSmallInt(Option<u16>),
MediumInt(Option<u16>),
UnsignedMediumInt(Option<u16>),
Int2,
Int4,
Int8,
Expand Down Expand Up @@ -224,10 +226,12 @@ impl Arbitrary for SqlType {
(1..255u16).prop_map(|p| BigInt(Some(p))).boxed(),
(1..255u16).prop_map(|p| SmallInt(Some(p))).boxed(),
option::of(1..255u16).prop_map(TinyInt).boxed(),
option::of(1..255u16).prop_map(MediumInt).boxed(),
option::of(1..255u16).prop_map(UnsignedInt).boxed(),
option::of(1..255u16).prop_map(UnsignedSmallInt).boxed(),
option::of(1..255u16).prop_map(UnsignedBigInt).boxed(),
option::of(1..255u16).prop_map(UnsignedTinyInt).boxed(),
option::of(1..255u16).prop_map(UnsignedMediumInt).boxed(),
Just(TinyText).boxed(),
Just(MediumText).boxed(),
Just(LongText).boxed(),
Expand Down Expand Up @@ -335,6 +339,11 @@ impl DialectDisplay for SqlType {
write_with_len(f, "SMALLINT", len)?;
write!(f, " UNSIGNED")
}
SqlType::MediumInt(len) => write_with_len(f, "MEDIUMINT", len),
SqlType::UnsignedMediumInt(len) => {
write_with_len(f, "MEDIUMINT", len)?;
write!(f, " UNSIGNED")
}
SqlType::Int2 => write!(f, "INT2"),
SqlType::Int4 => write!(f, "INT4"),
SqlType::Int8 => write!(f, "INT8"),
Expand Down Expand Up @@ -773,6 +782,14 @@ fn type_identifier_part1(
value(SqlType::Int8, tag_no_case("int8")),
|i| int_type("tinyint", SqlType::UnsignedTinyInt, SqlType::TinyInt, i),
|i| int_type("smallint", SqlType::UnsignedSmallInt, SqlType::SmallInt, i),
cond_fail(dialect == Dialect::MySQL, |i| {
int_type(
"mediumint",
SqlType::UnsignedMediumInt,
SqlType::MediumInt,
i,
)
}),
|i| int_type("integer", SqlType::UnsignedInt, SqlType::Int, i),
|i| int_type("int", SqlType::UnsignedInt, SqlType::Int, i),
|i| int_type("bigint", SqlType::UnsignedBigInt, SqlType::BigInt, i),
Expand Down Expand Up @@ -1022,6 +1039,27 @@ pub fn mysql_int_cast_targets() -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&
}
}

/// Calls the parser if the condition is met; fails otherwise.
///
/// This mirrors the intent of [`nom::combinator::cond`], but for use within [`nom::branch::alt`]:
/// instead of resolving to [`None`] when the condition is not met, it returns a parser error so
/// that this branch fails and another alternative can be selected.
fn cond_fail<F>(pred: bool, f: F) -> impl FnMut(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], SqlType>
where
F: Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], SqlType>,
{
move |i| {
if pred {
f(i)
} else {
Err(nom::Err::Error(ParseError::from_error_kind(
i,
ErrorKind::Fail,
)))
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1146,6 +1184,18 @@ mod tests {
assert!(res.is_ok());
assert_eq!(res.unwrap().1, SqlType::Double);
}

#[test]
fn mediumint() {
assert_eq!(
test_parse!(type_identifier(Dialect::MySQL), b"mediumint(8)"),
SqlType::MediumInt(Some(8))
);
assert_eq!(
test_parse!(type_identifier(Dialect::MySQL), b"mediumint"),
SqlType::MediumInt(None)
);
}
}

mod postgres {
Expand Down Expand Up @@ -1432,5 +1482,16 @@ mod tests {
}
);
}

#[test]
fn mediumint_not_recognized() {
assert_eq!(
test_parse!(type_identifier(Dialect::PostgreSQL), b"mediumint"),
SqlType::Other(Relation {
schema: None,
name: "mediumint".into()
})
);
}
}
}
69 changes: 69 additions & 0 deletions readyset-data/src/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ pub(crate) fn coerce_f64(val: f64, to_ty: &DfType, from_ty: &DfType) -> ReadySet
DfType::UnsignedSmallInt => coerce_f64_to_uint::<u16>(val)
.ok_or_else(bounds_err)
.map(DfValue::from),
DfType::MediumInt => coerce_f64_to_int::<i32>(val)
.filter(|i| ((-1 << 23)..(1 << 23)).contains(i))
.ok_or_else(bounds_err)
.map(DfValue::from),
DfType::UnsignedMediumInt => coerce_f64_to_uint::<u32>(val)
.filter(|&i| i < (1 << 24))
.ok_or_else(bounds_err)
.map(DfValue::from),
DfType::Int => coerce_f64_to_int::<i32>(val)
.ok_or_else(bounds_err)
.map(DfValue::from),
Expand Down Expand Up @@ -184,6 +192,16 @@ pub(crate) fn coerce_decimal(
DfType::UnsignedTinyInt => val.to_u8().ok_or_else(err).map(DfValue::from),
DfType::SmallInt => val.to_i16().ok_or_else(err).map(DfValue::from),
DfType::UnsignedSmallInt => val.to_u16().ok_or_else(err).map(DfValue::from),
DfType::MediumInt => val
.to_i32()
.filter(|i| ((-1 << 23)..(1 << 23)).contains(i))
.ok_or_else(err)
.map(DfValue::from),
DfType::UnsignedMediumInt => val
.to_u32()
.filter(|&i| i < (1 << 24))
.ok_or_else(err)
.map(DfValue::from),
DfType::Int => val.to_i32().ok_or_else(err).map(DfValue::from),
DfType::UnsignedInt => val.to_u32().ok_or_else(err).map(DfValue::from),
DfType::BigInt => val.to_i64().ok_or_else(err).map(DfValue::from),
Expand Down Expand Up @@ -314,6 +332,30 @@ mod tests {
}
}

#[proptest]
fn float_to_mediumint(val: f32) {
if val < (-1i32 << 23) as f32 - 0.5 || val >= ((1i32 << 23) - 1) as f32 + 0.5 {
DfValue::Double(val as _)
.coerce_to(&DfType::MediumInt, &DfType::Unknown)
.expect_err("OOB");
} else {
assert_eq!(
DfValue::Double(val as f64).coerce_to(&DfType::MediumInt, &DfType::Unknown),
Ok(DfValue::Int(val.round() as i64))
);
}
if val < -0.5f32 || val >= ((1u32 << 24) - 1) as f32 + 0.5 {
DfValue::Double(val as _)
.coerce_to(&DfType::UnsignedMediumInt, &DfType::Unknown)
.expect_err("OOB");
} else {
assert_eq!(
DfValue::Double(val as f64).coerce_to(&DfType::UnsignedMediumInt, &DfType::Unknown),
Ok(DfValue::UnsignedInt(val.round() as u64))
);
}
}

#[proptest]
fn float_to_int(val: f64) {
if val < i32::MIN as f64 - 0.5 || val >= i32::MAX as f64 + 0.5 {
Expand Down Expand Up @@ -494,5 +536,32 @@ mod tests {
.coerce_to(&DfType::UnsignedBigInt, &DfType::Unknown),
Ok(DfValue::UnsignedInt(17946744073709551616))
);

assert_eq!(
DfValue::Double(-8388608.49).coerce_to(&DfType::MediumInt, &DfType::Unknown),
Ok(DfValue::Int(-8388608))
);

DfValue::Double(-8388608.51)
.coerce_to(&DfType::MediumInt, &DfType::Unknown)
.unwrap_err();

assert_eq!(
DfValue::Double(8388607.49).coerce_to(&DfType::MediumInt, &DfType::Unknown),
Ok(DfValue::Int(8388607))
);

DfValue::Double(8388607.51)
.coerce_to(&DfType::MediumInt, &DfType::Unknown)
.unwrap_err();

assert_eq!(
DfValue::Double(16777215.49).coerce_to(&DfType::UnsignedMediumInt, &DfType::Unknown),
Ok(DfValue::UnsignedInt(16777215))
);

DfValue::Double(16777215.51)
.coerce_to(&DfType::UnsignedMediumInt, &DfType::Unknown)
.unwrap_err();
}
}
10 changes: 10 additions & 0 deletions readyset-data/src/integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ where
DfType::UnsignedTinyInt => u8::try_from(val).map_err(|_| err()).map(DfValue::from),
DfType::SmallInt => i16::try_from(val).map_err(|_| err()).map(DfValue::from),
DfType::UnsignedSmallInt => u16::try_from(val).map_err(|_| err()).map(DfValue::from),
DfType::MediumInt => i32::try_from(val)
.ok()
.filter(|i| ((-1 << 23)..(1 << 23)).contains(i))
.ok_or_else(err)
.map(DfValue::from),
DfType::UnsignedMediumInt => u32::try_from(val)
.ok()
.filter(|&i| i < (1 << 24))
.ok_or_else(err)
.map(DfValue::from),
DfType::Int => i32::try_from(val).map_err(|_| err()).map(DfValue::from),
DfType::UnsignedInt => u32::try_from(val).map_err(|_| err()).map(DfValue::from),
DfType::BigInt => i64::try_from(val).map_err(|_| err()).map(DfValue::from),
Expand Down
6 changes: 6 additions & 0 deletions readyset-data/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2243,6 +2243,9 @@ mod arbitrary {
.boxed(),
Some(DfType::TinyInt) => any::<i8>().prop_map(|i| DfValue::Int(i as i64)).boxed(),
Some(DfType::SmallInt) => any::<i16>().prop_map(|i| DfValue::Int(i as i64)).boxed(),
Some(DfType::MediumInt) => ((-1i32 << 23)..(1i32 << 23))
.prop_map(|i| DfValue::Int(i as i64))
.boxed(),
Some(DfType::Int) => any::<i32>().prop_map(|i| DfValue::Int(i as i64)).boxed(),
Some(DfType::BigInt) => any::<i64>().prop_map(DfValue::Int).boxed(),
Some(DfType::UnsignedTinyInt) => any::<u8>()
Expand All @@ -2251,6 +2254,9 @@ mod arbitrary {
Some(DfType::UnsignedSmallInt) => any::<u16>()
.prop_map(|u| DfValue::UnsignedInt(u as u64))
.boxed(),
Some(DfType::UnsignedMediumInt) => (0..(1u32 << 24))
.prop_map(|u| DfValue::UnsignedInt(u as u64))
.boxed(),
Some(DfType::UnsignedInt) => any::<u32>()
.prop_map(|u| DfValue::UnsignedInt(u as u64))
.boxed(),
Expand Down

0 comments on commit 6665713

Please sign in to comment.