From 67f0afeba87f229c2658f4273339dbdbf129b1cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Moravec?= Date: Tue, 30 Jan 2024 20:12:34 +0100 Subject: [PATCH 1/9] fix: serialization of values into string --- minisql/src/type_system.rs | 101 +++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/minisql/src/type_system.rs b/minisql/src/type_system.rs index abae701..013a48d 100644 --- a/minisql/src/type_system.rs +++ b/minisql/src/type_system.rs @@ -17,6 +17,7 @@ pub type Uuid = u64; // I would rather have non-nullable values by default, // and something like an explicit Option type for nulls. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] pub enum Value { Number(f64), // TODO: Can't put floats as keys in maps, since they don't implement Eq. What to // do? @@ -24,6 +25,7 @@ pub enum Value { } #[derive(Debug, Ord, Eq, Clone, PartialOrd, PartialEq, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] pub enum IndexableValue { String(String), Int(u64), @@ -113,10 +115,88 @@ impl Value { } } +// Own string serialization so enums can be used as keys in maps +impl From for String { + fn from(value: IndexableValue) -> Self { + match value { + IndexableValue::String(s) => format!("String({s})"), + IndexableValue::Int(i) => format!("Int({i})"), + IndexableValue::Uuid(u) => format!("Uuid({u})"), + } + } +} + +impl TryFrom for IndexableValue { + type Error = String; + fn try_from(value: String) -> Result { + if !value.ends_with(')') { + return Err(format!("Invalid IndexableValue: {}", value)); + } + + if value.starts_with("String(") { + let s = value[7..value.len() - 1].to_string(); + return Ok(Self::String(s)); + } + + if value.starts_with("Int(") { + let s = value[4..value.len() - 1].to_string(); + let i = s + .parse::() + .map_err(|e| format!("Invalid Int: {}", e))?; + return Ok(Self::Int(i)); + } + + if value.starts_with("Uuid(") { + let s = value[5..value.len() - 1].to_string(); + let u = s + .parse::() + .map_err(|e| format!("Invalid UUID: {}", e))?; + return Ok(Self::Uuid(u)); + } + + Err(format!("Invalid IndexableValue: {}", value)) + } +} + +impl From for String { + fn from(value: Value) -> Self { + match value { + Value::Number(n) => format!("Number({n})"), + Value::Indexable(i) => format!("Indexable({})", String::from(i)), + } + } +} + +impl TryFrom for Value { + type Error = String; + fn try_from(value: String) -> Result { + if !value.ends_with(')') { + return Err(format!("Invalid Value: {}", value)); + } + + if value.starts_with("Number(") { + let s = value[7..value.len() - 1].to_string(); + let n = s + .parse::() + .map_err(|e| format!("Invalid Number: {}", e))?; + return Ok(Self::Number(n)); + } + + if value.starts_with("Indexable(") { + let s = value[10..value.len() - 1].to_string(); + let i = IndexableValue::try_from(s)?; + return Ok(Self::Indexable(i)); + } + + Err(format!("Invalid Value: {}", value)) + } +} + #[cfg(test)] mod tests { use super::{IndexableValue, Value}; use crate::error::TypeConversionError::UnknownType; + use crate::type_system::Value::{Indexable, Number}; #[test] fn test_encode_number() { @@ -213,4 +293,25 @@ mod tests { Err(UnknownType { oid: 2950, size: 8 }) )) } + + #[test] + fn test_value_stringification() { + let pairs = vec![ + (Number(1.0), "Number(1)"), + ( + Indexable(IndexableValue::String("hello".to_string())), + "Indexable(String(hello))", + ), + (Indexable(IndexableValue::Int(123)), "Indexable(Int(123))"), + (Indexable(IndexableValue::Uuid(123)), "Indexable(Uuid(123))"), + ]; + + for (value, string) in pairs { + let serialized = String::from(value.clone()); + assert_eq!(serialized, string); + + let deserialized = Value::try_from(serialized); + assert_eq!(deserialized, Ok(value)); + } + } } From 22d29e86bf87c88507ded1b32c7a50f7467567d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Moravec?= Date: Tue, 30 Jan 2024 20:19:06 +0100 Subject: [PATCH 2/9] fix: convert demo db file --- demo.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo.json b/demo.json index f7ac84a..32cf0f8 100644 --- a/demo.json +++ b/demo.json @@ -1 +1 @@ -{"table_name_position_mapping":{"users":0,"cars":1},"tables":[{"schema":{"table_name":"users","primary_key":0,"column_name_position_mapping":{"id":0,"name":1,"surname":2,"email":3},"types":["Uuid","String","String","String"]},"rows":{"1":[{"Indexable":{"Uuid":1}},{"Indexable":{"String":"Isabelle"}},{"Indexable":{"String":"Oppie"}},{"Indexable":{"String":"ioppie0@mapquest.com"}}],"2":[{"Indexable":{"Uuid":2}},{"Indexable":{"String":"Frederik"}},{"Indexable":{"String":"Vardy"}},{"Indexable":{"String":"fvardy1@bloomberg.com"}}],"3":[{"Indexable":{"Uuid":3}},{"Indexable":{"String":"Ronna"}},{"Indexable":{"String":"Faiers"}},{"Indexable":{"String":"rfaiers2@netlog.com"}}],"4":[{"Indexable":{"Uuid":4}},{"Indexable":{"String":"Wash"}},{"Indexable":{"String":"Dirkin"}},{"Indexable":{"String":"wdirkin3@google.com.au"}}],"5":[{"Indexable":{"Uuid":5}},{"Indexable":{"String":"Dusty"}},{"Indexable":{"String":"Ayshford"}},{"Indexable":{"String":"dayshford4@discuz.net"}}],"6":[{"Indexable":{"Uuid":6}},{"Indexable":{"String":"Brenn"}},{"Indexable":{"String":"McGucken"}},{"Indexable":{"String":"bmcgucken5@networksolutions.com"}}],"7":[{"Indexable":{"Uuid":7}},{"Indexable":{"String":"Elwyn"}},{"Indexable":{"String":"Padbery"}},{"Indexable":{"String":"epadbery6@dion.ne.jp"}}],"8":[{"Indexable":{"Uuid":8}},{"Indexable":{"String":"Jobyna"}},{"Indexable":{"String":"Playhill"}},{"Indexable":{"String":"jplayhill7@virginia.edu"}}],"9":[{"Indexable":{"Uuid":9}},{"Indexable":{"String":"Ermina"}},{"Indexable":{"String":"Ledekker"}},{"Indexable":{"String":"eledekker8@xing.com"}}],"10":[{"Indexable":{"Uuid":10}},{"Indexable":{"String":"Saudra"}},{"Indexable":{"String":"Montague"}},{"Indexable":{"String":"smontague9@youtube.com"}}],"11":[{"Indexable":{"Uuid":11}},{"Indexable":{"String":"Gan"}},{"Indexable":{"String":"Levick"}},{"Indexable":{"String":"glevicka@nih.gov"}}],"12":[{"Indexable":{"Uuid":12}},{"Indexable":{"String":"Nicoline"}},{"Indexable":{"String":"Pike"}},{"Indexable":{"String":"npikeb@nymag.com"}}],"13":[{"Indexable":{"Uuid":13}},{"Indexable":{"String":"Netty"}},{"Indexable":{"String":"Jeandet"}},{"Indexable":{"String":"njeandetc@slashdot.org"}}],"14":[{"Indexable":{"Uuid":14}},{"Indexable":{"String":"Erich"}},{"Indexable":{"String":"Blonfield"}},{"Indexable":{"String":"eblonfieldd@epa.gov"}}],"15":[{"Indexable":{"Uuid":15}},{"Indexable":{"String":"Natividad"}},{"Indexable":{"String":"Doddemeede"}},{"Indexable":{"String":"ndoddemeedee@blinklist.com"}}],"16":[{"Indexable":{"Uuid":16}},{"Indexable":{"String":"Jobi"}},{"Indexable":{"String":"Gore"}},{"Indexable":{"String":"jgoref@oaic.gov.au"}}],"17":[{"Indexable":{"Uuid":17}},{"Indexable":{"String":"Christina"}},{"Indexable":{"String":"Lisciandri"}},{"Indexable":{"String":"clisciandrig@pcworld.com"}}],"18":[{"Indexable":{"Uuid":18}},{"Indexable":{"String":"Hallsy"}},{"Indexable":{"String":"Isaksson"}},{"Indexable":{"String":"hisakssonh@uiuc.edu"}}],"19":[{"Indexable":{"Uuid":19}},{"Indexable":{"String":"Isac"}},{"Indexable":{"String":"Edwick"}},{"Indexable":{"String":"iedwicki@over-blog.com"}}],"20":[{"Indexable":{"Uuid":20}},{"Indexable":{"String":"Beatrix"}},{"Indexable":{"String":"Durdy"}},{"Indexable":{"String":"bdurdyj@tuttocitta.it"}}],"21":[{"Indexable":{"Uuid":21}},{"Indexable":{"String":"Oralia"}},{"Indexable":{"String":"Venning"}},{"Indexable":{"String":"ovenningk@businesswire.com"}}],"22":[{"Indexable":{"Uuid":22}},{"Indexable":{"String":"Leese"}},{"Indexable":{"String":"Kitchaside"}},{"Indexable":{"String":"lkitchasidel@princeton.edu"}}],"23":[{"Indexable":{"Uuid":23}},{"Indexable":{"String":"Magda"}},{"Indexable":{"String":"Yurshev"}},{"Indexable":{"String":"myurshevm@washington.edu"}}],"24":[{"Indexable":{"Uuid":24}},{"Indexable":{"String":"Benyamin"}},{"Indexable":{"String":"Dominguez"}},{"Indexable":{"String":"bdominguezn@skyrock.com"}}],"25":[{"Indexable":{"Uuid":25}},{"Indexable":{"String":"Jorey"}},{"Indexable":{"String":"Benzie"}},{"Indexable":{"String":"jbenzieo@tripod.com"}}]},"indexes":{}},{"schema":{"table_name":"cars","primary_key":0,"column_name_position_mapping":{"id":0,"brand":2,"vid":1,"year":4,"model":3},"types":["Uuid","String","String","String","Int"]},"rows":{"1":[{"Indexable":{"Uuid":1}},{"Indexable":{"String":"5XYZG3AB1CG391341"}},{"Indexable":{"String":"Volkswagen"}},{"Indexable":{"String":"Golf"}},{"Indexable":{"Int":1939}}],"2":[{"Indexable":{"Uuid":2}},{"Indexable":{"String":"JN1CV6APXBM635254"}},{"Indexable":{"String":"Chevrolet"}},{"Indexable":{"String":"S10"}},{"Indexable":{"Int":1969}}],"3":[{"Indexable":{"Uuid":3}},{"Indexable":{"String":"4T1BF3EK4BU925939"}},{"Indexable":{"String":"Toyota"}},{"Indexable":{"String":"MR2"}},{"Indexable":{"Int":1958}}],"4":[{"Indexable":{"Uuid":4}},{"Indexable":{"String":"WDDJK7DA7EF523150"}},{"Indexable":{"String":"Toyota"}},{"Indexable":{"String":"MR2"}},{"Indexable":{"Int":1903}}],"5":[{"Indexable":{"Uuid":5}},{"Indexable":{"String":"KNADM5A3XD6039514"}},{"Indexable":{"String":"Hyundai"}},{"Indexable":{"String":"Santa Fe"}},{"Indexable":{"Int":1970}}],"6":[{"Indexable":{"Uuid":6}},{"Indexable":{"String":"WBAKB0C52AC111480"}},{"Indexable":{"String":"Mitsubishi"}},{"Indexable":{"String":"3000GT"}},{"Indexable":{"Int":1907}}],"7":[{"Indexable":{"Uuid":7}},{"Indexable":{"String":"WBAAW33401E562288"}},{"Indexable":{"String":"Dodge"}},{"Indexable":{"String":"Avenger"}},{"Indexable":{"Int":1967}}],"8":[{"Indexable":{"Uuid":8}},{"Indexable":{"String":"WBAFR7C54BC365585"}},{"Indexable":{"String":"Hyundai"}},{"Indexable":{"String":"Accent"}},{"Indexable":{"Int":2010}}],"9":[{"Indexable":{"Uuid":9}},{"Indexable":{"String":"WA1AY74L29D206107"}},{"Indexable":{"String":"Ford"}},{"Indexable":{"String":"Escort"}},{"Indexable":{"Int":1938}}],"10":[{"Indexable":{"Uuid":10}},{"Indexable":{"String":"JN8AS5MTXBW273113"}},{"Indexable":{"String":"Dodge"}},{"Indexable":{"String":"Intrepid"}},{"Indexable":{"Int":1925}}],"11":[{"Indexable":{"Uuid":11}},{"Indexable":{"String":"3D73Y4CL4BG009999"}},{"Indexable":{"String":"Toyota"}},{"Indexable":{"String":"Yaris"}},{"Indexable":{"Int":1906}}],"12":[{"Indexable":{"Uuid":12}},{"Indexable":{"String":"SCFFDABE1BG998717"}},{"Indexable":{"String":"Lamborghini"}},{"Indexable":{"String":"Diablo"}},{"Indexable":{"Int":2007}}],"13":[{"Indexable":{"Uuid":13}},{"Indexable":{"String":"1G4GG5E35DF715445"}},{"Indexable":{"String":"Lotus"}},{"Indexable":{"String":"Exige"}},{"Indexable":{"Int":1998}}],"14":[{"Indexable":{"Uuid":14}},{"Indexable":{"String":"1G6AY5S37E0510437"}},{"Indexable":{"String":"Land Rover"}},{"Indexable":{"String":"Discovery"}},{"Indexable":{"Int":1967}}],"15":[{"Indexable":{"Uuid":15}},{"Indexable":{"String":"KMHGH4JH4FU924859"}},{"Indexable":{"String":"Toyota"}},{"Indexable":{"String":"4Runner"}},{"Indexable":{"Int":1946}}],"16":[{"Indexable":{"Uuid":16}},{"Indexable":{"String":"JTMHY7AJ3D4338549"}},{"Indexable":{"String":"Audi"}},{"Indexable":{"String":"S5"}},{"Indexable":{"Int":2020}}],"17":[{"Indexable":{"Uuid":17}},{"Indexable":{"String":"3D4PG3FG7BT795377"}},{"Indexable":{"String":"GMC"}},{"Indexable":{"String":"Sierra 2500"}},{"Indexable":{"Int":1936}}],"18":[{"Indexable":{"Uuid":18}},{"Indexable":{"String":"SALAG2D44CA789334"}},{"Indexable":{"String":"Mitsubishi"}},{"Indexable":{"String":"Diamante"}},{"Indexable":{"Int":2019}}],"19":[{"Indexable":{"Uuid":19}},{"Indexable":{"String":"5N1AR1NB1CC618825"}},{"Indexable":{"String":"Infiniti"}},{"Indexable":{"String":"QX"}},{"Indexable":{"Int":1938}}],"20":[{"Indexable":{"Uuid":20}},{"Indexable":{"String":"1GKKRNED6DJ757464"}},{"Indexable":{"String":"Subaru"}},{"Indexable":{"String":"Baja"}},{"Indexable":{"Int":1969}}],"21":[{"Indexable":{"Uuid":21}},{"Indexable":{"String":"WAULD54B82N660784"}},{"Indexable":{"String":"Mercedes-Benz"}},{"Indexable":{"String":"C-Class"}},{"Indexable":{"Int":2001}}],"22":[{"Indexable":{"Uuid":22}},{"Indexable":{"String":"3C63D2CL1CG144067"}},{"Indexable":{"String":"BMW"}},{"Indexable":{"String":"8 Series"}},{"Indexable":{"Int":1978}}],"23":[{"Indexable":{"Uuid":23}},{"Indexable":{"String":"SCFAD22363K791284"}},{"Indexable":{"String":"Chrysler"}},{"Indexable":{"String":"300"}},{"Indexable":{"Int":1939}}],"24":[{"Indexable":{"Uuid":24}},{"Indexable":{"String":"WAUML54D11N926735"}},{"Indexable":{"String":"Buick"}},{"Indexable":{"String":"Regal"}},{"Indexable":{"Int":1935}}],"25":[{"Indexable":{"Uuid":25}},{"Indexable":{"String":"WUARL48H07K869442"}},{"Indexable":{"String":"Toyota"}},{"Indexable":{"String":"Camry"}},{"Indexable":{"Int":1988}}]},"indexes":{}}]} \ No newline at end of file +{"table_name_position_mapping":{"users":0,"cars":1},"tables":[{"schema":{"table_name":"users","primary_key":0,"column_name_position_mapping":{"email":3,"name":1,"surname":2,"id":0},"types":["Uuid","String","String","String"]},"rows":{"1":["Indexable(Uuid(1))","Indexable(String(Isabelle))","Indexable(String(Oppie))","Indexable(String(ioppie0@mapquest.com))"],"2":["Indexable(Uuid(2))","Indexable(String(Frederik))","Indexable(String(Vardy))","Indexable(String(fvardy1@bloomberg.com))"],"3":["Indexable(Uuid(3))","Indexable(String(Ronna))","Indexable(String(Faiers))","Indexable(String(rfaiers2@netlog.com))"],"4":["Indexable(Uuid(4))","Indexable(String(Wash))","Indexable(String(Dirkin))","Indexable(String(wdirkin3@google.com.au))"],"5":["Indexable(Uuid(5))","Indexable(String(Dusty))","Indexable(String(Ayshford))","Indexable(String(dayshford4@discuz.net))"],"6":["Indexable(Uuid(6))","Indexable(String(Brenn))","Indexable(String(McGucken))","Indexable(String(bmcgucken5@networksolutions.com))"],"7":["Indexable(Uuid(7))","Indexable(String(Elwyn))","Indexable(String(Padbery))","Indexable(String(epadbery6@dion.ne.jp))"],"8":["Indexable(Uuid(8))","Indexable(String(Jobyna))","Indexable(String(Playhill))","Indexable(String(jplayhill7@virginia.edu))"],"9":["Indexable(Uuid(9))","Indexable(String(Ermina))","Indexable(String(Ledekker))","Indexable(String(eledekker8@xing.com))"],"10":["Indexable(Uuid(10))","Indexable(String(Saudra))","Indexable(String(Montague))","Indexable(String(smontague9@youtube.com))"],"11":["Indexable(Uuid(11))","Indexable(String(Gan))","Indexable(String(Levick))","Indexable(String(glevicka@nih.gov))"],"12":["Indexable(Uuid(12))","Indexable(String(Nicoline))","Indexable(String(Pike))","Indexable(String(npikeb@nymag.com))"],"13":["Indexable(Uuid(13))","Indexable(String(Netty))","Indexable(String(Jeandet))","Indexable(String(njeandetc@slashdot.org))"],"14":["Indexable(Uuid(14))","Indexable(String(Erich))","Indexable(String(Blonfield))","Indexable(String(eblonfieldd@epa.gov))"],"15":["Indexable(Uuid(15))","Indexable(String(Natividad))","Indexable(String(Doddemeede))","Indexable(String(ndoddemeedee@blinklist.com))"],"16":["Indexable(Uuid(16))","Indexable(String(Jobi))","Indexable(String(Gore))","Indexable(String(jgoref@oaic.gov.au))"],"17":["Indexable(Uuid(17))","Indexable(String(Christina))","Indexable(String(Lisciandri))","Indexable(String(clisciandrig@pcworld.com))"],"18":["Indexable(Uuid(18))","Indexable(String(Hallsy))","Indexable(String(Isaksson))","Indexable(String(hisakssonh@uiuc.edu))"],"19":["Indexable(Uuid(19))","Indexable(String(Isac))","Indexable(String(Edwick))","Indexable(String(iedwicki@over-blog.com))"],"20":["Indexable(Uuid(20))","Indexable(String(Beatrix))","Indexable(String(Durdy))","Indexable(String(bdurdyj@tuttocitta.it))"],"21":["Indexable(Uuid(21))","Indexable(String(Oralia))","Indexable(String(Venning))","Indexable(String(ovenningk@businesswire.com))"],"22":["Indexable(Uuid(22))","Indexable(String(Leese))","Indexable(String(Kitchaside))","Indexable(String(lkitchasidel@princeton.edu))"],"23":["Indexable(Uuid(23))","Indexable(String(Magda))","Indexable(String(Yurshev))","Indexable(String(myurshevm@washington.edu))"],"24":["Indexable(Uuid(24))","Indexable(String(Benyamin))","Indexable(String(Dominguez))","Indexable(String(bdominguezn@skyrock.com))"],"25":["Indexable(Uuid(25))","Indexable(String(Jorey))","Indexable(String(Benzie))","Indexable(String(jbenzieo@tripod.com))"]},"indexes":{}},{"schema":{"table_name":"cars","primary_key":0,"column_name_position_mapping":{"id":0,"brand":2,"vid":1,"model":3,"year":4},"types":["Uuid","String","String","String","Int"]},"rows":{"1":["Indexable(Uuid(1))","Indexable(String(5XYZG3AB1CG391341))","Indexable(String(Volkswagen))","Indexable(String(Golf))","Indexable(Int(1939))"],"2":["Indexable(Uuid(2))","Indexable(String(JN1CV6APXBM635254))","Indexable(String(Chevrolet))","Indexable(String(S10))","Indexable(Int(1969))"],"3":["Indexable(Uuid(3))","Indexable(String(4T1BF3EK4BU925939))","Indexable(String(Toyota))","Indexable(String(MR2))","Indexable(Int(1958))"],"4":["Indexable(Uuid(4))","Indexable(String(WDDJK7DA7EF523150))","Indexable(String(Toyota))","Indexable(String(MR2))","Indexable(Int(1903))"],"5":["Indexable(Uuid(5))","Indexable(String(KNADM5A3XD6039514))","Indexable(String(Hyundai))","Indexable(String(Santa Fe))","Indexable(Int(1970))"],"6":["Indexable(Uuid(6))","Indexable(String(WBAKB0C52AC111480))","Indexable(String(Mitsubishi))","Indexable(String(3000GT))","Indexable(Int(1907))"],"7":["Indexable(Uuid(7))","Indexable(String(WBAAW33401E562288))","Indexable(String(Dodge))","Indexable(String(Avenger))","Indexable(Int(1967))"],"8":["Indexable(Uuid(8))","Indexable(String(WBAFR7C54BC365585))","Indexable(String(Hyundai))","Indexable(String(Accent))","Indexable(Int(2010))"],"9":["Indexable(Uuid(9))","Indexable(String(WA1AY74L29D206107))","Indexable(String(Ford))","Indexable(String(Escort))","Indexable(Int(1938))"],"10":["Indexable(Uuid(10))","Indexable(String(JN8AS5MTXBW273113))","Indexable(String(Dodge))","Indexable(String(Intrepid))","Indexable(Int(1925))"],"11":["Indexable(Uuid(11))","Indexable(String(3D73Y4CL4BG009999))","Indexable(String(Toyota))","Indexable(String(Yaris))","Indexable(Int(1906))"],"12":["Indexable(Uuid(12))","Indexable(String(SCFFDABE1BG998717))","Indexable(String(Lamborghini))","Indexable(String(Diablo))","Indexable(Int(2007))"],"13":["Indexable(Uuid(13))","Indexable(String(1G4GG5E35DF715445))","Indexable(String(Lotus))","Indexable(String(Exige))","Indexable(Int(1998))"],"14":["Indexable(Uuid(14))","Indexable(String(1G6AY5S37E0510437))","Indexable(String(Land Rover))","Indexable(String(Discovery))","Indexable(Int(1967))"],"15":["Indexable(Uuid(15))","Indexable(String(KMHGH4JH4FU924859))","Indexable(String(Toyota))","Indexable(String(4Runner))","Indexable(Int(1946))"],"16":["Indexable(Uuid(16))","Indexable(String(JTMHY7AJ3D4338549))","Indexable(String(Audi))","Indexable(String(S5))","Indexable(Int(2020))"],"17":["Indexable(Uuid(17))","Indexable(String(3D4PG3FG7BT795377))","Indexable(String(GMC))","Indexable(String(Sierra 2500))","Indexable(Int(1936))"],"18":["Indexable(Uuid(18))","Indexable(String(SALAG2D44CA789334))","Indexable(String(Mitsubishi))","Indexable(String(Diamante))","Indexable(Int(2019))"],"19":["Indexable(Uuid(19))","Indexable(String(5N1AR1NB1CC618825))","Indexable(String(Infiniti))","Indexable(String(QX))","Indexable(Int(1938))"],"20":["Indexable(Uuid(20))","Indexable(String(1GKKRNED6DJ757464))","Indexable(String(Subaru))","Indexable(String(Baja))","Indexable(Int(1969))"],"21":["Indexable(Uuid(21))","Indexable(String(WAULD54B82N660784))","Indexable(String(Mercedes-Benz))","Indexable(String(C-Class))","Indexable(Int(2001))"],"22":["Indexable(Uuid(22))","Indexable(String(3C63D2CL1CG144067))","Indexable(String(BMW))","Indexable(String(8 Series))","Indexable(Int(1978))"],"23":["Indexable(Uuid(23))","Indexable(String(SCFAD22363K791284))","Indexable(String(Chrysler))","Indexable(String(300))","Indexable(Int(1939))"],"24":["Indexable(Uuid(24))","Indexable(String(WAUML54D11N926735))","Indexable(String(Buick))","Indexable(String(Regal))","Indexable(Int(1935))"],"25":["Indexable(Uuid(25))","Indexable(String(WUARL48H07K869442))","Indexable(String(Toyota))","Indexable(String(Camry))","Indexable(Int(1988))"]},"indexes":{}}]} \ No newline at end of file From 8cc5b9280813581f15a51b524c484c70bbe51aa2 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:25:09 +0100 Subject: [PATCH 3/9] Remove TODO's about Null. We'll implement Option in another PR --- minisql/src/type_system.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/minisql/src/type_system.rs b/minisql/src/type_system.rs index abae701..532025f 100644 --- a/minisql/src/type_system.rs +++ b/minisql/src/type_system.rs @@ -13,9 +13,6 @@ pub enum DbType { // ==============Values================ pub type Uuid = u64; -// TODO: What about nulls? I would rather not have that in SQL, it sucks. -// I would rather have non-nullable values by default, -// and something like an explicit Option type for nulls. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum Value { Number(f64), // TODO: Can't put floats as keys in maps, since they don't implement Eq. What to @@ -28,7 +25,6 @@ pub enum IndexableValue { String(String), Int(u64), Uuid(Uuid), - // TODO: what about null? } impl DbType { From df108f581ccdc142fd95da6d6740ba8514d116e7 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:25:43 +0100 Subject: [PATCH 4/9] Explain why floats are different with respect to indexing --- minisql/src/type_system.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/minisql/src/type_system.rs b/minisql/src/type_system.rs index 532025f..c0ae728 100644 --- a/minisql/src/type_system.rs +++ b/minisql/src/type_system.rs @@ -15,8 +15,20 @@ pub type Uuid = u64; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum Value { - Number(f64), // TODO: Can't put floats as keys in maps, since they don't implement Eq. What to - // do? + // Note that it doesn't really make sense to compare floats on equality without specifying + // precision. You can ofcourse convert a float to string or to a bytevector and then compare + // equality of those, but that's not the right equality. And ofcourse Rust designers are aware + // of this, so floats don't implement the Eq trait. + // This ofcourse complicates indexing of Number columns. + // + // Either we'd have to design a specific key-value map data-structure where keys are floats, + // s.t. to index with a given float K you also specify a tolerance error so that the resulting + // value set will contain all values whose keys are close to K within that tolerence. This + // seems highly non-trivial. + // + // So we choose to make a distinction between indexable and non-indexable types, and Number is + // not indexable. + Number(f64), Indexable(IndexableValue), } From 83fd46b4f5095e8a5ef24f9b9a2682004d912b56 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:29:05 +0100 Subject: [PATCH 5/9] Explain why implementing Debug for Response::Selected(...) is problematic --- minisql/src/interpreter.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/minisql/src/interpreter.rs b/minisql/src/interpreter.rs index 773eac1..89df2b2 100644 --- a/minisql/src/interpreter.rs +++ b/minisql/src/interpreter.rs @@ -35,10 +35,9 @@ impl std::fmt::Debug for Response<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { use Response::*; match self { - Selected(_schema, _columns, _rows) => - // TODO: How can we iterate through the rows without having to take ownership of - // them? - { + Selected(_schema, _columns, _rows) => { + // It seems that Rust requires ownership of rows to format them here. + // This is why we output the string below f.write_str("Some rows... trust me") } Inserted => f.write_str("Inserted"), From 5909df60b0aa0a52f56051a2b102eb9770b1125a Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:30:59 +0100 Subject: [PATCH 6/9] Explain one occurence of extra braces around a block of code in tests --- minisql/src/interpreter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/minisql/src/interpreter.rs b/minisql/src/interpreter.rs index 89df2b2..e474673 100644 --- a/minisql/src/interpreter.rs +++ b/minisql/src/interpreter.rs @@ -574,8 +574,8 @@ pub fn example() { { { - // TODO: Why do I have to write these braces explicitely? Why doesn't Rust compiler - // "infer" them? + // HEY_BTW: Seems that there is some Rust inference feature still missing when it comes to + // ownership. That's why we put this block in braces. let _delete_response: Response = state .interpret(Delete(users_position, Some(Eq(id_column, id0.clone())))) .unwrap(); From e22b31dc415fe9682edfde32771e7fd73979dbd5 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:31:42 +0100 Subject: [PATCH 7/9] Locking per table is a concern of a different PR --- minisql/src/internals/table.rs | 3 +-- minisql/src/interpreter.rs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/minisql/src/internals/table.rs b/minisql/src/internals/table.rs index 0f9bb2c..66df647 100644 --- a/minisql/src/internals/table.rs +++ b/minisql/src/internals/table.rs @@ -12,8 +12,7 @@ use crate::type_system::{IndexableValue, Uuid, Value}; #[derive(Debug, Serialize, Deserialize)] pub struct Table { schema: TableSchema, - rows: Rows, // TODO: Consider wrapping this in a lock. Also consider if we need to have the - // same lock for both rows and indexes + rows: Rows, indexes: HashMap, } diff --git a/minisql/src/interpreter.rs b/minisql/src/interpreter.rs index e474673..5955c54 100644 --- a/minisql/src/interpreter.rs +++ b/minisql/src/interpreter.rs @@ -88,7 +88,6 @@ impl State { } pub fn interpret<'a>(&'a mut self, operation: Operation) -> DbResult> { - // TODO: lock stuff use Operation::*; match operation { From a3e3390c857b6a1de605d4ff0ff1565dfe492b75 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:35:10 +0100 Subject: [PATCH 8/9] Explain the role of the DESIGN_OLD.md file --- DESIGN.md => DESIGN_OLD.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) rename DESIGN.md => DESIGN_OLD.md (91%) diff --git a/DESIGN.md b/DESIGN_OLD.md similarity index 91% rename from DESIGN.md rename to DESIGN_OLD.md index d94f87b..b031efe 100644 --- a/DESIGN.md +++ b/DESIGN_OLD.md @@ -1,3 +1,7 @@ +Note that this is a Historical Document. It is a first attempt at +figuring out basic design requirements. + + # MiniSQL ## Official Description @@ -40,7 +44,7 @@ Possible usage: ```./minisql server start --db path/to/db/my-db.db --port 1433``` which will store the database as a file `path/to/db/my-db.db` and open a TCP server on port `1433` * Then on possibly a different machine you run `./minisql client connect server_ip_address:6666` to start a client. This will open a REPL with which you can send queries/db management commands -* TODO: We should also consider writing a rust library that allows you to spin up a client that connects to the server. +* We should also consider writing a rust library that allows you to spin up a client that connects to the server. How would the interface look like? ``` use mysql::{DB, DBConnection} @@ -69,7 +73,7 @@ which will store the database as a file `path/to/db/my-db.db` and open a TCP ser how will the parsing output look like? Consider something like ``` -// TODO: Parser has access to all table metadata +// Parser has access to all table metadata // Could also be called `SQLAbstractSyntaxTree` enum Operation { @@ -132,7 +136,7 @@ type TableName = String // Note that it is nice to split metadata from the data because // then you can give the metadata to the parser without giving it the data. struct TableMetaData { - name: TableName, // TODO: Is this really necessary? probably not + name: TableName, columns: Vec<(ColumnName, DbType, ColumnPosition)> } @@ -142,7 +146,7 @@ struct Table { meta: TableMetaData, rows: Rows // defined below indexes: - BTree // TODO: Consider generalizing ColumnName to semething that would also apply to a pair of ColumnNames etc + BTree } type Tables = HashMap @@ -183,16 +187,13 @@ Vec> * Interpreter ``` trait SqlConsumer { - // TODO: ??? } fn interpret(operation: Operation, tables: &mut Tables, consumer: T) -> () { - // TODO: lock stuff match operation { Select(table_name, column_selection, maybe_condition) => { let table: Table = ... - // TODO: Wrap this into a response select(table, column_selection, maybe_condition, consumer) }, Insert(table_name, Vec<(ColumnName, DbValue)>) => { @@ -209,7 +210,7 @@ fn interpret(operation: Operation, tables: &mut Tables, consumer enum Response { - Selected(impl Iter) // TODO: How to do this? Some reference to an iterator somehow... slice..? + Selected(impl Iter) // How to do this? Some reference to an iterator somehow... slice..? Inserted(???), Deleted(usize), // how many were deleted } @@ -221,7 +222,7 @@ fn select(table: Table, ColumnName ``` -* TODO: Consider streaming the response to the client and not just dumping 10K rows at once. +* Consider streaming the response to the client and not just dumping 10K rows at once. From 7c9caa032b7888f2134c577af42b14c600cea871 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Thu, 1 Feb 2024 14:20:24 +0100 Subject: [PATCH 9/9] Remove HEY_BTW: comment --- minisql/src/interpreter.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/minisql/src/interpreter.rs b/minisql/src/interpreter.rs index 5955c54..09b5e18 100644 --- a/minisql/src/interpreter.rs +++ b/minisql/src/interpreter.rs @@ -573,8 +573,6 @@ pub fn example() { { { - // HEY_BTW: Seems that there is some Rust inference feature still missing when it comes to - // ownership. That's why we put this block in braces. let _delete_response: Response = state .interpret(Delete(users_position, Some(Eq(id_column, id0.clone())))) .unwrap();