sql: clean up parser module

Author: erikgrinaker

src/sql/engine/local.rs

@@ -20,11 +20,13 @@ use crate::storage::{self, mvcc};
 /// Uses Cow to allow encoding borrowed values but decoding owned values.
 #[derive(Debug, Deserialize, Serialize)]
 pub enum Key<'a> {
-    /// A table schema, keyed by table name.
+    /// A table schema, keyed by table name. The value is a `sql::types::Table`.
     Table(Cow<'a, str>),
     /// A column index entry, keyed by table name, column name, and index value.
+    /// The value is a `BTreeSet` of `sql::types::Value` primary key values.
     Index(Cow<'a, str>, Cow<'a, str>, Cow<'a, Value>),
-    /// A table row, keyed by table name and primary key value.
+    /// A table row, keyed by table name and primary key value. The value is a
+    /// `sql::types::Row`.
     Row(Cow<'a, str>, Cow<'a, Value>),
 }
 

src/sql/engine/session.rs

@@ -31,7 +31,7 @@ impl<'a, E: Engine<'a>> Session<'a, E> {
     pub fn execute(&mut self, statement: &str) -> Result<StatementResult> {
         // Parse and execute the statement. Transaction control is handled here,
         // other statements are handled by the SQL executor.
-        Ok(match Parser::new(statement).parse()? {
+        Ok(match Parser::parse(statement)? {
             // BEGIN: starts a new transaction and returns its state.
             ast::Statement::Begin { read_only, as_of } => {
                 if self.txn.is_some() {

src/sql/mod.rs

@@ -194,7 +194,7 @@ mod tests {
 
             // Output the plan if requested.
             if tags.remove("plan") {
-                let ast = Parser::new(input).parse()?;
+                let ast = Parser::parse(input)?;
                 let plan =
                     session.with_txn(true, |txn| Planner::new(txn).build(ast)?.optimize())?;
                 writeln!(output, "{plan}")?;
@@ -205,7 +205,7 @@ mod tests {
                 if tags.contains("plan") {
                     return Err("using both plan and opt is redundant".into());
                 }
-                let ast = Parser::new(input).parse()?;
+                let ast = Parser::parse(input)?;
                 let plan = session.with_txn(true, |txn| Planner::new(txn).build(ast))?;
                 let Plan::Select(mut root) = plan else {
                     return Err("can only use opt with SELECT plans".into());
@@ -289,11 +289,7 @@ mod tests {
             let mut tags = command.tags.clone();
 
             // Parse and build the expression.
-            let mut parser = Parser::new(input);
-            let ast = parser.parse_expression()?;
-            if let Some(next) = parser.lexer.next().transpose()? {
-                return Err(format!("unconsumed token {next}").into());
-            }
+            let ast = Parser::parse_expr(input)?;
             let expr = Planner::<Catalog>::build_expression(ast, &Scope::new())?;
 
             // Evaluate the expression.

src/sql/parser/ast.rs

@@ -1,48 +1,81 @@
 use std::collections::BTreeMap;
+use std::hash::{Hash, Hasher};
 
 use crate::sql::types::DataType;
 
-/// The statement is the root node of the Abstract Syntax Tree, and describes
-/// the syntactic structure of a SQL query. It is built from a raw SQL string by
-/// the parser, and passed on to the planner which validates it and builds an
+/// SQL statements are represented as an Abstract Syntax Tree (AST). The
+/// statement is the root node of this tree, and describes the syntactic
+/// structure of a SQL statement. It is built from a raw SQL string by the
+/// parser, and passed on to the planner which validates it and builds an
 /// execution plan from it.
 #[derive(Debug)]
 pub enum Statement {
-    /// Begin a new transaction.
-    Begin { read_only: bool, as_of: Option<u64> },
-    /// Commit a transaction.
+    /// BEGIN: begins a new transaction.
+    Begin {
+        /// READ ONLY: if true, begin a read-only transaction.
+        read_only: bool,
+        /// AS OF: if given, the MVCC version to read at.
+        as_of: Option<u64>,
+    },
+    /// COMMIT: commits a transaction.
     Commit,
-    /// Roll back a transaction.
+    /// ROLLBACK: rolls back a transaction.
     Rollback,
-    /// Explain a statement.
+    /// EXPLAIN: explains a SQL statement's execution plan.
     Explain(Box<Statement>),
-    /// Create a new table.
-    CreateTable { name: String, columns: Vec<Column> },
-    /// Drop a table.
-    DropTable { name: String, if_exists: bool },
-    /// Delete matching rows.
-    Delete { table: String, r#where: Option<Expression> },
-    /// Insert new rows into a table.
+    /// CREATE TABLE: creates a new table.
+    CreateTable {
+        /// The table name.
+        name: String,
+        /// Column specifications.
+        columns: Vec<Column>,
+    },
+    /// DROP TABLE: drops a table.
+    DropTable {
+        /// The table to drop.
+        name: String,
+        /// IF EXISTS: if true, don't error if the table doesn't exist.
+        if_exists: bool,
+    },
+    /// DELETE: deletes rows from a table.
+    Delete {
+        /// The table to delete from.
+        table: String,
+        /// WHERE: optional condition to match rows to delete.
+        r#where: Option<Expression>,
+    },
+    /// INSERT INTO: inserts new rows into a table.
     Insert {
+        /// Table to insert into.
         table: String,
-        columns: Option<Vec<String>>, // columns given in values, using default for rest
-        values: Vec<Vec<Expression>>, // rows to insert
+        /// Columns to insert values into. If None, all columns are used.
+        columns: Option<Vec<String>>,
+        /// Row values to insert.
+        values: Vec<Vec<Expression>>,
     },
-    /// Update rows in a table.
+    /// UPDATE: updates rows in a table.
     Update {
         table: String,
         set: BTreeMap<String, Option<Expression>>, // column → value, None for default value
         r#where: Option<Expression>,
     },
-    /// Select matching rows.
+    /// SELECT: selects rows, possibly from a table.
     Select {
-        select: Vec<(Expression, Option<String>)>, // optional column aliases
+        /// Expressions to select, with an optional column alias.
+        select: Vec<(Expression, Option<String>)>,
+        /// FROM: tables to select from.
         from: Vec<From>,
+        /// WHERE: optional condition to filter rows.
         r#where: Option<Expression>,
+        /// GROUP BY: expressions to group and aggregate by.
         group_by: Vec<Expression>,
+        /// HAVING: expression to filter groups by.
         having: Option<Expression>,
+        /// ORDER BY: expresisions to sort by, with direction.
         order_by: Vec<(Expression, Direction)>,
+        /// OFFSET: row offset to start from.
         offset: Option<Expression>,
+        /// LIMIT: maximum number of rows to return.
         limit: Option<Expression>,
     },
 }
@@ -51,9 +84,23 @@ pub enum Statement {
 #[derive(Debug)]
 pub enum From {
     /// A table.
-    Table { name: String, alias: Option<String> },
+    Table {
+        /// The table name.
+        name: String,
+        /// An optional alias for the table.
+        alias: Option<String>,
+    },
     /// A join of two or more tables (may be nested).
-    Join { left: Box<From>, right: Box<From>, r#type: JoinType, predicate: Option<Expression> },
+    Join {
+        /// The left table to join,
+        left: Box<From>,
+        /// The right table to join.
+        right: Box<From>,
+        /// The join type.
+        r#type: JoinType,
+        /// The join condition. None for a cross join.
+        predicate: Option<Expression>,
+    },
 }
 
 /// A CREATE TABLE column definition.
@@ -90,13 +137,14 @@ impl JoinType {
 }
 
 /// ORDER BY direction.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub enum Direction {
+    #[default]
     Ascending,
     Descending,
 }
 
-/// Expressions. Can be nested.
+/// SQL expressions, e.g. `a + 7 > b`. Can be nested.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub enum Expression {
     /// All columns, i.e. *.
@@ -121,27 +169,27 @@ pub enum Literal {
     String(String),
 }
 
-/// To allow using Expressions and Literals in e.g. hashmaps, implement simple
-/// equality and hash for all types, including Null and f64::NAN. This is not
-/// used for expression evaluation (handled by sql::types::Expression), where
-/// these values should not be considered equal to themselves, only in lookups.
-impl std::cmp::PartialEq for Literal {
+/// To allow using expressions and literals in e.g. hashmaps, implement simple
+/// equality by value for all types, including Null and f64::NAN. This only
+/// checks that the values are the same, and ignores SQL semantics for e.g. NULL
+/// and NaN (which is handled by SQL expression evaluation).
+impl PartialEq for Literal {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
+            (Self::Null, Self::Null) => true,
             (Self::Boolean(l), Self::Boolean(r)) => l == r,
             (Self::Integer(l), Self::Integer(r)) => l == r,
-            // Implies NaN == NaN but -NaN != NaN. Similarly with +/-0.0.
             (Self::Float(l), Self::Float(r)) => l.to_bits() == r.to_bits(),
             (Self::String(l), Self::String(r)) => l == r,
-            (l, r) => core::mem::discriminant(l) == core::mem::discriminant(r),
+            (_, _) => false,
         }
     }
 }
 
-impl std::cmp::Eq for Literal {}
+impl Eq for Literal {}
 
-impl std::hash::Hash for Literal {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+impl Hash for Literal {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         core::mem::discriminant(self).hash(state);
         match self {
             Self::Null => {}
@@ -166,7 +214,7 @@ pub enum Operator {
 
     Equal(Box<Expression>, Box<Expression>),       // a = b
     GreaterThan(Box<Expression>, Box<Expression>), // a > b
-    GreaterThanOrEqual(Box<Expression>, Box<Expression>), // a != b
+    GreaterThanOrEqual(Box<Expression>, Box<Expression>), // a >= b
     Is(Box<Expression>, Literal),                  // IS NULL or IS NAN
     LessThan(Box<Expression>, Box<Expression>),    // a < b
     LessThanOrEqual(Box<Expression>, Box<Expression>), // a <= b
@@ -190,31 +238,33 @@ impl Expression {
     /// Halts and returns false if the closure returns false.
     pub fn walk(&self, visitor: &mut impl FnMut(&Expression) -> bool) -> bool {
         use Operator::*;
+
         if !visitor(self) {
             return false;
         }
+
         match self {
-            Self::Operator(Add(lhs, rhs))
-            | Self::Operator(And(lhs, rhs))
-            | Self::Operator(Divide(lhs, rhs))
-            | Self::Operator(Equal(lhs, rhs))
-            | Self::Operator(Exponentiate(lhs, rhs))
-            | Self::Operator(GreaterThan(lhs, rhs))
-            | Self::Operator(GreaterThanOrEqual(lhs, rhs))
-            | Self::Operator(LessThan(lhs, rhs))
-            | Self::Operator(LessThanOrEqual(lhs, rhs))
-            | Self::Operator(Like(lhs, rhs))
-            | Self::Operator(Multiply(lhs, rhs))
-            | Self::Operator(NotEqual(lhs, rhs))
-            | Self::Operator(Or(lhs, rhs))
-            | Self::Operator(Remainder(lhs, rhs))
-            | Self::Operator(Subtract(lhs, rhs)) => lhs.walk(visitor) && rhs.walk(visitor),
+            Self::Operator(op) => match op {
+                Add(lhs, rhs)
+                | And(lhs, rhs)
+                | Divide(lhs, rhs)
+                | Equal(lhs, rhs)
+                | Exponentiate(lhs, rhs)
+                | GreaterThan(lhs, rhs)
+                | GreaterThanOrEqual(lhs, rhs)
+                | LessThan(lhs, rhs)
+                | LessThanOrEqual(lhs, rhs)
+                | Like(lhs, rhs)
+                | Multiply(lhs, rhs)
+                | NotEqual(lhs, rhs)
+                | Or(lhs, rhs)
+                | Remainder(lhs, rhs)
+                | Subtract(lhs, rhs) => lhs.walk(visitor) && rhs.walk(visitor),
 
-            Self::Operator(Factorial(expr))
-            | Self::Operator(Identity(expr))
-            | Self::Operator(Is(expr, _))
-            | Self::Operator(Negate(expr))
-            | Self::Operator(Not(expr)) => expr.walk(visitor),
+                Factorial(expr) | Identity(expr) | Is(expr, _) | Negate(expr) | Not(expr) => {
+                    expr.walk(visitor)
+                }
+            },
 
             Self::Function(_, exprs) => exprs.iter().any(|expr| expr.walk(visitor)),
 
@@ -230,39 +280,40 @@ impl Expression {
 
     /// Find and collects expressions for which the given closure returns true,
     /// adding them to c. Does not recurse into matching expressions.
-    pub fn collect(&self, visitor: &impl Fn(&Expression) -> bool, c: &mut Vec<Expression>) {
+    pub fn collect(&self, visitor: &impl Fn(&Expression) -> bool, exprs: &mut Vec<Expression>) {
+        use Operator::*;
+
         if visitor(self) {
-            c.push(self.clone());
+            exprs.push(self.clone());
             return;
         }
-        use Operator::*;
-        match self {
-            Self::Operator(Add(lhs, rhs))
-            | Self::Operator(And(lhs, rhs))
-            | Self::Operator(Divide(lhs, rhs))
-            | Self::Operator(Equal(lhs, rhs))
-            | Self::Operator(Exponentiate(lhs, rhs))
-            | Self::Operator(GreaterThan(lhs, rhs))
-            | Self::Operator(GreaterThanOrEqual(lhs, rhs))
-            | Self::Operator(LessThan(lhs, rhs))
-            | Self::Operator(LessThanOrEqual(lhs, rhs))
-            | Self::Operator(Like(lhs, rhs))
-            | Self::Operator(Multiply(lhs, rhs))
-            | Self::Operator(NotEqual(lhs, rhs))
-            | Self::Operator(Or(lhs, rhs))
-            | Self::Operator(Remainder(lhs, rhs))
-            | Self::Operator(Subtract(lhs, rhs)) => {
-                lhs.collect(visitor, c);
-                rhs.collect(visitor, c);
-            }
 
-            Self::Operator(Factorial(expr))
-            | Self::Operator(Identity(expr))
-            | Self::Operator(Is(expr, _))
-            | Self::Operator(Negate(expr))
-            | Self::Operator(Not(expr)) => expr.collect(visitor, c),
+        match self {
+            Self::Operator(op) => match op {
+                Add(lhs, rhs)
+                | And(lhs, rhs)
+                | Divide(lhs, rhs)
+                | Equal(lhs, rhs)
+                | Exponentiate(lhs, rhs)
+                | GreaterThan(lhs, rhs)
+                | GreaterThanOrEqual(lhs, rhs)
+                | LessThan(lhs, rhs)
+                | LessThanOrEqual(lhs, rhs)
+                | Like(lhs, rhs)
+                | Multiply(lhs, rhs)
+                | NotEqual(lhs, rhs)
+                | Or(lhs, rhs)
+                | Remainder(lhs, rhs)
+                | Subtract(lhs, rhs) => {
+                    lhs.collect(visitor, exprs);
+                    rhs.collect(visitor, exprs);
+                }
+                Factorial(expr) | Identity(expr) | Is(expr, _) | Negate(expr) | Not(expr) => {
+                    expr.collect(visitor, exprs);
+                }
+            },
 
-            Self::Function(_, exprs) => exprs.iter().for_each(|expr| expr.collect(visitor, c)),
+            Self::Function(_, args) => args.iter().for_each(|arg| arg.collect(visitor, exprs)),
 
             Self::All | Self::Column(_, _) | Self::Literal(_) => {}
         }