Implement and select and fix security bug in index

Discovered another security bug which could potentially be dangerous:
the low level impl of the index used a comparator where eager evaluation
of an expression led to an erroneous return.

server/src/engine/core/dml/mod.rs

@@ -26,6 +26,7 @@
 
 mod del;
 mod ins;
+mod sel;
 
 use crate::engine::{
     core::model::ModelData,
@@ -34,7 +35,7 @@ use crate::engine::{
     ql::dml::WhereClause,
 };
 
-pub use {del::delete, ins::insert};
+pub use {del::delete, ins::insert, sel::select_custom};
 
 impl ModelData {
     pub(self) fn resolve_where<'a>(

server/src/engine/core/dml/sel.rs

@@ -0,0 +1,74 @@
+/*
+ * Created on Thu May 11 2023
+ *
+ * This file is a part of Skytable
+ * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source
+ * NoSQL database written by Sayan Nandan ("the Author") with the
+ * vision to provide flexibility in data modelling without compromising
+ * on performance, queryability or scalability.
+ *
+ * Copyright (c) 2023, Sayan Nandan <ohsayan@outlook.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ *
+*/
+
+use crate::engine::{
+    core::{index::DcFieldIndex, GlobalNS},
+    data::cell::{Datacell, VirtualDatacell},
+    error::{DatabaseError, DatabaseResult},
+    idx::{STIndex, STIndexSeq},
+    ql::dml::sel::SelectStatement,
+    sync,
+};
+
+pub fn select_custom<F>(
+    gns: &GlobalNS,
+    mut select: SelectStatement,
+    mut cellfn: F,
+) -> DatabaseResult<()>
+where
+    F: FnMut(&Datacell),
+{
+    gns.with_model(select.entity(), |mdl| {
+        let irm = mdl.intent_read_model();
+        let target_key = mdl.resolve_where(select.clauses_mut())?;
+        let pkdc = VirtualDatacell::new(target_key.clone());
+        let g = sync::atm::cpin();
+        let mut read_field = |key, fields: &DcFieldIndex| {
+            match fields.st_get(key) {
+                Some(dc) => cellfn(dc),
+                None if key == mdl.p_key() => cellfn(&pkdc),
+                None => return Err(DatabaseError::FieldNotFound),
+            }
+            Ok(())
+        };
+        match mdl.primary_index().select(target_key.clone(), &g) {
+            Some(row) => {
+                let r = row.resolve_deltas_and_freeze(mdl.delta_state());
+                if select.is_wildcard() {
+                    for key in irm.fields().stseq_ord_key() {
+                        read_field(key.as_ref(), r.fields())?;
+                    }
+                } else {
+                    for key in select.into_fields() {
+                        read_field(key.as_str(), r.fields())?;
+                    }
+                }
+            }
+            None => return Err(DatabaseError::DmlEntryNotFound),
+        }
+        Ok(())
+    })
+}

server/src/engine/core/index/key.rs

@@ -203,13 +203,13 @@ impl<'a> PartialEq<LitIR<'a>> for PrimaryIndexKey {
 
 impl<'a> Comparable<LitIR<'a>> for PrimaryIndexKey {
     fn cmp_eq(&self, key: &LitIR<'a>) -> bool {
-        self == key
+        <PrimaryIndexKey as PartialEq<LitIR>>::eq(self, key)
     }
 }
 
 impl<'a> Comparable<PrimaryIndexKey> for LitIR<'a> {
     fn cmp_eq(&self, key: &PrimaryIndexKey) -> bool {
-        key == self
+        <PrimaryIndexKey as PartialEq<LitIR>>::eq(key, self)
     }
 }
 

server/src/engine/core/model/alt.rs

@@ -110,7 +110,7 @@ impl<'a> AlterPlan<'a> {
                 }) {
                     can_ignore!(AlterAction::Remove(r))
                 } else if not_found {
-                    return Err(DatabaseError::DdlModelAlterFieldNotFound);
+                    return Err(DatabaseError::FieldNotFound);
                 } else {
                     return Err(DatabaseError::DdlModelAlterProtectedField);
                 }
@@ -146,7 +146,7 @@ impl<'a> AlterPlan<'a> {
                     mv.guard_pk(&field_name)?;
                     // get the current field
                     let Some(current_field) = wm.fields().st_get(field_name.as_str()) else {
-                        return Err(DatabaseError::DdlModelAlterFieldNotFound);
+                        return Err(DatabaseError::FieldNotFound);
                     };
                     // check props
                     let is_nullable = check_nullable(&mut props)?;

server/src/engine/core/tests/ddl_model/alt.rs

@@ -156,7 +156,7 @@ mod plan {
                 |_| {}
             )
             .unwrap_err(),
-            DatabaseError::DdlModelAlterFieldNotFound
+            DatabaseError::FieldNotFound
         );
     }
     #[test]
@@ -216,7 +216,7 @@ mod plan {
                 |_| {}
             )
             .unwrap_err(),
-            DatabaseError::DdlModelAlterFieldNotFound
+            DatabaseError::FieldNotFound
         );
     }
     fn bad_type_cast(orig_ty: &str, new_ty: &str) {

server/src/engine/core/tests/dml/mod.rs

@@ -26,10 +26,11 @@
 
 mod delete;
 mod insert;
+mod select;
 
 use crate::engine::{
     core::{dml, index::Row, model::ModelData, GlobalNS},
-    data::lit::LitIR,
+    data::{cell::Datacell, lit::LitIR},
     error::DatabaseResult,
     ql::{
         ast::{parse_ast_node_full, Entity},
@@ -48,7 +49,7 @@ fn _exec_only_create_space_model(gns: &GlobalNS, model: &str) -> DatabaseResult<
     ModelData::exec_create(gns, stmt_create_model)
 }
 
-fn _exec_only_insert_only<T>(
+fn _exec_only_insert<T>(
     gns: &GlobalNS,
     insert: &str,
     and_then: impl Fn(Entity) -> T,
@@ -96,6 +97,14 @@ fn _exec_delete_only(gns: &GlobalNS, delete: &str, key: &str) -> DatabaseResult<
     Ok(())
 }
 
+fn _exec_only_select(gns: &GlobalNS, select: &str) -> DatabaseResult<Vec<Datacell>> {
+    let lex_sel = lex_insecure(select.as_bytes()).unwrap();
+    let select = parse_ast_node_full(&lex_sel[1..]).unwrap();
+    let mut r = Vec::new();
+    dml::select_custom(gns, select, |cell| r.push(cell.clone()))?;
+    Ok(r)
+}
+
 pub(self) fn exec_insert<T: Default>(
     gns: &GlobalNS,
     model: &str,
@@ -104,13 +113,13 @@ pub(self) fn exec_insert<T: Default>(
     f: impl Fn(Row) -> T,
 ) -> DatabaseResult<T> {
     _exec_only_create_space_model(gns, model)?;
-    _exec_only_insert_only(gns, insert, |entity| {
+    _exec_only_insert(gns, insert, |entity| {
         _exec_only_read_key_and_then(gns, entity, key_name, |row| f(row))
     })?
 }
 
 pub(self) fn exec_insert_only(gns: &GlobalNS, insert: &str) -> DatabaseResult<()> {
-    _exec_only_insert_only(gns, insert, |_| {})
+    _exec_only_insert(gns, insert, |_| {})
 }
 
 pub(self) fn exec_delete(
@@ -122,7 +131,22 @@ pub(self) fn exec_delete(
 ) -> DatabaseResult<()> {
     _exec_only_create_space_model(gns, model)?;
     if let Some(insert) = insert {
-        _exec_only_insert_only(gns, insert, |_| {})?;
+        _exec_only_insert(gns, insert, |_| {})?;
     }
     _exec_delete_only(gns, delete, key)
 }
+
+pub(self) fn exec_select(
+    gns: &GlobalNS,
+    model: &str,
+    insert: &str,
+    select: &str,
+) -> DatabaseResult<Vec<Datacell>> {
+    _exec_only_create_space_model(gns, model)?;
+    _exec_only_insert(gns, insert, |_| {})?;
+    _exec_only_select(gns, select)
+}
+
+pub(self) fn exec_select_only(gns: &GlobalNS, select: &str) -> DatabaseResult<Vec<Datacell>> {
+    _exec_only_select(gns, select)
+}

server/src/engine/core/tests/dml/select.rs

@@ -0,0 +1,102 @@
+/*
+ * Created on Thu May 11 2023
+ *
+ * This file is a part of Skytable
+ * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source
+ * NoSQL database written by Sayan Nandan ("the Author") with the
+ * vision to provide flexibility in data modelling without compromising
+ * on performance, queryability or scalability.
+ *
+ * Copyright (c) 2023, Sayan Nandan <ohsayan@outlook.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ *
+*/
+
+use crate::engine::{core::GlobalNS, data::cell::Datacell, error::DatabaseError};
+
+#[test]
+fn simple_select_wildcard() {
+    let gns = GlobalNS::empty();
+    assert_eq!(
+        super::exec_select(
+            &gns,
+            "create model myspace.mymodel(username: string, password: string)",
+            "insert into myspace.mymodel('sayan', 'pass123')",
+            "select * from myspace.mymodel where username = 'sayan'",
+        )
+        .unwrap(),
+        intovec!["sayan", "pass123"]
+    );
+}
+
+#[test]
+fn simple_select_specified_same_order() {
+    let gns = GlobalNS::empty();
+    assert_eq!(
+        super::exec_select(
+            &gns,
+            "create model myspace.mymodel(username: string, password: string)",
+            "insert into myspace.mymodel('sayan', 'pass123')",
+            "select username, password from myspace.mymodel where username = 'sayan'",
+        )
+        .unwrap(),
+        intovec!["sayan", "pass123"]
+    );
+}
+
+#[test]
+fn simple_select_specified_reversed_order() {
+    let gns = GlobalNS::empty();
+    assert_eq!(
+        super::exec_select(
+            &gns,
+            "create model myspace.mymodel(username: string, password: string)",
+            "insert into myspace.mymodel('sayan', 'pass123')",
+            "select password, username from myspace.mymodel where username = 'sayan'",
+        )
+        .unwrap(),
+        intovec!["pass123", "sayan"]
+    );
+}
+
+#[test]
+fn select_null() {
+    let gns = GlobalNS::empty();
+    assert_eq!(
+        super::exec_select(
+            &gns,
+            "create model myspace.mymodel(username: string, null password: string)",
+            "insert into myspace.mymodel('sayan', null)",
+            "select username, password from myspace.mymodel where username = 'sayan'",
+        )
+        .unwrap(),
+        intovec!["sayan", Datacell::null()]
+    );
+}
+
+#[test]
+fn select_nonexisting() {
+    let gns = GlobalNS::empty();
+    assert_eq!(
+        super::exec_select(
+            &gns,
+            "create model myspace.mymodel(username: string, null password: string)",
+            "insert into myspace.mymodel('sayan', null)",
+            "select username, password from myspace.mymodel where username = 'notsayan'",
+        )
+        .unwrap_err(),
+        DatabaseError::DmlEntryNotFound
+    );
+}

server/src/engine/error.rs

@@ -112,7 +112,7 @@ pub enum DatabaseError {
     /// the alter model statement is "wrong"
     DdlModelAlterBad,
     /// an alter attempted to update an nx field
-    DdlModelAlterFieldNotFound,
+    FieldNotFound,
     /// bad type definition to alter
     DdlModelAlterBadTypedef,
     /// didn't find the model

server/src/engine/idx/mtchm/mod.rs

@@ -428,8 +428,8 @@ impl<T: TreeElement, C: Config> RawTree<T, C> {
     ) -> Option<&'g T> {
         self._lookup(access::RModeElementRef::new(k), g)
     }
-    fn _lookup<'g, R: access::ReadMode<T>>(&'g self, r: R, g: &'g Guard) -> R::Ret<'g> {
-        let mut hash = self.hash(r.target());
+    fn _lookup<'g, R: access::ReadMode<T>>(&'g self, read_spec: R, g: &'g Guard) -> R::Ret<'g> {
+        let mut hash = self.hash(read_spec.target());
         let mut current = &self.root;
         loop {
             let node = current.ld_acq(g);
@@ -442,10 +442,9 @@ impl<T: TreeElement, C: Config> RawTree<T, C> {
                     let mut ret = R::nx();
                     return unsafe {
                         // UNSAFE(@ohsayan): checked flag + nullck
-                        Self::read_data(node).iter().find_map(|e| {
-                            r.target().cmp_eq(e.key()).then_some({
-                                ret = R::ex(e);
-                                Some(())
+                        Self::read_data(node).iter().find_map(|e_current| {
+                            read_spec.target().cmp_eq(e_current.key()).then(|| {
+                                ret = R::ex(e_current);
                             })
                         });
                         ret

server/src/engine/macros.rs

@@ -150,3 +150,8 @@ macro_rules! into_dict {
 macro_rules! pairvec {
     ($($x:expr),*) => {{ let mut v = Vec::new(); $( let (a, b) = $x; v.push((a.into(), b.into())); )* v }};
 }
+
+#[cfg(test)]
+macro_rules! intovec {
+    ($($x:expr),* $(,)?) => { vec![$(core::convert::From::from($x),)*] };
+}

server/src/util/test_utils.rs

@@ -27,6 +27,7 @@
 use {
     rand::{
         distributions::{uniform::SampleUniform, Alphanumeric},
+        rngs::ThreadRng,
         Rng,
     },
     std::{
@@ -102,3 +103,7 @@ pub fn hash_rs<T: Hash + ?Sized>(rs: &RandomState, item: &T) -> u64 {
     item.hash(&mut hasher);
     hasher.finish()
 }
+
+pub fn randomizer() -> ThreadRng {
+    rand::thread_rng()
+}