Added: order by clause sanitization

Author: MordechaiHadad

src/builders/select.rs

@@ -59,11 +59,22 @@ impl FromReady {
         self
     }
 
+    /// Adds an `ORDER BY` term for the given field and options.
+    ///
+    /// Trailing SurrealDB order modifiers embedded in `field` (`ASC`, `DESC`,
+    /// `NUMERIC`, `COLLATE`) are automatically stripped and discarded before
+    /// building the term. Final ordering is determined exclusively by the
+    /// explicit `order` argument (or its defaults when none is provided).
+    ///
+    /// This prevents duplicate tokens such as `"name DESC DESC"` when callers
+    /// accidentally include direction keywords in the field string.
     pub fn order_by(mut self, field: &str, order: impl Into<OrderOptions>) -> Self {
+        let sanitized_field = OrderTerm::sanitize_field(field);
+
         let opt = order.into();
 
         let order_term = OrderTerm {
-            field: field.to_string(),
+            field: sanitized_field,
             direction: opt.direction,
             numeric: opt.numeric,
             collate: opt.collate,

src/types/select.rs

@@ -30,6 +30,57 @@ pub struct OrderTerm {
     pub collate: bool,
 }
 
+impl OrderTerm {
+    /// Strips trailing SurrealDB order modifiers (`ASC`, `DESC`, `NUMERIC`, `COLLATE`)
+    /// from the end of a field string.
+    ///
+    /// Returns the sanitized field string (trailing modifiers removed).
+    /// Any trailing modifiers are discarded by this function and are not returned;
+    /// they are intended only for diagnostics in callers and do not influence ordering.
+    /// Final ordering is always determined by the explicit [`OrderOptions`]/[`Sort`]
+    /// provided to the API.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use surrealex::types::select::OrderTerm;
+    /// let field = OrderTerm::sanitize_field("name DESC");
+    /// assert_eq!(field, "name");
+    ///
+    /// let field = OrderTerm::sanitize_field("score COLLATE NUMERIC DESC");
+    /// assert_eq!(field, "score");
+    ///
+    /// let field = OrderTerm::sanitize_field("LOWER(name) DESC");
+    /// assert_eq!(field, "LOWER(name)");
+    /// ```
+    pub fn sanitize_field(s: &str) -> String {
+        const MODIFIERS: &[&str] = &["ASC", "DESC", "NUMERIC", "COLLATE"];
+
+        let mut remaining = s.trim_end().to_string();
+
+        loop {
+            let upper = remaining.to_ascii_uppercase();
+            let mut found = false;
+
+            for &modifier in MODIFIERS {
+                let suffix = format!(" {modifier}");
+                if upper.ends_with(&suffix) {
+                    let new_len = remaining.len() - suffix.len();
+                    remaining = remaining[..new_len].trim_end().to_string();
+                    found = true;
+                    break;
+                }
+            }
+
+            if !found {
+                break;
+            }
+        }
+
+        remaining
+    }
+}
+
 impl Display for OrderTerm {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         if self.numeric && self.collate {

tests/select.rs

@@ -509,6 +509,171 @@ fn select_star_and_subquery_field_builds() {
     );
 }
 
+#[test]
+fn order_by_field_with_trailing_desc_uses_default_asc() {
+    // Suffix "DESC" is stripped; OrderOptions::default() => Sort::Asc applies.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("users")
+        .order_by(
+            "name DESC",
+            surrealex::types::select::OrderOptions::default(),
+        )
+        .build();
+    assert_eq!(sql, "SELECT id FROM users ORDER BY name ASC");
+}
+
+#[test]
+fn order_by_field_with_trailing_desc_and_explicit_desc_no_duplication() {
+    // Suffix "DESC" is stripped; explicit Sort::Desc still produces a single DESC.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("users")
+        .order_by("name DESC", Sort::Desc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM users ORDER BY name DESC");
+}
+
+#[test]
+fn order_by_field_with_trailing_desc_explicit_asc_wins() {
+    // Suffix "DESC" is stripped and discarded; explicit Sort::Asc wins.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("users")
+        .order_by("name DESC", Sort::Asc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM users ORDER BY name ASC");
+}
+
+#[test]
+fn order_by_function_call_with_trailing_desc_uses_default_asc() {
+    // Suffix after closing paren is stripped; default Sort::Asc applies.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("users")
+        .order_by("LOWER(name) DESC", ())
+        .build();
+    assert_eq!(sql, "SELECT id FROM users ORDER BY LOWER(name) ASC");
+}
+
+#[test]
+fn order_by_collate_numeric_desc_combination_all_stripped() {
+    // All three trailing modifiers are stripped; default Sort::Asc applies.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("scores")
+        .order_by("score COLLATE NUMERIC DESC", ())
+        .build();
+    assert_eq!(sql, "SELECT id FROM scores ORDER BY score ASC");
+}
+
+#[test]
+fn order_by_trailing_asc_stripped_explicit_desc_wins() {
+    // Trailing "ASC" in field is stripped; explicit Sort::Desc applies.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("name ASC", Sort::Desc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY name DESC");
+}
+
+#[test]
+fn order_by_trailing_numeric_stripped_explicit_sort_applies() {
+    // Trailing "NUMERIC" is stripped; explicit Sort::Asc applies.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("score NUMERIC", Sort::Asc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY score ASC");
+}
+
+#[test]
+fn order_by_no_trailing_modifier_unchanged() {
+    // No suffix present — field passes through unmodified.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("name", Sort::Desc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY name DESC");
+}
+
+#[test]
+fn order_by_lowercase_trailing_desc_stripped() {
+    // Stripping is case-insensitive; lowercase "desc" is stripped.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("name desc", Sort::Asc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY name ASC");
+}
+
+#[test]
+fn order_by_mixed_case_trailing_modifier_stripped() {
+    // Mixed-case modifier is stripped.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("name Desc", Sort::Desc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY name DESC");
+}
+
+#[test]
+fn order_by_field_name_containing_desc_substring_not_stripped() {
+    // "described" does not end with the token " DESC", so nothing is stripped.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("described", Sort::Asc)
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY described ASC");
+}
+
+#[test]
+fn order_by_explicit_numeric_collate_options_still_applied_after_sanitize() {
+    // Even when a suffix is present, explicit numeric+collate flags from Sort::Desc are kept.
+    let sql = QueryBuilder::select(surrealex::fields!("id"))
+        .from("t")
+        .order_by("name DESC", Sort::Desc.collate().numeric())
+        .build();
+    assert_eq!(sql, "SELECT id FROM t ORDER BY name COLLATE NUMERIC DESC");
+}
+
+#[test]
+fn sanitize_field_strips_single_desc() {
+    use surrealex::types::select::OrderTerm;
+    let field = OrderTerm::sanitize_field("name DESC");
+    assert_eq!(field, "name");
+}
+
+#[test]
+fn sanitize_field_strips_collate_numeric_desc_chain() {
+    use surrealex::types::select::OrderTerm;
+    let field = OrderTerm::sanitize_field("score COLLATE NUMERIC DESC");
+    assert_eq!(field, "score");
+}
+
+#[test]
+fn sanitize_field_strips_suffix_after_function_call() {
+    use surrealex::types::select::OrderTerm;
+    let field = OrderTerm::sanitize_field("LOWER(name) DESC");
+    assert_eq!(field, "LOWER(name)");
+}
+
+#[test]
+fn sanitize_field_no_modifier_returns_unchanged() {
+    use surrealex::types::select::OrderTerm;
+    let field = OrderTerm::sanitize_field("name");
+    assert_eq!(field, "name");
+}
+
+#[test]
+fn sanitize_field_case_insensitive_strip() {
+    use surrealex::types::select::OrderTerm;
+    let field = OrderTerm::sanitize_field("name desc");
+    assert_eq!(field, "name");
+}
+
+#[test]
+fn sanitize_field_embedded_desc_substring_not_stripped() {
+    use surrealex::types::select::OrderTerm;
+    let field = OrderTerm::sanitize_field("described");
+    assert_eq!(field, "described");
+}
+
 #[test]
 fn explain_simple_builds() {
     let sql = QueryBuilder::select(surrealex::fields!("id"))