diff --git a/phono-models/migrations/20260213180250_text_filters.down.sql b/phono-models/migrations/20260213180250_text_filters.down.sql new file mode 100644 index 0000000..5d0f312 --- /dev/null +++ b/phono-models/migrations/20260213180250_text_filters.down.sql @@ -0,0 +1,2 @@ +alter table portals add column if not exists table_filter jsonb not null default 'null'; +alter table portals drop column if exists filter; diff --git a/phono-models/migrations/20260213180250_text_filters.up.sql b/phono-models/migrations/20260213180250_text_filters.up.sql new file mode 100644 index 0000000..0c892d2 --- /dev/null +++ b/phono-models/migrations/20260213180250_text_filters.up.sql @@ -0,0 +1,5 @@ +alter table portals add column if not exists filter text not null default ''; +-- This is irreversible and ordinarily should be run in a later migration, but +-- it's being rolled out while manually verifying that there will be negligible +-- impact to users, so I'm folding it into this migration for convenience. +alter table portals drop column if exists table_filter; diff --git a/phono-models/src/expression.rs b/phono-models/src/expression.rs deleted file mode 100644 index fb65ef4..0000000 --- a/phono-models/src/expression.rs +++ /dev/null @@ -1,162 +0,0 @@ -use std::fmt::Display; - -use phono_pestgros::{Datum, QueryFragment, escape_identifier}; -use serde::{Deserialize, Serialize}; - -/// Building block of a syntax tree for a constrained subset of SQL that can be -/// statically analyzed, to validate that user-provided expressions perform only -/// operations that are read-only and otherwise safe to execute. -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -#[serde(tag = "t", content = "c")] -pub enum PgExpressionAny { - Comparison(PgComparisonExpression), - Identifier(PgIdentifierExpression), - Literal(Datum), - ToJson(PgToJsonExpression), -} - -impl PgExpressionAny { - pub fn into_query_fragment(self) -> QueryFragment { - match self { - Self::Comparison(expr) => expr.into_query_fragment(), - Self::Identifier(expr) => expr.into_query_fragment(), - Self::Literal(expr) => { - if expr.is_none() { - QueryFragment::from_sql("null") - } else { - QueryFragment::from_param(expr) - } - } - Self::ToJson(expr) => expr.into_query_fragment(), - } - } -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -#[serde(tag = "t", content = "c")] -pub enum PgComparisonExpression { - Infix(PgInfixExpression), - IsNull(PgIsNullExpression), - IsNotNull(PgIsNotNullExpression), -} - -impl PgComparisonExpression { - fn into_query_fragment(self) -> QueryFragment { - match self { - Self::Infix(expr) => expr.into_query_fragment(), - Self::IsNull(expr) => expr.into_query_fragment(), - Self::IsNotNull(expr) => expr.into_query_fragment(), - } - } -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct PgInfixExpression { - pub operator: T, - pub lhs: Box, - pub rhs: Box, -} - -impl PgInfixExpression { - fn into_query_fragment(self) -> QueryFragment { - QueryFragment::concat([ - QueryFragment::from_sql("(("), - self.lhs.into_query_fragment(), - QueryFragment::from_sql(&format!(") {} (", self.operator)), - self.rhs.into_query_fragment(), - QueryFragment::from_sql("))"), - ]) - } -} - -#[derive(Clone, Debug, strum::Display, Deserialize, PartialEq, Serialize)] -pub enum PgComparisonOperator { - #[strum(to_string = "and")] - And, - #[strum(to_string = "=")] - Eq, - #[strum(to_string = ">")] - Gt, - #[strum(to_string = "<")] - Lt, - #[strum(to_string = "<>")] - Neq, - #[strum(to_string = "or")] - Or, -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct PgIsNullExpression { - lhs: Box, -} - -impl PgIsNullExpression { - fn into_query_fragment(self) -> QueryFragment { - QueryFragment::concat([ - QueryFragment::from_sql("(("), - self.lhs.into_query_fragment(), - QueryFragment::from_sql(") is null)"), - ]) - } -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct PgIsNotNullExpression { - lhs: Box, -} - -impl PgIsNotNullExpression { - fn into_query_fragment(self) -> QueryFragment { - QueryFragment::concat([ - QueryFragment::from_sql("(("), - self.lhs.into_query_fragment(), - QueryFragment::from_sql(") is not null)"), - ]) - } -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct PgIdentifierExpression { - pub parts_raw: Vec, -} - -impl PgIdentifierExpression { - fn into_query_fragment(self) -> QueryFragment { - QueryFragment::join( - self.parts_raw - .iter() - .map(|part| QueryFragment::from_sql(&escape_identifier(part))), - QueryFragment::from_sql("."), - ) - } -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct PgToJsonExpression { - entries: Vec<(String, PgExpressionAny)>, -} - -impl PgToJsonExpression { - /// Generates a query fragment to the effect of: - /// `to_json((select ($expr) as "ident", ($expr2) as "ident2"))` - fn into_query_fragment(self) -> QueryFragment { - if self.entries.is_empty() { - QueryFragment::from_sql("'{}'") - } else { - QueryFragment::concat([ - QueryFragment::from_sql("to_json((select "), - QueryFragment::join( - self.entries.into_iter().map(|(key, value)| { - QueryFragment::concat([ - QueryFragment::from_sql("("), - value.into_query_fragment(), - QueryFragment::from_sql(&format!(") as {}", escape_identifier(&key))), - ]) - }), - QueryFragment::from_sql(", "), - ), - QueryFragment::from_sql("))"), - ]) - } - } -} diff --git a/phono-models/src/lib.rs b/phono-models/src/lib.rs index 869931d..6edf586 100644 --- a/phono-models/src/lib.rs +++ b/phono-models/src/lib.rs @@ -18,7 +18,6 @@ pub mod accessors; pub mod client; pub mod cluster; pub mod errors; -pub mod expression; pub mod field; pub mod language; mod macros; diff --git a/phono-models/src/portal.rs b/phono-models/src/portal.rs index f64ad0e..2e23bd2 100644 --- a/phono-models/src/portal.rs +++ b/phono-models/src/portal.rs @@ -3,13 +3,11 @@ use std::sync::LazyLock; use derive_builder::Builder; use regex::Regex; use serde::Serialize; -use sqlx::{postgres::types::Oid, query, query_as, types::Json}; +use sqlx::{postgres::types::Oid, query, query_as}; use uuid::Uuid; use validator::Validate; -use crate::{ - client::AppDbClient, errors::QueryError, expression::PgExpressionAny, macros::with_id_query, -}; +use crate::{client::AppDbClient, errors::QueryError, macros::with_id_query}; pub static RE_PORTAL_NAME: LazyLock = LazyLock::new(|| Regex::new(r"^[a-zA-Z0-9][()a-zA-Z0-9 _-]*[a-zA-Z0-9()_-]$").unwrap()); @@ -36,7 +34,7 @@ pub struct Portal { /// JSONB-encoded expression to use for filtering rows in the web-based /// table view. - pub table_filter: Json>, + pub filter: String, } impl Portal { @@ -65,7 +63,7 @@ select workspace_id, class_oid, form_public, - table_filter as "table_filter: Json>" + filter from portals where id = $1 "#, @@ -87,7 +85,7 @@ select workspace_id, class_oid, form_public, - table_filter as "table_filter: Json>" + filter from portals where workspace_id = $1 "#, @@ -122,7 +120,7 @@ select workspace_id, class_oid, form_public, - table_filter as "table_filter: Json>" + filter from portals where workspace_id = $1 and class_oid = $2 "#, @@ -161,7 +159,7 @@ returning workspace_id, class_oid, form_public, - table_filter as "table_filter: Json>" + filter "#, self.workspace_id, self.class_oid, @@ -180,7 +178,7 @@ pub struct Update { form_public: Option, #[builder(default, setter(strip_option = true))] - table_filter: Option>, + filter: Option, #[builder(default, setter(strip_option = true))] #[validate(regex(path = *RE_PORTAL_NAME))] @@ -196,16 +194,16 @@ impl Update { query!( "update portals set form_public = $1 where id = $2", form_public, - self.id + self.id, ) .execute(app_db.get_conn()) .await?; } - if let Some(table_filter) = self.table_filter { + if let Some(filter) = self.filter { query!( - "update portals set table_filter = $1 where id = $2", - Json(table_filter) as Json>, - self.id + "update portals set filter = $1 where id = $2", + filter, + self.id, ) .execute(app_db.get_conn()) .await?; diff --git a/phono-pestgros/src/func_invocation_tests.rs b/phono-pestgros/src/func_invocation_tests.rs index 4372ecf..f548ad1 100644 --- a/phono-pestgros/src/func_invocation_tests.rs +++ b/phono-pestgros/src/func_invocation_tests.rs @@ -1,6 +1,6 @@ use std::error::Error; -use crate::{ArithOp, Datum, Expr, FnArgs, InfixOp}; +use crate::{Datum, Expr, FnArgs, InfixOp}; #[test] fn parses_without_args() -> Result<(), Box> { @@ -29,7 +29,7 @@ fn parses_with_args() -> Result<(), Box> { Expr::Literal(Datum::Text(Some("hello!".to_owned()))), Expr::Infix { lhs: Box::new(Expr::Literal(Datum::Numeric(Some(1.into())))), - op: InfixOp::ArithInfix(ArithOp::Add), + op: InfixOp::Add, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(2.into())))), } ], diff --git a/phono-pestgros/src/lib.rs b/phono-pestgros/src/lib.rs index be7404f..2b12a16 100644 --- a/phono-pestgros/src/lib.rs +++ b/phono-pestgros/src/lib.rs @@ -155,6 +155,7 @@ static PRATT_PARSER: LazyLock> = LazyLock::new(|| { /// operators that theoretically evaluates to some value, such as a boolean /// condition, an object name, or a string dynamically derived from other /// values. An expression is *not* a complete SQL statement, command, or query. +#[non_exhaustive] #[derive(Clone, Debug, PartialEq)] pub enum Expr { Infix { @@ -186,23 +187,17 @@ impl TryFrom<&str> for Expr { } } +#[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq)] pub enum InfixOp { - ArithInfix(ArithOp), - BoolInfix(BoolOp), -} - -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum ArithOp { + // Arithmetic ops: Add, Concat, Div, Mult, Sub, -} -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum BoolOp { + // Boolean ops: And, Or, Eq, @@ -277,19 +272,19 @@ fn parse_expr_pairs(expr_pairs: Pairs<'_, Rule>) -> Result { .map_infix(|lhs, op, rhs| Ok(Expr::Infix { lhs: Box::new(lhs?), op: match op.as_rule() { - Rule::Add => InfixOp::ArithInfix(ArithOp::Add), - Rule::ConcatInfixOp => InfixOp::ArithInfix(ArithOp::Concat), - Rule::Divide => InfixOp::ArithInfix(ArithOp::Div), - Rule::Multiply => InfixOp::ArithInfix(ArithOp::Mult), - Rule::Subtract => InfixOp::ArithInfix(ArithOp::Sub), - Rule::And => InfixOp::BoolInfix(BoolOp::And), - Rule::Eq => InfixOp::BoolInfix(BoolOp::Eq), - Rule::Gt => InfixOp::BoolInfix(BoolOp::Gt), - Rule::GtEq => InfixOp::BoolInfix(BoolOp::Gte), - Rule::Lt => InfixOp::BoolInfix(BoolOp::Lt), - Rule::LtEq => InfixOp::BoolInfix(BoolOp::Lte), - Rule::NotEq => InfixOp::BoolInfix(BoolOp::Neq), - Rule::Or => InfixOp::BoolInfix(BoolOp::Or), + Rule::Add => InfixOp::Add, + Rule::ConcatInfixOp => InfixOp::Concat, + Rule::Divide => InfixOp::Div, + Rule::Multiply => InfixOp::Mult, + Rule::Subtract => InfixOp::Sub, + Rule::And => InfixOp::And, + Rule::Eq => InfixOp::Eq, + Rule::Gt => InfixOp::Gt, + Rule::GtEq => InfixOp::Gte, + Rule::Lt => InfixOp::Lt, + Rule::LtEq => InfixOp::Lte, + Rule::NotEq => InfixOp::Neq, + Rule::Or => InfixOp::Or, rule => Err(ParseError::UnknownRule(rule))?, }, rhs: Box::new(rhs?), diff --git a/phono-pestgros/src/op_tests.rs b/phono-pestgros/src/op_tests.rs index fbeab91..c912f73 100644 --- a/phono-pestgros/src/op_tests.rs +++ b/phono-pestgros/src/op_tests.rs @@ -1,6 +1,6 @@ //! Unit tests for infix operator parsing within expressions. -use crate::{ArithOp, Datum, Expr, InfixOp}; +use crate::{Datum, Expr, InfixOp}; #[test] fn add_op_parses() { @@ -9,7 +9,7 @@ fn add_op_parses() { Expr::try_from("six + 7"), Ok(Expr::Infix { lhs: Box::new(Expr::ObjName(vec!["six".to_owned()])), - op: InfixOp::ArithInfix(ArithOp::Add), + op: InfixOp::Add, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(7.into())))), }) ); @@ -21,7 +21,7 @@ fn mult_op_parses() { Expr::try_from("six * 7"), Ok(Expr::Infix { lhs: Box::new(Expr::ObjName(vec!["six".to_owned()])), - op: InfixOp::ArithInfix(ArithOp::Mult), + op: InfixOp::Mult, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(7.into())))), }) ); @@ -35,13 +35,13 @@ fn arith_precedence() { lhs: Box::new(Expr::Infix { lhs: Box::new(Expr::Infix { lhs: Box::new(Expr::Literal(Datum::Numeric(Some(1.into())))), - op: InfixOp::ArithInfix(ArithOp::Add), + op: InfixOp::Add, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(2.into())))), }), - op: InfixOp::ArithInfix(ArithOp::Mult), + op: InfixOp::Mult, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(3.into())))), }), - op: InfixOp::ArithInfix(ArithOp::Add), + op: InfixOp::Add, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(4.into())))), }) ); @@ -49,13 +49,13 @@ fn arith_precedence() { Expr::try_from("1 - 2 / (3 - 4)"), Ok(Expr::Infix { lhs: Box::new(Expr::Literal(Datum::Numeric(Some(1.into())))), - op: InfixOp::ArithInfix(ArithOp::Sub), + op: InfixOp::Sub, rhs: Box::new(Expr::Infix { lhs: Box::new(Expr::Literal(Datum::Numeric(Some(2.into())))), - op: InfixOp::ArithInfix(ArithOp::Div), + op: InfixOp::Div, rhs: Box::new(Expr::Infix { lhs: Box::new(Expr::Literal(Datum::Numeric(Some(3.into())))), - op: InfixOp::ArithInfix(ArithOp::Sub), + op: InfixOp::Sub, rhs: Box::new(Expr::Literal(Datum::Numeric(Some(4.into())))), }), }) diff --git a/phono-pestgros/src/query_builders.rs b/phono-pestgros/src/query_builders.rs index 239abeb..b642422 100644 --- a/phono-pestgros/src/query_builders.rs +++ b/phono-pestgros/src/query_builders.rs @@ -3,7 +3,7 @@ use sqlx::{Postgres, QueryBuilder}; -use crate::{ArithOp, BoolOp, Datum, Expr, FnArgs, InfixOp, escape_identifier}; +use crate::{Datum, Expr, FnArgs, InfixOp, escape_identifier}; /// Representation of a partial, parameterized SQL query. Allows callers to /// build queries iteratively and dynamically, handling parameter numbering @@ -183,19 +183,19 @@ impl From for QueryFragment { impl From for QueryFragment { fn from(value: InfixOp) -> Self { Self::from_sql(match value { - InfixOp::ArithInfix(ArithOp::Add) => "+", - InfixOp::ArithInfix(ArithOp::Concat) => "||", - InfixOp::ArithInfix(ArithOp::Div) => "/", - InfixOp::ArithInfix(ArithOp::Mult) => "*", - InfixOp::ArithInfix(ArithOp::Sub) => "-", - InfixOp::BoolInfix(BoolOp::And) => "and", - InfixOp::BoolInfix(BoolOp::Or) => "or", - InfixOp::BoolInfix(BoolOp::Eq) => "=", - InfixOp::BoolInfix(BoolOp::Gt) => ">", - InfixOp::BoolInfix(BoolOp::Gte) => ">=", - InfixOp::BoolInfix(BoolOp::Lt) => "<", - InfixOp::BoolInfix(BoolOp::Lte) => "<=", - InfixOp::BoolInfix(BoolOp::Neq) => "<>", + InfixOp::Add => "+", + InfixOp::Concat => "||", + InfixOp::Div => "/", + InfixOp::Mult => "*", + InfixOp::Sub => "-", + InfixOp::And => "and", + InfixOp::Or => "or", + InfixOp::Eq => "=", + InfixOp::Gt => ">", + InfixOp::Gte => ">=", + InfixOp::Lt => "<", + InfixOp::Lte => "<=", + InfixOp::Neq => "<>", }) } } diff --git a/phono-server/src/routes/relations_single/get_data_handler.rs b/phono-server/src/routes/relations_single/get_data_handler.rs index 0ad2e46..bc0c67d 100644 --- a/phono-server/src/routes/relations_single/get_data_handler.rs +++ b/phono-server/src/routes/relations_single/get_data_handler.rs @@ -8,16 +8,14 @@ use axum::{ use phono_backends::{pg_acl::PgPrivilegeType, pg_attribute::PgAttribute, pg_class::PgClass}; use phono_models::{ accessors::{Accessor, Actor, portal::PortalAccessor}, - expression::PgExpressionAny, field::Field, }; -use phono_pestgros::{Datum, QueryFragment, escape_identifier}; +use phono_pestgros::{Datum, Expr, FnArgs, InfixOp, QueryFragment, escape_identifier}; use serde::{Deserialize, Serialize}; use sqlx::{ Postgres, QueryBuilder, postgres::{PgRow, types::Oid}, }; -use tracing::debug; use uuid::Uuid; use validator::Validate; @@ -39,62 +37,12 @@ pub(super) struct PathParams { #[derive(Debug, Deserialize, Validate)] pub(super) struct FormBody { - subfilter: Option, + #[serde(default)] + subfilter: String, } const FRONTEND_ROW_LIMIT: i64 = 1000; -/// Helper type to make it easier to build and reason about multiple related SQL -/// queries. -#[derive(Clone, Debug)] -pub struct SelectQuery { - /// Query fragment following (not including) "select ". - pub selection: QueryFragment, - - /// Query fragment following (not including) "from ". - pub source: QueryFragment, - - /// Query fragment following (not including) "where ", or empty if not - /// applicable. - pub filters: QueryFragment, - - /// Query fragment following (not including) "order by ", or empty if not - /// applicable. - pub order: QueryFragment, - - /// Query fragment following (not including) "limit ", or empty if not - /// applicable. - pub limit: QueryFragment, -} - -impl From for QueryFragment { - fn from(value: SelectQuery) -> Self { - let mut result = QueryFragment::from_sql("select "); - result.push(value.selection); - result.push(QueryFragment::from_sql(" from ")); - result.push(value.source); - if !value.filters.is_empty() { - result.push(QueryFragment::from_sql(" where ")); - result.push(value.filters); - } - if !value.order.is_empty() { - result.push(QueryFragment::from_sql(" order by ")); - result.push(value.order); - } - if !value.limit.is_empty() { - result.push(QueryFragment::from_sql(" limit ")); - result.push(value.limit); - } - result - } -} - -impl From for QueryBuilder<'_, Postgres> { - fn from(value: SelectQuery) -> Self { - QueryFragment::from(value).into() - } -} - /// HTTP GET handler for an API endpoint returning a JSON encoding of portal /// data to display in a table or similar form. If the `subfilter` URL parameter /// is specified, it is `&&`-ed with the portal's stored filter. @@ -176,24 +124,8 @@ pub(super) async fn get( )), filters: QueryFragment::join( [ - portal - .table_filter - .0 - .map(|filter| filter.into_query_fragment()), - form.subfilter - .and_then(|value| { - if value.is_empty() { - None - } else { - serde_json::from_str::>(&value) - // Ignore invalid input. A user likely pasted incorrectly - // or made a typo. - .inspect_err(|_| debug!("ignoring invalid subfilter expression")) - .ok() - .flatten() - } - }) - .map(|filter| filter.into_query_fragment()), + into_safe_filter_sql(&portal.filter), + into_safe_filter_sql(&form.subfilter), ] .into_iter() .flatten(), @@ -263,3 +195,155 @@ pub(super) async fn get( }) .into_response()) } + +/// Helper type to make it easier to build and reason about multiple related SQL +/// queries. +#[derive(Clone, Debug)] +pub struct SelectQuery { + /// Query fragment following (not including) "select ". + pub selection: QueryFragment, + + /// Query fragment following (not including) "from ". + pub source: QueryFragment, + + /// Query fragment following (not including) "where ", or empty if not + /// applicable. + pub filters: QueryFragment, + + /// Query fragment following (not including) "order by ", or empty if not + /// applicable. + pub order: QueryFragment, + + /// Query fragment following (not including) "limit ", or empty if not + /// applicable. + pub limit: QueryFragment, +} + +impl From for QueryFragment { + fn from(value: SelectQuery) -> Self { + let mut result = QueryFragment::from_sql("select "); + result.push(value.selection); + result.push(QueryFragment::from_sql(" from ")); + result.push(value.source); + if !value.filters.is_empty() { + result.push(QueryFragment::from_sql(" where ")); + result.push(value.filters); + } + if !value.order.is_empty() { + result.push(QueryFragment::from_sql(" order by ")); + result.push(value.order); + } + if !value.limit.is_empty() { + result.push(QueryFragment::from_sql(" limit ")); + result.push(value.limit); + } + result + } +} + +impl From for QueryBuilder<'_, Postgres> { + fn from(value: SelectQuery) -> Self { + QueryFragment::from(value).into() + } +} + +/// Users are allowed to put any text they want in the `Portal.filter` field. +/// This needs to be either transformed into a SQL expression that we trust to +/// be injected into a `WHERE` clause or disregarded if no such expression can +/// be generated. +/// +/// Given the known (not to mention unknown) limitations of [`phono_pestgros`]'s +/// homegrown PostgreSQL grammar, trying to positively establish the correctness +/// and trustworthiness of a filter expression exactly as written would be +/// impractical and dangerous. Instead, we validate the syntax tree as parsed by +/// [`phono_pestgros`] (even if the parsing logic isn't spec-compliant), and use +/// the SQL expression only after it has been converted back from parsed form. +fn into_safe_filter_sql(expr_text: &str) -> Option { + if let Ok(expr) = Expr::try_from(expr_text) + && is_safe_filter_expr(&expr) + { + Some(expr.into()) + } else { + None + } +} + +fn is_safe_filter_expr(expr: &Expr) -> bool { + match expr { + &Expr::Literal(_) | &Expr::ObjName(_) => true, + &Expr::Infix { + ref lhs, + op, + ref rhs, + } => match op { + // Most if not all infix operators should be safe, but enumerate + // them just in case. + + // Arithmetic: + InfixOp::Add + | InfixOp::Concat + | InfixOp::Div + | InfixOp::Mult + | InfixOp::Sub + // Boolean: + | InfixOp::And + | InfixOp::Or + | InfixOp::Eq + | InfixOp::Gt + | InfixOp::Gte + | InfixOp::Lt + | InfixOp::Lte + | InfixOp::Neq => is_safe_filter_expr(lhs) && is_safe_filter_expr(rhs), + _ => false, + }, + &Expr::FnCall { ref name, ref args } => match name + .iter() + .map(|s| s.as_str()) + .collect::>() + .as_slice() + { + // Math: + &["abs"] + | &["ceil"] + | &["floor"] + | &["ln"] + | &["log"] + | &["mod"] + | &["power"] + | &["pi"] + | &["round"] + | &["sqrt"] + | &["trunc"] + // Timestamp: + | &["now"] + | &["to_timestamp"] + // Strings: + | &["upper"] + | &["lower"] + | &["replace"] + | &["btrim"] + | &["length"] + | &["concat_ws"] + | &["lpad"] + | &["rpad"] + | &["regexp_replace"] + | &["regexp_matches"] + | &["to_char"] + // Misc: + | &["any"] => match args { + FnArgs::Exprs { + distinct_flag, + exprs, + } => !distinct_flag && exprs.iter().all(is_safe_filter_expr), + _ => false, + }, + _ => false, + }, + &Expr::Not(ref inner) => is_safe_filter_expr(inner), + &Expr::Nullness { + is_null: _, + expr: ref inner, + } => is_safe_filter_expr(inner), + _ => false, + } +} diff --git a/phono-server/src/routes/relations_single/portal_handler.rs b/phono-server/src/routes/relations_single/portal_handler.rs index 0339f48..e2bb208 100644 --- a/phono-server/src/routes/relations_single/portal_handler.rs +++ b/phono-server/src/routes/relations_single/portal_handler.rs @@ -6,7 +6,6 @@ use axum::{ use phono_backends::{pg_acl::PgPrivilegeType, pg_attribute::PgAttribute}; use phono_models::{ accessors::{Accessor, Actor, portal::PortalAccessor}, - expression::PgExpressionAny, workspace::Workspace, }; use serde::{Deserialize, Serialize}; @@ -98,7 +97,7 @@ pub(super) async fn get( struct ResponseTemplate { columns: Vec, attr_names: Vec, - filter: Option, + filter: String, settings: Settings, subfilter_str: String, navbar: WorkspaceNav, @@ -107,7 +106,7 @@ pub(super) async fn get( ResponseTemplate { columns, attr_names, - filter: portal.table_filter.0, + filter: portal.filter, navbar: WorkspaceNav::builder() .navigator(navigator) .workspace(workspace) diff --git a/phono-server/src/routes/relations_single/set_filter_handler.rs b/phono-server/src/routes/relations_single/set_filter_handler.rs index de7eb64..1af8608 100644 --- a/phono-server/src/routes/relations_single/set_filter_handler.rs +++ b/phono-server/src/routes/relations_single/set_filter_handler.rs @@ -8,7 +8,6 @@ use axum::{ use axum_extra::extract::Form; use phono_models::{ accessors::{Accessor, Actor, portal::PortalAccessor}, - expression::PgExpressionAny, portal::Portal, }; use serde::Deserialize; @@ -32,11 +31,10 @@ pub(super) struct PathParams { #[derive(Debug, Deserialize)] pub(super) struct FormBody { - filter_expression: Option, + filter: String, } -/// HTTP POST handler for applying a [`PgExpressionAny`] filter to a portal's -/// table viewer. +/// HTTP POST handler for applying a filter to a portal's table viewer. /// /// This handler expects 3 path parameters with the structure described by /// [`PathParams`]. @@ -51,7 +49,7 @@ pub(super) async fn post( rel_oid, workspace_id, }): Path, - Form(form): Form, + Form(FormBody { filter }): Form, ) -> Result { // FIXME: csrf @@ -70,12 +68,9 @@ pub(super) async fn post( .fetch_one() .await?; - let filter: Option = - serde_json::from_str(&form.filter_expression.unwrap_or("null".to_owned()))?; - Portal::update() .id(portal.id) - .table_filter(filter) + .filter(filter) .build()? .execute(&mut app_db) .await?; diff --git a/phono-server/templates/relations_single/portal_table.html b/phono-server/templates/relations_single/portal_table.html index 74a0775..170cc6f 100644 --- a/phono-server/templates/relations_single/portal_table.html +++ b/phono-server/templates/relations_single/portal_table.html @@ -9,8 +9,7 @@ Portal Settings {% endblock %} diff --git a/static/expression-editor.css b/static/expression-editor.css deleted file mode 100644 index 8972599..0000000 --- a/static/expression-editor.css +++ /dev/null @@ -1,78 +0,0 @@ -.expression-editor__container { - background: #eee; - border-radius: var(--default-border-radius--rounded); - display: flex; -} - -.expression-editor__sidebar { - display: grid; - grid-template: - 'padding-top' 1fr - 'operator-selector' max-content - 'actions' minmax(max-content, 1fr); -} - -.expression-editor__main { - background: #fff; - border-radius: var(--default-border-radius--rounded); - border: solid 1px var(--default-border-color); - flex: 1; - padding: var(--default-padding); -} - -.expression-editor__action-button { - padding: var(--default-padding); - - svg path { - fill: currentColor; - } -} - -.expression-editor__params { - display: flex; - flex-direction: column; - gap: var(--default-padding); -} - -.expression-selector { - grid-area: operator-selector; -} - -.expression-selector__expression-button { - align-items: center; - display: flex; - justify-content: center; - height: 2.5rem; - padding: 0; - width: 2.5rem; - - svg path { - fill: currentColor; - } -} - -.expression-selector__popover:popover-open { - top: anchor(bottom); - margin-top: 0.25rem; - position: absolute; - display: flex; - flex-direction: column; - padding: 0; - background: #fff; -} - -.expression-selector__section { - align-items: center; - display: grid; - grid-template-columns: repeat(3, 1fr); - justify-content: center; - list-style-type: none; - margin: var(--default-padding); - padding: 0; -} - -.expression-selector__li { - align-items: center; - display: flex; - justify-content: center; -} diff --git a/static/main.css b/static/main.css index 4928b0c..947c971 100644 --- a/static/main.css +++ b/static/main.css @@ -1,8 +1,3 @@ -/* -@use 'forms'; -@use 'condition-editor'; -*/ - /* ======== Theming ======== */ :root { diff --git a/static/portal-table.css b/static/portal-table.css index b86db6a..cb1628c 100644 --- a/static/portal-table.css +++ b/static/portal-table.css @@ -1,5 +1,3 @@ -@import "./expression-editor.css"; - :root { --table-header-border-color: var(--default-border-color); --table-cell-border-color: oklch(from var(--default-border-color) calc(l * 1.15) c h); diff --git a/svelte/src/expression-editor.webc.svelte b/svelte/src/expression-editor.webc.svelte deleted file mode 100644 index 2a1ff78..0000000 --- a/svelte/src/expression-editor.webc.svelte +++ /dev/null @@ -1,91 +0,0 @@ - - - - -
-
- -
- {#if value !== undefined} -
-
- {#if value.t === "Comparison"} - {#if value.c.t === "Infix"} - - - {:else if value.c.t === "IsNull" || value.c.t === "IsNotNull"} - - {/if} - {:else if value.t === "Identifier"} - - {:else if value.t === "Literal"} - - {/if} -
-
- {/if} -
diff --git a/svelte/src/expression-selector.svelte b/svelte/src/expression-selector.svelte deleted file mode 100644 index 360041c..0000000 --- a/svelte/src/expression-selector.svelte +++ /dev/null @@ -1,185 +0,0 @@ - - - - -
- -
- {#each expressions as section} -
    - {#each section.expressions as expr} - {@const iconography = expression_icon(expr)} -
  • - -
    • - {/each} -
- {/each} -
-
diff --git a/svelte/src/expression.svelte.ts b/svelte/src/expression.svelte.ts deleted file mode 100644 index 89a14ef..0000000 --- a/svelte/src/expression.svelte.ts +++ /dev/null @@ -1,175 +0,0 @@ -import { z } from "zod"; - -import { datum_schema } from "./datum.svelte.ts"; - -export const all_expression_types = [ - "Comparison", - "Identifier", - "Literal", - "ToJson", -] as const; -// Type checking to ensure that all valid enum tags are included. -type Assert<_T extends true> = void; -type _ = Assert; - -export const expression_type_schema = z.enum(all_expression_types); - -export const all_infix_comparison_operators = [ - "Eq", - "Neq", - "Gt", - "Lt", - "And", - "Or", -] as const; - -const pg_comparison_operator_schema = z.enum(all_infix_comparison_operators); - -const pg_infix_expression_schema = z.object({ - operator: z.union([pg_comparison_operator_schema]), - get lhs() { - return pg_expression_any_schema.optional(); - }, - get rhs() { - return pg_expression_any_schema.optional(); - }, -}); - -const pg_comparison_expression_infix_schema = z.object({ - t: z.literal("Infix"), - c: pg_infix_expression_schema, -}); - -const pg_is_null_expression_schema = z.object({ - get lhs() { - return pg_expression_any_schema.optional(); - }, -}); - -const pg_comparison_expression_is_null_schema = z.object({ - t: z.literal("IsNull"), - c: pg_is_null_expression_schema, -}); - -const pg_is_not_null_expression_schema = z.object({ - get lhs() { - return pg_expression_any_schema.optional(); - }, -}); - -const pg_comparison_expression_is_not_null_schema = z.object({ - t: z.literal("IsNotNull"), - c: pg_is_not_null_expression_schema, -}); - -const pg_comparison_expression_schema = z.union([ - pg_comparison_expression_infix_schema, - pg_comparison_expression_is_null_schema, - pg_comparison_expression_is_not_null_schema, -]); - -const pg_expression_any_comparison_schema = z.object({ - t: z.literal("Comparison"), - c: pg_comparison_expression_schema, -}); - -const pg_identifier_expression_schema = z.object({ - parts_raw: z.array(z.string()), -}); - -const pg_expression_any_identifier_schema = z.object({ - t: z.literal("Identifier"), - c: pg_identifier_expression_schema, -}); - -const pg_expression_any_literal_schema = z.object({ - t: z.literal("Literal"), - c: datum_schema, -}); - -const pg_to_json_expression_schema = z.object({ - get entries() { - return z.array(z.tuple([z.string(), pg_expression_any_schema.optional()])); - }, -}); - -const pg_expression_any_to_json_expression_schema = z.object({ - t: z.literal("ToJson"), - c: pg_to_json_expression_schema, -}); - -export const pg_expression_any_schema = z.union([ - pg_expression_any_comparison_schema, - pg_expression_any_identifier_schema, - pg_expression_any_literal_schema, - pg_expression_any_to_json_expression_schema, -]); - -export type PgExpressionAny = z.infer; -export type PgExpressionType = z.infer; - -export function expression_human_name(expr_type: PgExpressionType): string { - if (expr_type === "Comparison") { - return "Condition"; - } - if (expr_type === "Identifier") { - return "Identifier"; - } - if (expr_type === "Literal") { - return "Literal"; - } - if (expr_type === "ToJson") { - return "JSON"; - } - // Type guard to check for exhaustive matching. - type _ = Assert; - throw new Error("this should be unreachable"); -} - -export function expression_icon(expr: PgExpressionAny): { - html: string; - label: string; -} { - if (expr.t === "Comparison") { - if (expr.c.t === "Infix") { - const op = expr.c.c.operator; - if (op === "And") { - return { html: "&&", label: "And" }; - } - if (op === "Eq") { - return { html: "=", label: "Is Equal To" }; - } - if (op === "Gt") { - return { html: ">", label: "Is Greater Than" }; - } - if (op === "Lt") { - return { html: "<", label: "Is Less Than" }; - } - if (op === "Or") { - return { html: "||", label: "Or" }; - } - if (op === "Neq") { - return { html: "\u2260", label: "Is Not Equal To" }; - } - // Type guard to check for exhaustive matching. - type _ = Assert; - throw new Error("this should be unreachable"); - } else if (expr.c.t === "IsNull") { - return { html: '', label: "Is Null" }; - } else if (expr.c.t === "IsNotNull") { - return { html: '', label: "Is Not Null" }; - } - // Type guard to check for exhaustive matching. - type _ = Assert; - throw new Error("this should be unreachable"); - } else if (expr.t === "Identifier") { - return { html: '', label: "Dynamic Value" }; - } else if (expr.t === "Literal") { - return { html: '', label: "Static Value" }; - } else if (expr.t === "ToJson") { - return { html: '', label: "JSON String" }; - } - // Type guard to check for exhaustive matching. - type _ = Assert; - throw new Error("this should be unreachable"); -} diff --git a/svelte/src/filter-menu.webc.svelte b/svelte/src/filter-menu.webc.svelte index e555aaa..2aafdad 100644 --- a/svelte/src/filter-menu.webc.svelte +++ b/svelte/src/filter-menu.webc.svelte @@ -1,51 +1,42 @@
Filter
- +
Filter expression (SQL)
+
-
diff --git a/svelte/src/table-viewer.webc/index.svelte b/svelte/src/table-viewer.webc/index.svelte index 077f525..fd40cc7 100644 --- a/svelte/src/table-viewer.webc/index.svelte +++ b/svelte/src/table-viewer.webc/index.svelte @@ -32,7 +32,7 @@ component. subfilter?: string; }; - let { columns = [], subfilter = "null" }: Props = $props(); + let { columns = [], subfilter = "" }: Props = $props(); type LazyData = { count: number; @@ -71,7 +71,7 @@ component. {columns} fields={lazy_data.fields} rows_main={lazy_data.rows} - subfilter_active={!!subfilter && subfilter !== "null"} + subfilter_active={!!subfilter} total_count={lazy_data.count} /> {/if}