Skip to content

Commit 860925e

Browse files
Adjust MySQL ColumnDefinitions
This commits aims to make Readyset ColumnDefinition package more aligned with MySQL's ColumnDefinition package. The main changes are: - Add `decimals` to the `Column` struct. - Made `column_length` non optional. Before we were always returning it to 1024. Now we have proper lenght based on field type. - Adjusted most of the Flags to match MySQL. - Added `sql_type` to `ColumnBase`. Some DfTypes don't record the precision, like all sql text fields (TinyText, MediumText, LongText) translate to DfType::Text, they all have a different length. MySQL Boolean come as tinyint(1) during snapshot, but Boolean during replication. Added an integration test to validate we are returning same column definitions for both upstream MySQL and Readyset. Closes: REA-4505 Closes: #1321 Release-Note-Core: Adjust MySQL ColumnDefinitions to match MySQL. Some drivers, like ruby-mysql2, use the column definition to round decimal numbers. Change-Id: I1a2c2e7fe5915b32d9f96cb600c408c6b20c874b Reviewed-on: https://gerrit.readyset.name/c/readyset/+/9075 Tested-by: Buildkite CI Reviewed-by: Michael Zink <michael.z@readyset.io>
1 parent 6b6c815 commit 860925e

File tree

11 files changed

+589
-131
lines changed

11 files changed

+589
-131
lines changed

mysql-srv/src/lib.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,10 @@
8282
//! table: String::new(),
8383
//! column: "@@max_allowed_packet".to_owned(),
8484
//! coltype: myc::constants::ColumnType::MYSQL_TYPE_LONG,
85-
//! column_length: None,
85+
//! column_length: 11,
8686
//! colflags: myc::constants::ColumnFlags::UNSIGNED_FLAG,
8787
//! character_set: myc::constants::UTF8_GENERAL_CI,
88+
//! decimals: 0,
8889
//! }];
8990
//! let mut w = results.start(cols).await.expect("cols");
9091
//! w.write_row(iter::once(67108864u32)).await.expect("writer");
@@ -98,17 +99,19 @@
9899
//! table: "foo".to_string(),
99100
//! column: "a".to_string(),
100101
//! coltype: ColumnType::MYSQL_TYPE_LONGLONG,
101-
//! column_length: None,
102+
//! column_length: 11,
102103
//! colflags: ColumnFlags::empty(),
103104
//! character_set: myc::constants::UTF8_GENERAL_CI,
105+
//! decimals: 0,
104106
//! },
105107
//! Column {
106108
//! table: "foo".to_string(),
107109
//! column: "b".to_string(),
108110
//! coltype: ColumnType::MYSQL_TYPE_STRING,
109-
//! column_length: None,
111+
//! column_length: 11,
110112
//! colflags: ColumnFlags::empty(),
111113
//! character_set: myc::constants::UTF8_GENERAL_CI,
114+
//! decimals: 0,
112115
//! },
113116
//! ];
114117
//!
@@ -226,13 +229,16 @@ pub struct Column {
226229
/// This column's type.
227230
pub coltype: ColumnType,
228231
/// This column's display length.
229-
pub column_length: Option<u32>,
232+
pub column_length: u32,
230233
/// Holds the character set for this column.
231234
pub character_set: u16,
232235
/// Any flags associated with this column.
233236
///
234237
/// Of particular interest are [`ColumnFlags::UNSIGNED_FLAG`] and [`ColumnFlags::NOT_NULL_FLAG`]
235238
pub colflags: ColumnFlags,
239+
240+
/// Column Decimal Precision
241+
pub decimals: u8,
236242
}
237243

238244
impl From<&mysql_async::Column> for Column {
@@ -241,9 +247,10 @@ impl From<&mysql_async::Column> for Column {
241247
table: c.table_str().to_string(),
242248
column: c.name_str().to_string(),
243249
coltype: c.column_type(),
244-
column_length: Some(c.column_length()),
250+
column_length: c.column_length(),
245251
character_set: c.character_set(),
246252
colflags: c.flags(),
253+
decimals: c.decimals(),
247254
}
248255
}
249256
}

mysql-srv/src/value/decode.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,9 +359,10 @@ mod tests {
359359
table: String::new(),
360360
column: String::new(),
361361
coltype: $ct,
362-
column_length: None,
362+
column_length: 0,
363363
colflags: ColumnFlags::empty(),
364364
character_set: 33,
365+
decimals: 0,
365366
};
366367

367368
if !$sig {

mysql-srv/src/value/encode.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,9 +913,10 @@ mod tests {
913913
table: String::new(),
914914
column: String::new(),
915915
coltype: $ct,
916-
column_length: None,
916+
column_length: 0,
917917
colflags: ColumnFlags::empty(),
918918
character_set: 33,
919+
decimals: 0,
919920
};
920921

921922
if !$sig {

mysql-srv/src/writers.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ fn col_enc_len(c: &Column) -> usize {
111111
+ (1 + 2 + 4 + 1 + 2 + 1 + 2)
112112
}
113113

114-
// See https://dev.mysql.com/doc/internals/en/com-query-response.html for documentation
114+
// See https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_query_response_text_resultset_column_definition.html for documentation
115115
fn write_column_definition(c: &Column, buf: &mut Vec<u8>) {
116116
// The following unwraps are fine because writes to a Vec can't fail
117117

@@ -135,14 +135,13 @@ fn write_column_definition(c: &Column, buf: &mut Vec<u8>) {
135135
//
136136
// TODO: `column_length` should not be an option. Using 1024 as a default is not necessarily
137137
// correct
138-
buf.write_u32::<LittleEndian>(c.column_length.unwrap_or(1024))
139-
.unwrap();
138+
buf.write_u32::<LittleEndian>(c.column_length).unwrap();
140139
// Column Type (1 byte)
141140
buf.write_u8(c.coltype as u8).unwrap();
142141
// Column Flags (2 bytes)
143142
buf.write_u16::<LittleEndian>(c.colflags.bits()).unwrap();
144143
// Decimals (1 byte) - maximum shown decimal digits
145-
buf.write_all(&[0x00]).unwrap(); // decimals
144+
buf.write_all(&[c.decimals]).unwrap(); // decimals
146145
buf.write_all(&[0x00, 0x00]).unwrap(); // unused
147146
}
148147

0 commit comments

Comments
 (0)