Avoid panicking in certain unsupported SQL paths (#121)

Prior to this commit, it was possible for certain CLI `spacetime sql` invocations
to crash SpacetimeDB by including unsupported features.
This was due to the SQL parser using the `todo!` macro,
where it should have returned a `PlanError::Unsupported` error.

With this commit, several uses of `todo!` are rewritten
to instead return errors,
which are translated appropriately into 400 responses,
rather than crashing SpacetimeDB.

crates/core/src/sql/ast.rs

@@ -338,7 +338,9 @@ fn compile_expr_value(table: &From, field: Option<&ProductTypeElement>, of: SqlE
 fn compile_expr_field(table: &From, field: Option<&ProductTypeElement>, of: SqlExpr) -> Result<FieldExpr, PlanError> {
     match compile_expr_value(table, field, of)? {
         ColumnOp::Field(field) => Ok(field),
-        x => todo!("Complex expression {x} on insert..."),
+        x => Err(PlanError::Unsupported {
+            feature: format!("Complex expression {x} on insert..."),
+        }),
     }
 }
 
@@ -475,8 +477,12 @@ fn compile_from(db: &RelationalDB, tx: &MutTxId, from: &[TableWithJoins]) -> Res
                                     (ColumnOp::Field(FieldExpr::Name(lhs)), ColumnOp::Field(FieldExpr::Name(rhs))) => {
                                         (lhs, rhs)
                                     }
-                                    _ => {
-                                        todo!()
+                                    (lhs, rhs) => {
+                                        return Err(PlanError::Unsupported {
+                                            feature: format!(
+                                                "Can't compare non-field expressions {lhs} and {rhs} in JOIN clause"
+                                            ),
+                                        });
                                     }
                                 };
 
@@ -523,14 +529,14 @@ fn compile_select_item(from: &From, select_item: SelectItem) -> Result<Column, P
                 let value = compile_expr_value(from, None, expr)?;
                 match value {
                     ColumnOp::Field(value) => match value {
-                        FieldExpr::Name(_) => {
-                            unreachable!("Should not be an identifier in Expr::Value")
-                        }
+                        FieldExpr::Name(_) => Err(PlanError::Unsupported {
+                            feature: "Should not be an identifier in Expr::Value".to_string(),
+                        }),
                         FieldExpr::Value(x) => Ok(Column::UnnamedExpr(Expr::Value(x))),
                     },
-                    x => {
-                        unreachable!("Should not be an {} in Expr::Value", x)
-                    }
+                    x => Err(PlanError::Unsupported {
+                        feature: format!("Should not be an {x} in Expr::Value"),
+                    }),
                 }
             }
             sqlparser::ast::Expr::Nested(x) => compile_select_item(from, SelectItem::UnnamedExpr(*x)),

crates/core/src/vm.rs

@@ -149,24 +149,21 @@ impl<'db, 'tx> DbProgram<'db, 'tx> {
 
     fn _execute_insert(&mut self, table: &Table, rows: Vec<ProductValue>) -> Result<Code, ErrorVm> {
         match table {
-            Table::MemTable(_) => {
-                todo!("How deal with mutating values?")
-            }
+            // TODO: How do we deal with mutating values?
+            Table::MemTable(_) => Err(ErrorVm::Other(anyhow::anyhow!("How deal with mutating values?"))),
             Table::DbTable(x) => {
                 for row in rows {
                     self.db.insert(self.tx, x.table_id, row)?;
                 }
+                Ok(Code::Pass)
             }
         }
-
-        Ok(Code::Pass)
     }
 
     fn _execute_delete(&mut self, table: &Table, rows: Vec<ProductValue>) -> Result<Code, ErrorVm> {
         match table {
-            Table::MemTable(_) => {
-                todo!("How deal with mutating values?")
-            }
+            // TODO: How do we deal with mutating values?
+            Table::MemTable(_) => Err(ErrorVm::Other(anyhow::anyhow!("How deal with mutating values?"))),
             Table::DbTable(t) => {
                 let count = self.db.delete_by_rel(self.tx, t.table_id, rows)?;
                 Ok(Code::Value(count.unwrap_or_default().into()))