From 69cfe15ffac74d07d999a0296366d30a75e4a207 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Wed, 7 Oct 2026 09:47:20 +0000 Subject: [PATCH 1/5] Represent internal query events without dummy row state --- src/webserver/database/execute_queries.rs | 475 ++++++++++++++++------ src/webserver/database/sql.rs | 7 +- 2 files changed, 363 insertions(+), 119 deletions(-) diff --git a/src/webserver/database/execute_queries.rs b/src/webserver/database/execute_queries.rs index 810cba2d0..636d2d8c8 100644 --- a/src/webserver/database/execute_queries.rs +++ b/src/webserver/database/execute_queries.rs @@ -10,10 +10,9 @@ use tracing::Instrument; use super::csv_import::run_csv_import; use super::error_highlighting::{display_stmt_db_error, display_stmt_error, is_positioned_error}; use super::sql::{ - DatabaseQuery, FileStatement, OutputColumn, Query, QueryBody, SourceSpan, SqlFile, - StaticSimpleSelect, + DatabaseQuery, FileStatement, Query, QueryBody, SourceSpan, SqlFile, StaticSimpleSelect, }; -use super::sqlpage_expr::{NoInputs, RowExpr, RowInputs}; +use super::sqlpage_expr::{NoInputs, RowInputs}; use crate::dynamic_component::parse_dynamic_rows; use crate::utils::add_value_to_map; use crate::webserver::ErrorWithStatus; @@ -35,10 +34,24 @@ use sqlx::value::ValueRef; pub type DbConn = Option>; -/// One database result together with private values reserved for computed -/// columns and therefore omitted from the user-visible row. -struct QueryResult { - item: DbItem, +/// Database events before computed columns and dynamic components are evaluated. +enum QueryEvent { + Row(QueryRow), + FinishedQuery, +} + +impl QueryEvent { + fn into_db_item(self) -> DbItem { + match self { + Self::Row(row) => DbItem::Row(row.value), + Self::FinishedQuery => DbItem::FinishedQuery, + } + } +} + +/// A decoded row together with private values reserved for computed columns. +struct QueryRow { + value: Value, inputs: RowInputs, output_column_count: usize, } @@ -224,32 +237,29 @@ pub fn stream_query_results_with_conn<'a>( let connection = take_connection(&request.app_state.db, db_connection, request).await?; let mut stream = connection.fetch_many(query); loop { - let start_next = std::time::Instant::now(); - let next_elem = stream.next().instrument(query_span.clone()).await; - query_metrics.add_duration(start_next.elapsed()); - let Some(elem) = next_elem else { break; }; - - let mut query_result = parse_single_sql_result(source_file, stmt, statement.source_span, elem); - if let DbItem::Error(e) = query_result.item { - error = Some(e); - break; - } - if matches!(query_result.item, DbItem::Row(_)) { - returned_rows += 1; - } - apply_json_columns(&mut query_result.item, &stmt.json_columns); + let Some(result) = next_query_event( + &mut stream, &mut query_metrics, &mut returned_rows, + source_file, stmt, statement.source_span, + ).await else { break; }; + let mut query_result = match result { + Ok(event) => event, + Err(e) => { + error = Some(e); + break; + } + }; if buffer_rows { deferred_query_results.push(query_result); } else { let mut computed_connection = None; - if let Err(err) = evaluate_computed_columns(request, &stmt.computed_columns, &mut query_result, &mut computed_connection) + if let Err(err) = prepare_query_event(request, stmt, &mut query_result, &mut computed_connection) .instrument(query_span.clone()) .await { error = Some(err); break; } - for db_item in parse_dynamic_rows(query_result.item) { + for db_item in parse_dynamic_rows(query_result.into_db_item()) { yield db_item; } } @@ -258,9 +268,9 @@ pub fn stream_query_results_with_conn<'a>( } if error.is_none() && buffer_rows { for mut query_result in deferred_query_results { - if let Err(err) = evaluate_computed_columns( + if let Err(err) = prepare_query_event( request, - &stmt.computed_columns, + stmt, &mut query_result, db_connection, ) @@ -270,7 +280,7 @@ pub fn stream_query_results_with_conn<'a>( error = Some(err); break; } - for db_item in parse_dynamic_rows(query_result.item) { + for db_item in parse_dynamic_rows(query_result.into_db_item()) { yield db_item; } } @@ -402,7 +412,7 @@ async fn execute_scalar_query<'a>( }; ensure_scalar_column_count(static_select.columns.len())?; let row = execute_static_simple_select(static_select, request, db_connection).await?; - return scalar_value_from_row(DbItem::Row(row)); + return scalar_value_from_row(row); }; let query = bind_query(database_query, request, db_connection).await?; log::debug!("Executing scalar query: {:?}", query.sql); @@ -422,17 +432,20 @@ async fn execute_scalar_query<'a>( let connection = take_connection(&request.app_state.db, db_connection, request).await?; let mut stream = connection.fetch_many(query); loop { - let start_next = std::time::Instant::now(); - let next_elem = stream.next().instrument(query_span.clone()).await; - query_metrics.add_duration(start_next.elapsed()); - let Some(elem) = next_elem else { break }; - - let result = - parse_single_sql_result(source_file, database_query, statement.source_span, elem); - let output_column_count = result.output_column_count; - match result.item { - row @ DbItem::Row(_) => { - returned_rows += 1; + let Some(result) = next_query_event( + &mut stream, + &mut query_metrics, + &mut returned_rows, + source_file, + database_query, + statement.source_span, + ) + .await + else { + break; + }; + match result { + Ok(QueryEvent::Row(row)) => { if scalar_row.is_some() { debug_assert_eq!( scalar_subquery_behavior, @@ -443,17 +456,13 @@ async fn execute_scalar_query<'a>( )); break; } - scalar_row = Some(QueryResult { - item: row, - inputs: result.inputs, - output_column_count, - }); + scalar_row = Some(row); if scalar_subquery_behavior == ScalarSubqueryBehavior::FirstRow { break; } } - DbItem::FinishedQuery => {} - DbItem::Error(err) => { + Ok(QueryEvent::FinishedQuery) => {} + Err(err) => { error = Some(err); break; } @@ -469,15 +478,9 @@ async fn execute_scalar_query<'a>( ); } else if let Some(mut row) = scalar_row { ensure_scalar_column_count(row.output_column_count)?; - apply_json_columns(&mut row.item, &database_query.json_columns); - if let Err(error) = evaluate_computed_columns( - request, - &database_query.computed_columns, - &mut row, - db_connection, - ) - .instrument(query_span.clone()) - .await + if let Err(error) = prepare_query_row(request, database_query, &mut row, db_connection) + .instrument(query_span.clone()) + .await { let connection = take_connection(&request.app_state.db, db_connection, request).await?; return Err(record_error_and_rollback( @@ -488,7 +491,7 @@ async fn execute_scalar_query<'a>( ) .await); } - scalar_value_from_row(row.item)? + scalar_value_from_row(row.value)? } else { None }; @@ -508,8 +511,8 @@ async fn record_error_and_rollback( error } -fn scalar_value_from_row(item: DbItem) -> anyhow::Result> { - let DbItem::Row(Value::Object(row)) = item else { +fn scalar_value_from_row(value: Value) -> anyhow::Result> { + let Value::Object(row) = value else { anyhow::bail!("SET scalar query did not return a row object"); }; ensure_scalar_column_count(row.len())?; @@ -609,51 +612,57 @@ async fn set_trace_context(connection: &mut AnyConnection, db: &Database) { } } +/// Poll and decode one physical result while measuring only database fetch time. +async fn next_query_event( + stream: &mut (impl Stream>> + Unpin), + metrics: &mut DbQueryMetricsContext<'_>, + returned_rows: &mut i64, + source_file: &Path, + query: &DatabaseQuery, + source_span: SourceSpan, +) -> Option> { + let start_next = std::time::Instant::now(); + let next = stream.next().instrument(metrics.span.clone()).await; + metrics.add_duration(start_next.elapsed()); + let result = parse_single_sql_result(source_file, query, source_span, next?); + if matches!(result, Ok(QueryEvent::Row(_))) { + *returned_rows += 1; + } + Some(result) +} + #[inline] fn parse_single_sql_result( source_file: &Path, query: &DatabaseQuery, source_span: SourceSpan, res: sqlx::error::Result>, -) -> QueryResult { +) -> anyhow::Result { match res { Ok(Either::Right(r)) => { if log::log_enabled!(log::Level::Trace) { debug_row(&r); } - match super::sql_to_json::row_to_json_with_inputs(&r, query.row_input_json.len()) { - Ok((row, mut inputs)) => { - decode_json_values(&mut inputs, &query.row_input_json); - QueryResult { - item: DbItem::Row(row), - inputs: RowInputs::new(inputs), - output_column_count: r.columns().len() - query.row_input_json.len() - + query.computed_columns.len(), - } - } - Err(error) => QueryResult { - item: DbItem::Error(error), - inputs: RowInputs::new(Vec::new()), - output_column_count: 0, - }, - } + let (row, mut inputs) = + super::sql_to_json::row_to_json_with_inputs(&r, query.row_input_json.len())?; + decode_json_values(&mut inputs, &query.row_input_json); + Ok(QueryEvent::Row(QueryRow { + value: row, + inputs: RowInputs::new(inputs), + output_column_count: r.columns().len() - query.row_input_json.len() + + query.computed_columns.len(), + })) } Ok(Either::Left(res)) => { log::debug!("Finished query with result: {res:?}"); - QueryResult { - item: DbItem::FinishedQuery, - inputs: RowInputs::new(Vec::new()), - output_column_count: 0, - } - } - Err(err) => { - let nice_err = display_stmt_db_error(source_file, &query.sql, source_span, err); - QueryResult { - item: DbItem::Error(nice_err), - inputs: RowInputs::new(Vec::new()), - output_column_count: 0, - } + Ok(QueryEvent::FinishedQuery) } + Err(err) => Err(display_stmt_db_error( + source_file, + &query.sql, + source_span, + err, + )), } } @@ -725,17 +734,30 @@ async fn bind_query<'a>( }) } -async fn evaluate_computed_columns( +async fn prepare_query_event( request: &ExecutionContext, - columns: &[OutputColumn], - result: &mut QueryResult, + query: &DatabaseQuery, + event: &mut QueryEvent, db_connection: &mut DbConn, ) -> anyhow::Result<()> { - if let DbItem::Row(Value::Object(results)) = &mut result.item { - for column in columns { + match event { + QueryEvent::Row(row) => prepare_query_row(request, query, row, db_connection).await, + QueryEvent::FinishedQuery => Ok(()), + } +} + +async fn prepare_query_row( + request: &ExecutionContext, + query: &DatabaseQuery, + row: &mut QueryRow, + db_connection: &mut DbConn, +) -> anyhow::Result<()> { + apply_json_columns(&mut row.value, &query.json_columns); + if let Value::Object(results) = &mut row.value { + for column in &query.computed_columns { let value = column .value - .evaluate(request, db_connection, &mut result.inputs) + .evaluate(request, db_connection, &mut row.inputs) .await? .into_json(); let old_results = std::mem::take(results); @@ -745,8 +767,8 @@ async fn evaluate_computed_columns( Ok(()) } -fn apply_json_columns(item: &mut DbItem, json_columns: &[String]) { - if let DbItem::Row(Value::Object(row)) = item { +fn apply_json_columns(value: &mut Value, json_columns: &[String]) { + if let Value::Object(row) = value { for column in json_columns { if let Some(value) = row.get_mut(column) { if let Value::String(json_str) = value { @@ -831,13 +853,9 @@ mod tests { use tracing_subscriber::prelude::*; use tracing_subscriber::registry::LookupSpan; - fn create_row_item(value: Value) -> DbItem { - DbItem::Row(value) - } - - fn assert_json_value(item: &DbItem, key: &str, expected: Value) { - let DbItem::Row(Value::Object(row)) = item else { - panic!("Expected DbItem::Row"); + fn assert_json_value(item: &Value, key: &str, expected: Value) { + let Value::Object(row) = item else { + panic!("Expected a row object"); }; assert_eq!(row[key], expected); drop(expected); @@ -845,10 +863,10 @@ mod tests { #[test] fn test_basic_json_string_conversion() { - let mut item = create_row_item(json!({ + let mut item = json!({ "json_col": "{\"key\": \"value\"}", "normal_col": "text" - })); + }); apply_json_columns(&mut item, &["json_col".to_string()]); assert_json_value(&item, "json_col", json!({"key": "value"})); assert_json_value(&item, "normal_col", json!("text")); @@ -856,10 +874,10 @@ mod tests { #[test] fn test_json_array_conversion() { - let mut item = create_row_item(json!({ + let mut item = json!({ "array_col": ["{\"a\": 1}", "{\"b\": 2}"], "normal_array": ["text"] - })); + }); apply_json_columns(&mut item, &["array_col".to_string()]); assert_json_value(&item, "array_col", json!([{"a": 1}, {"b": 2}])); assert_json_value(&item, "normal_array", json!(["text"])); @@ -867,10 +885,10 @@ mod tests { #[test] fn test_invalid_json_handling() { - let mut item = create_row_item(json!({ + let mut item = json!({ "invalid_json": "{not valid json}", "normal_col": "text" - })); + }); apply_json_columns(&mut item, &["invalid_json".to_string()]); assert_json_value(&item, "invalid_json", json!("{not valid json}")); assert_json_value(&item, "normal_col", json!("text")); @@ -878,31 +896,258 @@ mod tests { #[test] fn test_missing_column_handling() { - let mut item = create_row_item(json!({ + let mut item = json!({ "existing_col": "text" - })); + }); apply_json_columns(&mut item, &["missing_col".to_string()]); assert_json_value(&item, "existing_col", json!("text")); } #[test] - fn test_non_row_dbitem_handling() { - let mut item = DbItem::FinishedQuery; + fn test_non_object_json_handling() { + let mut item = Value::Null; apply_json_columns(&mut item, &["json_col".to_string()]); - assert!(matches!(item, DbItem::FinishedQuery)); + assert!(item.is_null()); } #[test] fn test_duplicate_json_column_names() { - let mut item = create_row_item(json!({ + let mut item = json!({ "json_col": "{\"key\": \"value\"}", "normal_col": "text" - })); + }); apply_json_columns(&mut item, &["json_col".to_string(), "json_col".to_string()]); assert_json_value(&item, "json_col", json!({"key": "value"})); assert_json_value(&item, "normal_col", json!("text")); } + async fn execution_context(web_root: &Path) -> ExecutionContext { + let mut config: crate::app_config::AppConfig = serde_json::from_str("{}").unwrap(); + config.database_url = crate::app_config::test_database_url(); + config.max_database_pool_connections = Some(1); + config.database_connection_acquire_timeout_seconds = 1.0; + config.web_root = web_root.to_owned(); + let state = Arc::new(crate::AppState::init(&config).await.unwrap()); + let mut request = actix_web::test::TestRequest::get().to_srv_request(); + crate::webserver::http_request_info::extract_request_info( + &mut request, + state, + crate::webserver::server_timing::ServerTiming::default(), + ) + .await + .unwrap() + } + + async fn collect_rows( + request: &ExecutionContext, + connection: &mut DbConn, + sql: &str, + ) -> anyhow::Result> { + let file = SqlFile::new(&request.app_state.db, sql, Path::new("execution-test.sql")); + let stream = stream_query_results_with_conn(&file, request, connection); + futures_util::pin_mut!(stream); + let mut rows = Vec::new(); + while let Some(item) = stream.next().await { + match item { + DbItem::Row(row) => rows.push(row), + DbItem::FinishedQuery => {} + DbItem::Error(error) => return Err(error), + } + } + Ok(rows) + } + + #[actix_web::test] + async fn streaming_rows_keep_private_inputs_out_of_duplicate_outputs() { + let request = execution_context(Path::new(".")).await; + let rows = collect_rows( + &request, + &mut None, + "SELECT '[1]' AS values_json, '[2]' AS values_json, + sqlpage.url_encode(name) AS encoded, name AS encoded + FROM (SELECT 'a b' AS name) source_rows", + ) + .await + .unwrap(); + assert_eq!( + rows, + [json!({ + "values_json": ["[1]", "[2]"], + "encoded": ["a b", "a%20b"] + })] + ); + if request.app_state.db.info.kind == sqlx::any::AnyKind::Sqlite { + let rows = collect_rows( + &request, + &mut None, + "SELECT json('[1]') AS values_json, json('[2]') AS values_json, + sqlpage.url_encode(name) AS encoded + FROM (SELECT 'a b' AS name) source_rows", + ) + .await + .unwrap(); + assert_eq!( + rows, + [json!({"values_json": [[1], [2]], "encoded": "a%20b"})] + ); + } + } + + #[actix_web::test] + async fn scalar_rows_check_physical_column_count_and_database_row_policy() { + let request = execution_context(Path::new(".")).await; + let mut connection = None; + let rows = collect_rows( + &request, + &mut connection, + "SET empty_value = (SELECT n FROM (SELECT 1 AS n) source_rows WHERE 1 = 0); + SET encoded_value = (SELECT sqlpage.url_encode(name) FROM (SELECT 'a b' AS name) source_rows); + SELECT $empty_value AS empty_value, $encoded_value AS encoded_value", + ) + .await + .unwrap(); + assert_eq!( + rows, + [json!({"empty_value": null, "encoded_value": "a%20b"})] + ); + + let error = collect_rows( + &request, + &mut connection, + "SET invalid_value = (SELECT n AS duplicate, n AS duplicate FROM (SELECT 1 AS n) source_rows)", + ) + .await + .unwrap_err(); + assert!(format!("{error:#}").contains("more than one column")); + + let result = collect_rows( + &request, + &mut connection, + "SET multiple_value = (SELECT n FROM (SELECT 1 AS n UNION ALL SELECT 2 AS n) source_rows); + SELECT $multiple_value AS value", + ) + .await; + match request + .app_state + .db + .info + .database_type + .scalar_subquery_behavior() + { + ScalarSubqueryBehavior::FirstRow => { + assert_eq!(result.unwrap(), [json!({"value": "1"})]); + } + ScalarSubqueryBehavior::ErrorOnMultipleRows => { + assert!(format!("{:#}", result.unwrap_err()).contains("more than one row")); + } + } + } + + #[actix_web::test] + async fn buffered_and_scalar_run_sql_reuse_the_only_connection() { + let root = tempfile::tempdir().unwrap(); + std::fs::write( + root.path().join("child.sql"), + "SELECT sqlpage.run_sql('grandchild.sql') AS nested FROM (SELECT 1 AS n) source_rows", + ) + .unwrap(); + std::fs::write( + root.path().join("grandchild.sql"), + "SELECT 'nested' AS value FROM (SELECT 1 AS n) source_rows", + ) + .unwrap(); + let request = execution_context(root.path()).await; + let mut connection = None; + let rows = tokio::time::timeout( + std::time::Duration::from_secs(5), + collect_rows(&request, &mut connection, + "SELECT n, sqlpage.run_sql(path) AS included + FROM (SELECT 1 AS n, 'child.sql' AS path UNION ALL SELECT 2 AS n, 'child.sql' AS path) source_rows + ORDER BY n; + SET scalar_included = (SELECT sqlpage.run_sql(path) FROM (SELECT 'child.sql' AS path) source_rows); + SELECT $scalar_included AS scalar_included" + ), + ).await.expect("nested execution should reuse the existing connection").unwrap(); + let included = r#"[{"nested":"[{\"value\":\"nested\"}]"}]"#; + assert_eq!( + rows, + [ + json!({"n": 1, "included": included}), + json!({"n": 2, "included": included}), + json!({"scalar_included": included}), + ] + ); + } + + #[actix_web::test] + async fn dropping_stream_stops_later_rows_and_releases_connection_borrow() { + let request = execution_context(Path::new(".")).await; + let file = SqlFile::new( + &request.app_state.db, + "SELECT n, sqlpage.fetch(url) AS value + FROM (SELECT 1 AS n, NULL AS url UNION ALL SELECT 2 AS n, 'invalid URL' AS url) source_rows + ORDER BY n; + SET after_query = 'unexpected'", + Path::new("execution-test.sql"), + ); + let mut connection = None; + { + let stream = stream_query_results_with_conn(&file, &request, &mut connection); + futures_util::pin_mut!(stream); + let Some(DbItem::Row(row)) = stream.next().await else { + panic!("Expected the first row before the later function error"); + }; + assert_eq!(row, json!({"n": 1, "value": null})); + } + assert!(!request.set_variables.borrow().contains_key("after_query")); + assert_eq!( + collect_rows( + &request, + &mut connection, + "SELECT n FROM (SELECT 7 AS n) source_rows" + ) + .await + .unwrap(), + [json!({"n": 7})] + ); + } + + #[actix_web::test] + async fn computed_column_failure_rolls_back_and_leaves_connection_usable() { + let request = execution_context(Path::new(".")).await; + if request.app_state.db.info.kind != sqlx::any::AnyKind::Sqlite { + return; + } + let mut connection = None; + collect_rows( + &request, + &mut connection, + "CREATE TEMPORARY TABLE query_event_rollback(value INTEGER)", + ) + .await + .unwrap(); + let error = collect_rows( + &request, + &mut connection, + "BEGIN; + INSERT INTO query_event_rollback VALUES (1); + SELECT sqlpage.fetch(url) AS value FROM (SELECT 'invalid URL' AS url) source_rows", + ) + .await + .unwrap_err(); + assert!(format!("{error:#}").contains("invalid URL")); + assert_eq!( + collect_rows( + &request, + &mut connection, + "SELECT COUNT(*) AS count FROM query_event_rollback" + ) + .await + .unwrap(), + [json!({"count": 0})] + ); + } + #[derive(Default)] struct RecordedFields(HashMap<&'static str, String>); diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 9cefc8787..02f4c28d0 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -41,13 +41,12 @@ mod dialect; mod rewrite; mod statement; -#[cfg(test)] -pub(super) use statement::SourceLocation; pub use statement::SqlFile; pub(super) use statement::{ - DatabaseQuery, FileStatement, OutputColumn, Query, QueryBody, SourceSpan, StaticSimpleSelect, - VariableName, + DatabaseQuery, FileStatement, Query, QueryBody, SourceSpan, StaticSimpleSelect, VariableName, }; +#[cfg(test)] +pub(super) use statement::{OutputColumn, SourceLocation}; impl SqlFile { #[must_use] From e61c67e4a55c91e6fb3531a59a4d43d2ebd64fb6 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Wed, 7 Oct 2026 10:08:12 +0000 Subject: [PATCH 2/5] Reuse SQL fixtures and transaction coverage for query cleanup --- src/webserver/database/execute_queries.rs | 208 ++---------------- ...ar_query_returned_more_than_one_column.sql | 3 + .../sqlite/set_multiple_rows.sql | 3 + .../sql_test_files/data/query_event_rows.sql | 9 + .../data/set_variable_to_sqlpage_function.sql | 5 +- tests/sql_test_files/mod.rs | 4 + tests/sql_test_files/query_events/child.sql | 1 + .../query_events/grandchild.sql | 1 + tests/sql_test_files/query_events/rows.sql | 9 + .../query_events/sqlite_json.sql | 3 + tests/transactions/failed_computed_column.sql | 4 + tests/transactions/mod.rs | 16 ++ 12 files changed, 72 insertions(+), 194 deletions(-) create mode 100644 tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql create mode 100644 tests/sql_test_files/data/query_event_rows.sql create mode 100644 tests/sql_test_files/query_events/child.sql create mode 100644 tests/sql_test_files/query_events/grandchild.sql create mode 100644 tests/sql_test_files/query_events/rows.sql create mode 100644 tests/sql_test_files/query_events/sqlite_json.sql create mode 100644 tests/transactions/failed_computed_column.sql diff --git a/src/webserver/database/execute_queries.rs b/src/webserver/database/execute_queries.rs index 636d2d8c8..500da529a 100644 --- a/src/webserver/database/execute_queries.rs +++ b/src/webserver/database/execute_queries.rs @@ -921,167 +921,21 @@ mod tests { assert_json_value(&item, "normal_col", json!("text")); } - async fn execution_context(web_root: &Path) -> ExecutionContext { + #[actix_web::test] + async fn dropping_stream_stops_later_rows_and_releases_connection_borrow() { let mut config: crate::app_config::AppConfig = serde_json::from_str("{}").unwrap(); config.database_url = crate::app_config::test_database_url(); config.max_database_pool_connections = Some(1); config.database_connection_acquire_timeout_seconds = 1.0; - config.web_root = web_root.to_owned(); let state = Arc::new(crate::AppState::init(&config).await.unwrap()); let mut request = actix_web::test::TestRequest::get().to_srv_request(); - crate::webserver::http_request_info::extract_request_info( + let request = crate::webserver::http_request_info::extract_request_info( &mut request, state, crate::webserver::server_timing::ServerTiming::default(), ) .await - .unwrap() - } - - async fn collect_rows( - request: &ExecutionContext, - connection: &mut DbConn, - sql: &str, - ) -> anyhow::Result> { - let file = SqlFile::new(&request.app_state.db, sql, Path::new("execution-test.sql")); - let stream = stream_query_results_with_conn(&file, request, connection); - futures_util::pin_mut!(stream); - let mut rows = Vec::new(); - while let Some(item) = stream.next().await { - match item { - DbItem::Row(row) => rows.push(row), - DbItem::FinishedQuery => {} - DbItem::Error(error) => return Err(error), - } - } - Ok(rows) - } - - #[actix_web::test] - async fn streaming_rows_keep_private_inputs_out_of_duplicate_outputs() { - let request = execution_context(Path::new(".")).await; - let rows = collect_rows( - &request, - &mut None, - "SELECT '[1]' AS values_json, '[2]' AS values_json, - sqlpage.url_encode(name) AS encoded, name AS encoded - FROM (SELECT 'a b' AS name) source_rows", - ) - .await - .unwrap(); - assert_eq!( - rows, - [json!({ - "values_json": ["[1]", "[2]"], - "encoded": ["a b", "a%20b"] - })] - ); - if request.app_state.db.info.kind == sqlx::any::AnyKind::Sqlite { - let rows = collect_rows( - &request, - &mut None, - "SELECT json('[1]') AS values_json, json('[2]') AS values_json, - sqlpage.url_encode(name) AS encoded - FROM (SELECT 'a b' AS name) source_rows", - ) - .await - .unwrap(); - assert_eq!( - rows, - [json!({"values_json": [[1], [2]], "encoded": "a%20b"})] - ); - } - } - - #[actix_web::test] - async fn scalar_rows_check_physical_column_count_and_database_row_policy() { - let request = execution_context(Path::new(".")).await; - let mut connection = None; - let rows = collect_rows( - &request, - &mut connection, - "SET empty_value = (SELECT n FROM (SELECT 1 AS n) source_rows WHERE 1 = 0); - SET encoded_value = (SELECT sqlpage.url_encode(name) FROM (SELECT 'a b' AS name) source_rows); - SELECT $empty_value AS empty_value, $encoded_value AS encoded_value", - ) - .await - .unwrap(); - assert_eq!( - rows, - [json!({"empty_value": null, "encoded_value": "a%20b"})] - ); - - let error = collect_rows( - &request, - &mut connection, - "SET invalid_value = (SELECT n AS duplicate, n AS duplicate FROM (SELECT 1 AS n) source_rows)", - ) - .await - .unwrap_err(); - assert!(format!("{error:#}").contains("more than one column")); - - let result = collect_rows( - &request, - &mut connection, - "SET multiple_value = (SELECT n FROM (SELECT 1 AS n UNION ALL SELECT 2 AS n) source_rows); - SELECT $multiple_value AS value", - ) - .await; - match request - .app_state - .db - .info - .database_type - .scalar_subquery_behavior() - { - ScalarSubqueryBehavior::FirstRow => { - assert_eq!(result.unwrap(), [json!({"value": "1"})]); - } - ScalarSubqueryBehavior::ErrorOnMultipleRows => { - assert!(format!("{:#}", result.unwrap_err()).contains("more than one row")); - } - } - } - - #[actix_web::test] - async fn buffered_and_scalar_run_sql_reuse_the_only_connection() { - let root = tempfile::tempdir().unwrap(); - std::fs::write( - root.path().join("child.sql"), - "SELECT sqlpage.run_sql('grandchild.sql') AS nested FROM (SELECT 1 AS n) source_rows", - ) - .unwrap(); - std::fs::write( - root.path().join("grandchild.sql"), - "SELECT 'nested' AS value FROM (SELECT 1 AS n) source_rows", - ) .unwrap(); - let request = execution_context(root.path()).await; - let mut connection = None; - let rows = tokio::time::timeout( - std::time::Duration::from_secs(5), - collect_rows(&request, &mut connection, - "SELECT n, sqlpage.run_sql(path) AS included - FROM (SELECT 1 AS n, 'child.sql' AS path UNION ALL SELECT 2 AS n, 'child.sql' AS path) source_rows - ORDER BY n; - SET scalar_included = (SELECT sqlpage.run_sql(path) FROM (SELECT 'child.sql' AS path) source_rows); - SELECT $scalar_included AS scalar_included" - ), - ).await.expect("nested execution should reuse the existing connection").unwrap(); - let included = r#"[{"nested":"[{\"value\":\"nested\"}]"}]"#; - assert_eq!( - rows, - [ - json!({"n": 1, "included": included}), - json!({"n": 2, "included": included}), - json!({"scalar_included": included}), - ] - ); - } - - #[actix_web::test] - async fn dropping_stream_stops_later_rows_and_releases_connection_borrow() { - let request = execution_context(Path::new(".")).await; let file = SqlFile::new( &request.app_state.db, "SELECT n, sqlpage.fetch(url) AS value @@ -1100,52 +954,20 @@ mod tests { assert_eq!(row, json!({"n": 1, "value": null})); } assert!(!request.set_variables.borrow().contains_key("after_query")); - assert_eq!( - collect_rows( - &request, - &mut connection, - "SELECT n FROM (SELECT 7 AS n) source_rows" - ) - .await - .unwrap(), - [json!({"n": 7})] + let file = SqlFile::new( + &request.app_state.db, + "SELECT n FROM (SELECT 7 AS n) source_rows", + Path::new("execution-test.sql"), ); - } - - #[actix_web::test] - async fn computed_column_failure_rolls_back_and_leaves_connection_usable() { - let request = execution_context(Path::new(".")).await; - if request.app_state.db.info.kind != sqlx::any::AnyKind::Sqlite { - return; + let stream = stream_query_results_with_conn(&file, &request, &mut connection); + futures_util::pin_mut!(stream); + let Some(DbItem::Row(row)) = stream.next().await else { + panic!("Expected a row from the reused connection"); + }; + assert_eq!(row, json!({"n": 7})); + while let Some(item) = stream.next().await { + assert!(matches!(item, DbItem::FinishedQuery)); } - let mut connection = None; - collect_rows( - &request, - &mut connection, - "CREATE TEMPORARY TABLE query_event_rollback(value INTEGER)", - ) - .await - .unwrap(); - let error = collect_rows( - &request, - &mut connection, - "BEGIN; - INSERT INTO query_event_rollback VALUES (1); - SELECT sqlpage.fetch(url) AS value FROM (SELECT 'invalid URL' AS url) source_rows", - ) - .await - .unwrap_err(); - assert!(format!("{error:#}").contains("invalid URL")); - assert_eq!( - collect_rows( - &request, - &mut connection, - "SELECT COUNT(*) AS count FROM query_event_rollback" - ) - .await - .unwrap(), - [json!({"count": 0})] - ); } #[derive(Default)] diff --git a/tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql b/tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql new file mode 100644 index 000000000..e3086c7ff --- /dev/null +++ b/tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql @@ -0,0 +1,3 @@ +set invalid_value = ( + select n as duplicate, n as duplicate from (select 1 as n) source_rows +); diff --git a/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql b/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql index 61a8c3b17..f9a774a3d 100644 --- a/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql +++ b/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql @@ -5,3 +5,6 @@ set actual = ( ); select 'first' as expected, $actual as actual; + +select '[{"values_json":[[1],[2]],"encoded":"a%20b"}]' as expected, + sqlpage.run_sql('tests/sql_test_files/query_events/sqlite_json.sql') as actual; diff --git a/tests/sql_test_files/data/query_event_rows.sql b/tests/sql_test_files/data/query_event_rows.sql new file mode 100644 index 000000000..e41826109 --- /dev/null +++ b/tests/sql_test_files/data/query_event_rows.sql @@ -0,0 +1,9 @@ +-- Compare the complete rows, including order and absence of private input columns. +set included = json_array(json_object('nested', '[{"value":"nested"}]')); +set expected = json_array( + json_object('n', 1, 'included', $included), + json_object('n', 2, 'included', $included), + json_object('scalar_included', $included), + json_object('values_json', json_array('[1]', '[2]'), 'encoded', json_array('a b', 'a%20b')) +); +select $expected as expected, sqlpage.run_sql('tests/sql_test_files/query_events/rows.sql') as actual; diff --git a/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql b/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql index 7fc0c5157..869236c8a 100644 --- a/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql +++ b/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql @@ -1,3 +1,6 @@ set my_var = sqlpage.url_encode(' '); select '%20' as expected, - $my_var as actual; \ No newline at end of file + $my_var as actual; + +set encoded = (select sqlpage.url_encode(name) from (select 'a b' as name) source_rows); +select 'a%20b' as expected, $encoded as actual; diff --git a/tests/sql_test_files/mod.rs b/tests/sql_test_files/mod.rs index c8cf53f32..f8a937390 100644 --- a/tests/sql_test_files/mod.rs +++ b/tests/sql_test_files/mod.rs @@ -78,6 +78,10 @@ fn get_sql_test_cases() -> Vec { "tests/sql_test_files/data", SqlTestFormat::Json, )); + tests.extend(read_sql_tests_in_dir( + "tests/sql_test_files/column_validation", + SqlTestFormat::Html, + )); tests } diff --git a/tests/sql_test_files/query_events/child.sql b/tests/sql_test_files/query_events/child.sql new file mode 100644 index 000000000..ce01ab0d8 --- /dev/null +++ b/tests/sql_test_files/query_events/child.sql @@ -0,0 +1 @@ +select sqlpage.run_sql('tests/sql_test_files/query_events/grandchild.sql') as nested from (select 1 as n) source_rows; diff --git a/tests/sql_test_files/query_events/grandchild.sql b/tests/sql_test_files/query_events/grandchild.sql new file mode 100644 index 000000000..4fd4f80d8 --- /dev/null +++ b/tests/sql_test_files/query_events/grandchild.sql @@ -0,0 +1 @@ +select 'nested' as value from (select 1 as n) source_rows; diff --git a/tests/sql_test_files/query_events/rows.sql b/tests/sql_test_files/query_events/rows.sql new file mode 100644 index 000000000..f18110cb1 --- /dev/null +++ b/tests/sql_test_files/query_events/rows.sql @@ -0,0 +1,9 @@ +select n, sqlpage.run_sql(path) as included +from (select 1 as n, 'tests/sql_test_files/query_events/child.sql' as path + union all select 2 as n, 'tests/sql_test_files/query_events/child.sql' as path) source_rows +order by n; +set scalar_included = (select sqlpage.run_sql(path) from (select 'tests/sql_test_files/query_events/child.sql' as path) source_rows); +select $scalar_included as scalar_included; +select '[1]' as values_json, '[2]' as values_json, + sqlpage.url_encode(name) as encoded, name as encoded +from (select 'a b' as name) source_rows; diff --git a/tests/sql_test_files/query_events/sqlite_json.sql b/tests/sql_test_files/query_events/sqlite_json.sql new file mode 100644 index 000000000..6d6cfa014 --- /dev/null +++ b/tests/sql_test_files/query_events/sqlite_json.sql @@ -0,0 +1,3 @@ +select json('[1]') as values_json, json('[2]') as values_json, + sqlpage.url_encode(name) as encoded +from (select 'a b' as name) source_rows; diff --git a/tests/transactions/failed_computed_column.sql b/tests/transactions/failed_computed_column.sql new file mode 100644 index 000000000..09593a61e --- /dev/null +++ b/tests/transactions/failed_computed_column.sql @@ -0,0 +1,4 @@ +create temporary table query_event_rollback(value integer); +begin; +insert into query_event_rollback values (1); +select sqlpage.fetch(url) as value from (select 'invalid URL' as url) source_rows; diff --git a/tests/transactions/mod.rs b/tests/transactions/mod.rs index 3718f09bf..18a1d8a9f 100644 --- a/tests/transactions/mod.rs +++ b/tests/transactions/mod.rs @@ -1,5 +1,6 @@ use actix_web::{http::StatusCode, test}; use sqlpage::webserver::{database::SupportedDatabase, http::main_handler}; +use sqlx::row::Row as _; use crate::common::{get_request_to_with_data, make_app_data}; @@ -26,6 +27,21 @@ async fn test_transaction_error() -> actix_web::Result<()> { body_str.contains("error") && body_str.contains("null"), "{body_str}\nexpected to contain: constraint failed" ); + if data.db.info.database_type == SupportedDatabase::Sqlite { + let req = get_request_to_with_data( + "/tests/transactions/failed_computed_column.sql", + data.clone(), + ) + .await? + .to_srv_request(); + let body = test::read_body(main_handler(req).await?).await; + assert!(String::from_utf8_lossy(&body).contains("invalid URL")); + let row = sqlx::query::query("SELECT COUNT(*) AS count FROM query_event_rollback") + .fetch_one(&data.db.connection) + .await + .unwrap(); + assert_eq!(row.try_get::("count").unwrap(), 0); + } // Now query again, with ?x=1447 let path_with_param = path.to_string() + "?x=1447"; let req = get_request_to_with_data(&path_with_param, data.clone()) From d99153f742f34f397a383716f8282b2e600311f4 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Wed, 7 Oct 2026 11:09:32 +0000 Subject: [PATCH 3/5] Use portable text markers in query event row assertions --- tests/sql_test_files/data/query_event_rows.sql | 4 ++-- tests/sql_test_files/query_events/rows.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/sql_test_files/data/query_event_rows.sql b/tests/sql_test_files/data/query_event_rows.sql index e41826109..2815ecdda 100644 --- a/tests/sql_test_files/data/query_event_rows.sql +++ b/tests/sql_test_files/data/query_event_rows.sql @@ -1,8 +1,8 @@ -- Compare the complete rows, including order and absence of private input columns. set included = json_array(json_object('nested', '[{"value":"nested"}]')); set expected = json_array( - json_object('n', 1, 'included', $included), - json_object('n', 2, 'included', $included), + json_object('n', 'row1', 'included', $included), + json_object('n', 'row2', 'included', $included), json_object('scalar_included', $included), json_object('values_json', json_array('[1]', '[2]'), 'encoded', json_array('a b', 'a%20b')) ); diff --git a/tests/sql_test_files/query_events/rows.sql b/tests/sql_test_files/query_events/rows.sql index f18110cb1..b1a09919b 100644 --- a/tests/sql_test_files/query_events/rows.sql +++ b/tests/sql_test_files/query_events/rows.sql @@ -1,6 +1,6 @@ select n, sqlpage.run_sql(path) as included -from (select 1 as n, 'tests/sql_test_files/query_events/child.sql' as path - union all select 2 as n, 'tests/sql_test_files/query_events/child.sql' as path) source_rows +from (select 'row1' as n, 'tests/sql_test_files/query_events/child.sql' as path + union all select 'row2' as n, 'tests/sql_test_files/query_events/child.sql' as path) source_rows order by n; set scalar_included = (select sqlpage.run_sql(path) from (select 'tests/sql_test_files/query_events/child.sql' as path) source_rows); select $scalar_included as scalar_included; From 01d62675fd3977c58e1da0a4a4f6af89ba1fa7a2 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Wed, 7 Oct 2026 11:13:26 +0000 Subject: [PATCH 4/5] Document database matrix and SQL regression test pitfalls --- AGENTS.md | 19 ++++++++++-- CONTRIBUTING.md | 79 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 87 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6448e0b28..ef135db2e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,9 +18,11 @@ JavaScript where needed. to a mutable SQLPage variable and is useful for reusing query results or controlling later statements. - **Request variables** (`src/webserver/request_variables.rs`, `src/webserver/http_request_info.rs`, `src/webserver/database/syntax_tree.rs`). `?name` refers to a URL/GET parameter, `:name` explicitly refers to a form/POST - value, and `$name` is the compatibility shorthand that uses a POST value when present and otherwise a GET - value (a SET variable takes precedence where applicable). Values are passed as parameters, not interpolated - into SQL. GET and POST variables are request inputs; SET variables are mutable during request execution. + value. The current implementation resolves `$name` through SET then GET and `:name` through SET then POST; + without a SET value, a matching POST field triggers a `$name` warning, but lookup does not fall back to POST. + Explicit SET NULL stops lookup. + Values are passed as parameters, not interpolated into SQL. GET and POST variables are request inputs; + SET variables are mutable during request execution. `sqlpage.variables()` exposes them as JSON, with SET > POST > GET precedence. - **SQLPage functions** (`src/webserver/database/sqlpage_functions/`). Calls such as `sqlpage.fetch`, `sqlpage.run_sql`, `sqlpage.set_variable`, file readers, hashing/HMAC helpers, request metadata, uploads, headers/cookies, URL helpers, OIDC user info, @@ -118,8 +120,19 @@ docker compose up --wait mssql # or postgres, mysql, mariadb, oracle DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test ``` +CI checks SQLite, PostgreSQL, MySQL, Microsoft SQL Server, Oracle via ODBC, and DuckDB via ODBC on Linux; +Windows also checks default SQLite. MariaDB is an optional local target, not a separate CI entry. +For SQL execution/binding/decoding changes, inspect the affected matrix results, not just local SQLite. + ODBC tests require the database-specific ODBC driver on the host; starting the container is not sufficient. See the Oracle and DuckDB matrix entries in [CI](./.github/workflows/ci.yml) for driver setup and connection strings. On Linux and macOS, `cargo test --features odbc-static` matches CI's static unixODBC linking. +Reuse existing SQL, request, transaction, and browser fixtures and parameterize repeated setup without +removing assertions or scenarios. Read [SQL regression test guidance](./CONTRIBUTING.md#writing-sql-regression-tests) +before adding portable SQL expectations: Oracle numeric JSON strings can differ (`1.0` versus `1`), SQLite +scalar SET queries take the first row where other engines error, SET NULL blocks request fallback, and +variable lookup differs from merged enumeration. Preserve one-connection tests for nested `run_sql`, +column ordering, and private-input exclusion. A test summary followed by a hung process is still a CI failure. + For dynamic frontend changes, run the Playwright tests under `tests/end-to-end/` as described in [CONTRIBUTING.md](./CONTRIBUTING.md). Component browser tests belong in `tests/end-to-end/fixtures//{index.sql,test.ts}` and must import the shared `fixture.ts` harness. Exercise components through SQL fixtures and normal page initialization; do not inject synthetic component DOM or call private initialization functions. Prefer Playwright locators and web-first assertions. For examples containing `test.hurl`, run `scripts/test-examples-hurl.sh `. ### Documentation diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 43487637c..8e8baa46e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,18 +84,81 @@ Run the backend tests: cargo test ``` -By default, the tests are run against an SQLite in-memory database. +By default, tests use in-memory SQLite. An exported `DATABASE_URL` overrides this default; +unset it when returning to SQLite. -If you want to run them against another database, -start a database server with `docker compose up database_name` (mssql, mysql, mariadb, or postgres) -and run the tests with the `DATABASE_URL` environment variable pointing to the database: +The Linux [CI database matrix](./.github/workflows/ci.yml) runs the same Rust test binaries against: + +| Database | Connection path | Local server/driver setup | +| --- | --- | --- | +| SQLite | Native | In-memory; no server required | +| PostgreSQL | Native | `docker compose up --wait postgres` | +| MySQL | Native | `docker compose up --wait mysql` | +| Microsoft SQL Server | Native | `docker compose up --wait mssql` | +| Oracle | ODBC | `docker compose up --wait oracle` and an Oracle ODBC driver on the host | +| DuckDB | ODBC | A DuckDB ODBC driver on the host; no server required | + +Windows also runs `cargo test` with the default SQLite database. MariaDB is available in +`docker-compose.yml` for additional local testing, but is not a separate CI matrix entry. + +For example, run the backend tests against SQL Server: ```bash -docker compose up mssql # or mysql, mariadb, postgres -export DATABASE_URL=mssql://root:Password123!@localhost/sqlpage -cargo test +docker compose up --wait mssql +DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test ``` +On Linux and macOS, `cargo test --features odbc-static` matches CI's static unixODBC linking. +This bundles the driver manager, **not** the database drivers. Oracle and DuckDB still need their +own installed drivers. Use the connection strings and driver setup in the CI matrix, including +Oracle's `LD_LIBRARY_PATH` and `ODBCSYSINI` environment settings. See +[`scripts/install-oracle-odbc.sh`](./scripts/install-oracle-odbc.sh) and +[`scripts/install-duckdb-odbc.sh`](./scripts/install-duckdb-odbc.sh) for the CI installation steps. + +A SQLite pass does not establish portability. For SQL execution, binding, or result decoding +changes, check the affected database matrix jobs before considering the change verified. +Distinguish an assertion failure from a driver/setup error or a timeout after the test summary; +a passing summary alone does not mean the test process exited successfully. + +#### Writing SQL regression tests + +Extend existing fixtures and parameterize shared setup before adding a new harness. Keep distinct +assertions for each behavior and failure mode; consolidate repetition without dropping scenarios. +Shared SQL fixtures live in [`tests/sql_test_files/`](./tests/sql_test_files/); database-specific +syntax belongs under `data/database-specific//`. Reuse the request and transaction suites +for precedence and rollback checks, and the common one-connection configuration for nested queries. + +Keep these differences in mind: + +- **Numbers inside JSON strings:** Oracle ODBC can serialize a selected numeric literal as `1.0` + while another driver or a JSON constructor produces `1`. A JSON string returned by + `sqlpage.run_sql` is compared as a string by the fixture harness, so these differ. Use text + markers when testing row order, or an explicit numeric comparison when testing numeric values; + do not weaken full-row assertions to substring checks to hide formatting differences. +- **Scalar `SET` queries:** Zero rows produce NULL. SQLite takes the first row when there are + several; other supported engines reject multiple rows. Each returned scalar row must have exactly + one output column, even when duplicate names would merge into one JSON property. Zero-row + queries return NULL without checking a row's column count. Keep engine-specific + cardinality expectations in the existing database-specific fixtures. +- **Variables:** The current implementation resolves `$name` through SET then GET, and `:name` + through SET then POST. With no SET value, a matching POST field triggers a `$name` warning; + lookup still does not fall back to POST. + An explicitly NULL SET value stops lookup rather than falling through to request inputs. + `sqlpage.variables()` has a different merged enumeration policy: SET > POST > GET. Test lookup + and enumeration separately, and check that `run_sql` child assignments do not change the parent. +- **Rows and computed values:** Duplicate physical output names accumulate values in fetched + column order; SQLPage-computed columns append afterwards, regardless of SELECT order. + For example, `select sqlpage.url_encode(name) as encoded, name as encoded` returns the physical + `name` value before the encoded value. Private function-input columns must stay out of response JSON. Nested + `run_sql` evaluation must release the active fetch stream before reusing its connection. Keep + the one-connection pool setting in regression fixtures so ownership mistakes cannot be hidden + by acquiring another connection. +- **Fixture conventions:** In JSON-mode tests, return `actual` and `expected`; an `expected` array + lists acceptable alternatives, not an expected row sequence. Compare a complete JSON string + when asserting row sequence. For HTML error fixtures, `error_.sql` supplies the expected + message (underscores become spaces). Use `_no` exclusions only for an established + backend limitation, with a comment explaining it. + ### End-to-End Tests We use Playwright for end-to-end testing of dynamic frontend features. @@ -230,7 +293,7 @@ git checkout -b feature/your-feature-name - Execute all tests across multiple platforms (Linux, Windows) - Build Docker images for multiple architectures - Run frontend linting, typechecking and unit tests (`npm test`) - - Test against multiple databases (SQLite, PostgreSQL, MySQL, MSSQL, Oracle, and ODBC) + - Test against SQLite, PostgreSQL, MySQL, Microsoft SQL Server, Oracle via ODBC, and DuckDB via ODBC ## Release Process From 1417d7259e9586b557589424a071d7d310350efd Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Fri, 9 Oct 2026 12:12:05 +0000 Subject: [PATCH 5/5] Fix Oracle cancellation assertion and prune contributor guidance --- AGENTS.md | 205 +++----------- CONTRIBUTING.md | 321 ++++------------------ src/webserver/database/execute_queries.rs | 9 +- 3 files changed, 96 insertions(+), 439 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ef135db2e..953898163 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,160 +1,45 @@ -# SQLPage architecture - -SQLPage is an SQL-only web application builder and web server. An application is primarily a set of `.sql` -files: SQLPage routes an HTTP request to a file, executes its statements against a database, interprets rows -whose `component` column names a UI component, and streams the resulting HTML (or another response) to the -client. It is intended for fast, data-centric applications while still allowing custom HTML, CSS, and -JavaScript where needed. - -## Features and repository layout - -- **Application entry point and configuration** (`src/main.rs`, `src/lib.rs`, `src/app_config.rs`, `src/cli/`). - The executable starts the server; application state, configuration, environment variables, and command-line - handling are defined here. `configuration.md` documents the user-facing settings. -- **SQL semantics and execution** (`src/webserver/database/`). SQLPage uses the database's SQL for selects, joins, aggregation, inserts, updates, - deletes, transactions, JSON processing, and database-specific features. It parses SQL, recognizes SQLPage - extensions, binds request values safely, and sends ordinary SQL to the selected database. SQL files contain - sequential statements; result sets become component invocations in response order. `SET` assigns a value - to a mutable SQLPage variable and is useful for reusing query results or controlling later statements. -- **Request variables** (`src/webserver/request_variables.rs`, `src/webserver/http_request_info.rs`, - `src/webserver/database/syntax_tree.rs`). `?name` refers to a URL/GET parameter, `:name` explicitly refers to a form/POST - value. The current implementation resolves `$name` through SET then GET and `:name` through SET then POST; - without a SET value, a matching POST field triggers a `$name` warning, but lookup does not fall back to POST. - Explicit SET NULL stops lookup. - Values are passed as parameters, not interpolated into SQL. GET and POST variables are request inputs; - SET variables are mutable during request execution. - `sqlpage.variables()` exposes them as JSON, with SET > POST > GET precedence. -- **SQLPage functions** (`src/webserver/database/sqlpage_functions/`). Calls such as `sqlpage.fetch`, `sqlpage.run_sql`, `sqlpage.set_variable`, file - readers, hashing/HMAC helpers, request metadata, uploads, headers/cookies, URL helpers, OIDC user info, - and HTTP fetch are registered in `src/webserver/database/sqlpage_functions/functions.rs`. Functions can - return values, alter response/request state, include another SQL file, or raise an error. `sqlpage.exec` - is deliberately disabled by default because it runs server processes. -- **Database support and pooling** (`src/webserver/database/connect.rs`, `execute_queries.rs`, `migrations.rs`). - Native drivers support SQLite, PostgreSQL, MySQL, and Microsoft SQL - Server; the ODBC driver provides access to other ODBC-compatible databases. SQLPage uses `sqlx` and a - reusable connection pool, with configurable maximum connections, idle/lifetime timeouts, acquire timeout, - retries, and optional `on_connect.sql`/`on_reset.sql` hooks. Database-specific SQL should be isolated or - covered by the relevant database tests. -- **Rendering and components** (`src/render.rs`, `src/templates.rs`, `src/dynamic_component.rs`, - `src/template_helpers.rs`, `sqlpage/templates/`, `frontend/src/`). Built-in components live in `sqlpage/templates/*.handlebars` and cover shells, text, - tables, lists, cards, charts, forms, navigation, modals, downloads, maps, and more. Query columns map to - component properties; nested/dynamic components and `sqlpage.run_sql` support composition and lazy loading. - Custom Handlebars components can be placed in the configured `sqlpage/templates` directory. Raw HTML and - custom assets are possible through the HTML/shell components. Rendering is streamed so the response can - start while later query results are still being processed. -- **Control flow and errors** (`src/webserver/error.rs`, `error_with_status.rs`, `routing.rs`, - `src/default_404.sql`). SQL remains declarative: use predicates, `CASE`, `SET`, component rows, and - the `redirect` component to conditionally continue, redirect, or implement guards/error pages. There is no - general SQLPage `IF` statement. Parse, database, function, component, and response errors are converted to - contextual HTTP errors; `default_404.sql` handles missing routes. Do not hide errors by changing unrelated - error handling or tests. -- **HTTP server and client** (`src/webserver/http.rs`, `http_client.rs`, `response_writer.rs`, `static_content.rs`, - `https.rs`, `content_security_policy.rs`, `server_timing.rs`). The server is built on Actix Web, supports normal HTTP request/response - handling, streaming, uploads, static assets, HTTP/2, HTTPS, and optional Unix sockets/serverless adapters. - The shared outbound client is used by HTTP-fetch and OIDC integrations and honors configured/native TLS - certificates and timeouts. Content-security-policy and response/header helpers are part of the request - pipeline. -- **OIDC** (`src/webserver/oidc.rs`, `src/webserver/database/sqlpage_functions/functions/user_info.rs`). - Optional OpenID Connect middleware protects configured path prefixes, performs provider discovery, - login/callback/logout, validates tokens, maintains an authenticated cookie, and exposes identity claims to - SQLPage functions. Configuration is in `AppConfig`/`configuration.md`; public paths can be excluded. -- **Caching and files** (`src/filesystem.rs`, `src/file_cache.rs`, `src/telemetry*.rs`). Parsed SQL files are cached. Files may come from the web root/filesystem or from the - database-backed `sqlpage_files` store, and templates/migrations are loaded from the configuration directory. - Telemetry, request timing, and debug logging help diagnose query, pool, and rendering performance. - -- **Examples, tests, and project operations** (`examples/`, `tests/`, `configuration.md`, `CONTRIBUTING.md`, - `README.md`, `.github/workflows/ci.yml`). Examples include the official documentation site and its migrations; - tests cover SQL fixtures, database variants, uploads, OIDC, and server timing. The contribution guide and CI - workflow define the development and validation conventions. -- **Deployment and local infrastructure** (`Dockerfile`, `lambda.Dockerfile`, `docker-compose.yml`, - `sqlpage.service`). These provide container, serverless, local database-testing, and service deployment support. - -## Documentation and release notes - -The official documentation site is itself an SQLPage application in `examples/official-site/`. Its database -schema and documentation content are created by the SQL migrations in -`examples/official-site/sqlpage/migrations/`; the site is recreated from scratch during deployment. Existing -official-site migrations are editable source files: update the migration that already documents a component, -function, configuration option, or feature in place. Do not create a new migration merely to update existing -documentation. Add a new migration only for genuinely new documentation content when that is the established -pattern for the relevant area. - -- Add or update a component's row, parameters, and examples in the component documentation migrations when - changing `sqlpage/templates/` or component behavior. -- Add or update a function's description, parameters, examples, and caveats in the function documentation - migrations when changing `sqlpage_functions` or any `sqlpage.*` function behavior. -- Update the relevant configuration documentation when adding or changing `AppConfig` settings, environment - variables, defaults, authentication behavior, HTTP/TLS behavior, database settings, or custom components. -- Document OIDC changes in the authentication/OIDC migrations, including configuration requirements, exposed - claims/functions, login/logout behavior, and security implications. -- Document other user-visible behavior—SQL syntax extensions, variables, control flow, errors, uploads, - rendering, HTTP endpoints, performance, or deployment—in the corresponding official-site SQL page or - migration. Follow nearby migrations and keep examples executable and database-portable where possible. -- Update `CHANGELOG.md` for user-visible changes only (new features, bug fixes, breaking changes, deprecations). Keep the entry concise and not too technical, focusing on the impact for users. Don't update the entry for a version that was already released (tagged). - -## Validation - -### When working on rust code -Mandatory formatting (rust): `cargo fmt --all` -Mandatory linting: `cargo clippy --all-targets --all-features -- -D warnings` - -### When working on css or js -Frontend formatting: `npm run format` -Mandatory frontend validation: `npm test` - -More about testing: see [github actions](./.github/workflows/ci.yml). -Contributor setup and validation: see [CONTRIBUTING.md](./CONTRIBUTING.md). Module overview: see [src/lib.rs](./src/lib.rs); architecture diagram: [docs/architecture-detailed.png](./docs/architecture-detailed.png). - -NEVER reformat/lint/touch files unrelated to your task. Always run tests/lints/format before stopping when you changed code. - -### Testing - -``` -cargo test # tests with in-memory SQLite by default -``` - -For other databases, see [docker testing setup](./docker-compose.yml) - -``` -docker compose up --wait mssql # or postgres, mysql, mariadb, oracle -DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test -``` - -CI checks SQLite, PostgreSQL, MySQL, Microsoft SQL Server, Oracle via ODBC, and DuckDB via ODBC on Linux; -Windows also checks default SQLite. MariaDB is an optional local target, not a separate CI entry. -For SQL execution/binding/decoding changes, inspect the affected matrix results, not just local SQLite. - -ODBC tests require the database-specific ODBC driver on the host; starting the container is not sufficient. See the Oracle and DuckDB matrix entries in [CI](./.github/workflows/ci.yml) for driver setup and connection strings. On Linux and macOS, `cargo test --features odbc-static` matches CI's static unixODBC linking. - -Reuse existing SQL, request, transaction, and browser fixtures and parameterize repeated setup without -removing assertions or scenarios. Read [SQL regression test guidance](./CONTRIBUTING.md#writing-sql-regression-tests) -before adding portable SQL expectations: Oracle numeric JSON strings can differ (`1.0` versus `1`), SQLite -scalar SET queries take the first row where other engines error, SET NULL blocks request fallback, and -variable lookup differs from merged enumeration. Preserve one-connection tests for nested `run_sql`, -column ordering, and private-input exclusion. A test summary followed by a hung process is still a CI failure. - -For dynamic frontend changes, run the Playwright tests under `tests/end-to-end/` as described in [CONTRIBUTING.md](./CONTRIBUTING.md). Component browser tests belong in `tests/end-to-end/fixtures//{index.sql,test.ts}` and must import the shared `fixture.ts` harness. Exercise components through SQL fixtures and normal page initialization; do not inject synthetic component DOM or call private initialization functions. Prefer Playwright locators and web-first assertions. For examples containing `test.hurl`, run `scripts/test-examples-hurl.sh `. - -### Documentation - -Components and functions are documented in [official-site migrations](./examples/official-site/sqlpage/migrations/). Edit the existing migration for an existing entity; add an appropriately ordered migration for a new entity. The official-site database is recreated from migrations on each deployment. - -official documentation website sql tables: - - `parameter_type(name)` -- the allowed values of `parameter.type` - - `component(name,description,icon,introduced_in_version)` -- icon name from tabler icon - - `parameter(top_level BOOLEAN, name, component REFERENCES component(name), description, description_md, type REFERENCES parameter_type(name), optional BOOLEAN)` parameter types: BOOLEAN, COLOR, HTML, ICON, INTEGER, JSON, REAL, TEXT, TIMESTAMP, URL. Set exactly one of `description` (plain text) and `description_md` (markdown). - - `example(component REFERENCES component(name), description, properties JSON)` - - `sqlpage_functions(name,icon,description_md,return_type,introduced_in_version)` - - `sqlpage_function_parameters(function,index,name,description_md,type)` - - `blog_posts(title,description,icon,external_url,content,created_at)` -- release announcements and long-form guides - - `example_cards(title,folder,db_engine,description)` - -#### Project Conventions - -- Built-in UI component templates: `sqlpage/templates/*.handlebars`; header/control components: `src/render.rs`. -- SQLPage functions: one `async fn` module under `src/webserver/database/sqlpage_functions/functions/`, declared with `mod` and registered with `sqlpage_functions!` in `functions.rs`. See its [README](./src/webserver/database/sqlpage_functions/README.md). -- [Configuration](./configuration.md): see [AppConfig](./src/app_config.rs) -- Routing: file-based in `src/webserver/routing.rs`. Missing paths use the nearest ancestor `404.sql`; without one, HTML uses `src/default_404.sql` and other formats receive a plain-text 404. -- Playwright component suites: `tests/end-to-end/fixtures//{index.sql,test.ts}` using `tests/end-to-end/fixture.ts`; official-site smoke tests remain in `tests/end-to-end/*.spec.ts`. -- Follow patterns from similar modules before introducing new abstractions. -- frontend: see [frontend/src](./frontend/src/), bundled by `npm run build` +# Working on SQLPage + +## Keep guidance small + +This file records durable architectural constraints and instructions for agents. +[CONTRIBUTING.md](./CONTRIBUTING.md) covers contributor setup and validation; +[CI](./.github/workflows/ci.yml) defines the supported test matrix. Discover module +layout, APIs, schemas, and detailed behavior from the code and nearby tests. + +Edit and prune these documents rather than appending after every task. Add guidance +only when it expresses a lasting rule that cannot be made clear in code. Put local +implementation caveats beside the implementation or regression test. Keep user-facing +reference material in the official documentation site. Do not duplicate it here. + +## Architectural contracts + +- SQL files execute statements sequentially and stream component rows in response + order. Preserve streaming, cancellation, transaction rollback, and contextual + errors; do not hide failures through unrelated error-handling changes. +- Request inputs are bound parameters, never interpolated into SQL. Preserve the + distinction between request inputs and mutable SET variables, including explicit + NULL and child execution isolation. +- Database behavior must remain portable across supported drivers. Isolate + engine-specific SQL and verify execution, binding, and decoding changes against + the affected CI matrix entries. +- Nested SQL execution must release an active fetch stream before reusing its + connection. Preserve one-connection regression coverage, output column ordering, + and exclusion of private function inputs from responses. +- Extend existing module and fixture patterns before introducing abstractions. + Browser component tests must exercise SQL fixtures and normal page initialization + through the shared Playwright harness. + +## Changes and validation + +Keep changes scoped to the task, including formatting. Reuse existing tests and +parameterize repeated setup without removing assertions or scenarios. Run the +relevant tests and the formatting/lint checks in CONTRIBUTING.md before stopping. +Report unavailable checks and failures accurately; a test summary followed by a +hung process is a failure. + +The official site in `examples/official-site/` is rebuilt from its SQL migrations +on deployment. Update the existing page or documentation migration for changed +behavior; add a migration only for new documentation content. Document user-visible +changes there and in configuration.md when applicable. Add a concise CHANGELOG.md +entry only for user-visible changes, and never edit an already tagged release. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8e8baa46e..358d6fffa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,309 +1,80 @@ # Contributing to SQLPage -Thank you for your interest in contributing to SQLPage! This document will guide you through the contribution process. +## Setup and build -## Development Setup - -1. Install Rust and Cargo (latest stable version): https://www.rust-lang.org/tools/install -2. Install Node.js: https://nodejs.org/en/download/ -3. Clone the repository - -```bash -git clone https://github.com/sqlpage/sqlpage -cd sqlpage -``` - -## Building the project - -The first time you build the project, Cargo and npm download their -dependencies, so you will need internet access, and the build may take a while. - -Run this command from the root of a checkout to install the locked npm dependencies, build the browser assets, and build SQLPage in development mode: +Install stable Rust and Node.js (see [CI](./.github/actions/install-frontend-dependencies/action.yml) +for the Node version), then run this command from a checkout: ```bash npm run build:rust ``` -After the browser assets have been built, you can run `cargo build` directly for Rust-only changes. Run `npm run build` again after changing frontend sources. Builds from the published crate use its included browser assets and do not need Node.js. +This installs locked npm dependencies, builds browser assets, and creates +`target/debug/sqlpage`. Afterwards, use `cargo build` for Rust changes and +`npm run build` for frontend changes. Add `-- --release` to `npm run build:rust` +for a release build. -The resulting executable will be in `target/debug/sqlpage`. +Default builds require a system ODBC driver manager. On Linux and macOS, +`--features odbc-static` bundles unixODBC, but database-specific drivers must still +be installed separately. Windows uses its system ODBC driver manager. Published +crates include browser assets and do not require Node.js. -### Release mode +## Validation -To build the project in release mode: +For Rust changes: ```bash -npm run build:rust -- --release -``` - -The resulting executable will be in `target/release/sqlpage`. - -### ODBC build modes - -SQLPage can either be built with an integrated odbc driver manager (static linking), -or depend on having one already installed on the system where it is running (dynamic linking). - -- Dynamic ODBC (default): `npm run build:rust` -- Static ODBC (Linux and MacOS only): `npm run build:rust -- --features odbc-static` - -Windows comes with ODBC pre-installed; SQLPage cannot statically link to the unixODBC driver manager on windows. - -## Code Style and Linting - -### Rust - -- Use `cargo fmt --all` to format your Rust code -- Run `cargo clippy` to catch common mistakes and improve code quality -- All code must pass the following checks: - -```bash -cargo fmt --all -- --check +cargo fmt --all cargo clippy --all-targets --all-features -- -D warnings -``` - -### Frontend - -We use Biome for linting and formatting of the frontend code, and TypeScript -to typecheck it. - -```bash -npm install # once -npm run format # apply formatting -npm test # the check CI runs: biome, typecheck, and the frontend unit tests -``` - -`npm test` checks the entire frontend codebase (html, css, js, ts). - -## Testing - -### Rust Tests - -Run the backend tests: - -```bash cargo test ``` -By default, tests use in-memory SQLite. An exported `DATABASE_URL` overrides this default; -unset it when returning to SQLite. - -The Linux [CI database matrix](./.github/workflows/ci.yml) runs the same Rust test binaries against: - -| Database | Connection path | Local server/driver setup | -| --- | --- | --- | -| SQLite | Native | In-memory; no server required | -| PostgreSQL | Native | `docker compose up --wait postgres` | -| MySQL | Native | `docker compose up --wait mysql` | -| Microsoft SQL Server | Native | `docker compose up --wait mssql` | -| Oracle | ODBC | `docker compose up --wait oracle` and an Oracle ODBC driver on the host | -| DuckDB | ODBC | A DuckDB ODBC driver on the host; no server required | - -Windows also runs `cargo test` with the default SQLite database. MariaDB is available in -`docker-compose.yml` for additional local testing, but is not a separate CI matrix entry. - -For example, run the backend tests against SQL Server: +For frontend changes: ```bash -docker compose up --wait mssql -DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test +npm run format +npm test +npm run build ``` -On Linux and macOS, `cargo test --features odbc-static` matches CI's static unixODBC linking. -This bundles the driver manager, **not** the database drivers. Oracle and DuckDB still need their -own installed drivers. Use the connection strings and driver setup in the CI matrix, including -Oracle's `LD_LIBRARY_PATH` and `ODBCSYSINI` environment settings. See -[`scripts/install-oracle-odbc.sh`](./scripts/install-oracle-odbc.sh) and -[`scripts/install-duckdb-odbc.sh`](./scripts/install-duckdb-odbc.sh) for the CI installation steps. - -A SQLite pass does not establish portability. For SQL execution, binding, or result decoding -changes, check the affected database matrix jobs before considering the change verified. -Distinguish an assertion failure from a driver/setup error or a timeout after the test summary; -a passing summary alone does not mean the test process exited successfully. - -#### Writing SQL regression tests - -Extend existing fixtures and parameterize shared setup before adding a new harness. Keep distinct -assertions for each behavior and failure mode; consolidate repetition without dropping scenarios. -Shared SQL fixtures live in [`tests/sql_test_files/`](./tests/sql_test_files/); database-specific -syntax belongs under `data/database-specific//`. Reuse the request and transaction suites -for precedence and rollback checks, and the common one-connection configuration for nested queries. - -Keep these differences in mind: - -- **Numbers inside JSON strings:** Oracle ODBC can serialize a selected numeric literal as `1.0` - while another driver or a JSON constructor produces `1`. A JSON string returned by - `sqlpage.run_sql` is compared as a string by the fixture harness, so these differ. Use text - markers when testing row order, or an explicit numeric comparison when testing numeric values; - do not weaken full-row assertions to substring checks to hide formatting differences. -- **Scalar `SET` queries:** Zero rows produce NULL. SQLite takes the first row when there are - several; other supported engines reject multiple rows. Each returned scalar row must have exactly - one output column, even when duplicate names would merge into one JSON property. Zero-row - queries return NULL without checking a row's column count. Keep engine-specific - cardinality expectations in the existing database-specific fixtures. -- **Variables:** The current implementation resolves `$name` through SET then GET, and `:name` - through SET then POST. With no SET value, a matching POST field triggers a `$name` warning; - lookup still does not fall back to POST. - An explicitly NULL SET value stops lookup rather than falling through to request inputs. - `sqlpage.variables()` has a different merged enumeration policy: SET > POST > GET. Test lookup - and enumeration separately, and check that `run_sql` child assignments do not change the parent. -- **Rows and computed values:** Duplicate physical output names accumulate values in fetched - column order; SQLPage-computed columns append afterwards, regardless of SELECT order. - For example, `select sqlpage.url_encode(name) as encoded, name as encoded` returns the physical - `name` value before the encoded value. Private function-input columns must stay out of response JSON. Nested - `run_sql` evaluation must release the active fetch stream before reusing its connection. Keep - the one-connection pool setting in regression fixtures so ownership mistakes cannot be hidden - by acquiring another connection. -- **Fixture conventions:** In JSON-mode tests, return `actual` and `expected`; an `expected` array - lists acceptable alternatives, not an expected row sequence. Compare a complete JSON string - when asserting row sequence. For HTML error fixtures, `error_.sql` supplies the expected - message (underscores become spaces). Use `_no` exclusions only for an established - backend limitation, with a comment explaining it. - -### End-to-End Tests - -We use Playwright for end-to-end testing of dynamic frontend features. -Tests are located in [`tests/end-to-end/`](./tests/end-to-end/). Key areas covered include: - -Component tests use deterministic SQL applications under `tests/end-to-end/fixtures//`. -Each suite contains an `index.sql` page and a `test.ts` file. Import `test` and `expect` from -`../../fixture`; the shared fixture opens the matching SQL page before every test and waits for -all SQLPage components to initialize. Prefer role, label, and text locators over CSS selectors -when the assertion does not specifically concern generated markup. - -Keep official-site smoke and integration tests in root-level `*.spec.ts` files. Component behavior -tests should render real components through their SQL fixture; do not inject component markup or -invoke SQLPage's JavaScript initialization functions directly. Parameterized fixtures may accept -request variables when several tests need the same component with different data. - -#### Run the tests +Keep formatting scoped to changed files. `npm test` checks formatting, lint, +types, and frontend unit tests. Dynamic frontend changes also need Playwright: ```bash -npm install cd tests/end-to-end -npx playwright install chromium -npm run test +npx playwright install --with-deps --only-shell chromium +npm test ``` -Playwright starts both servers itself on a free port. Set `SQLPAGE_BINARY` to run the servers from an already compiled binary instead of `cargo run`. - -## Documentation - -### Component Documentation - -When adding new components, comprehensive documentation is required. Example from a component documentation: - -```sql -INSERT INTO component(name, icon, description, introduced_in_version) VALUES - ('component_name', 'icon_name', 'Description of the component', 'version'); - --- Document all parameters -INSERT INTO parameter(component, name, description, type, top_level, optional) -VALUES ('component_name', 'param_name', 'param_description', 'TEXT|BOOLEAN|INTEGER|JSON|ICON|COLOR|HTML|REAL|TIMESTAMP|URL', false, true); - --- Use description_md instead of description when the text contains markdown -INSERT INTO parameter(component, name, description_md, type, top_level, optional) -VALUES ('component_name', 'other_param', 'Set to `true` to see [the docs](/documentation.sql).', 'BOOLEAN', true, true); - --- Include usage examples -INSERT INTO example(component, description, properties) VALUES - ('component_name', 'Example description in markdown', JSON('[ -{"component": "new_component_name", "top_level_property_1": "value1", "top_level_property_2": "value2"}, -{"row_level_property_1": "value1", "row_level_property_2": "value2"} -]')); -``` - -Component documentation is stored in [`./examples/official-site/sqlpage/migrations/`](./examples/official-site/sqlpage/migrations/). - -If you are editing an existing component, edit the existing sql documentation file directly. -If you are adding a new component, add a new sql file in the folder, and add the appropriate insert statements above. +Playwright starts its servers on free ports. Set `SQLPAGE_BINARY` to use an already +built executable. Component suites use `fixtures//{index.sql,test.ts}` and +import the shared `fixture.ts` harness; root-level `*.spec.ts` files cover the +official site. For examples with `test.hurl`, run +`scripts/test-examples-hurl.sh ''` from the repository root. -### SQLPage Function Documentation - -When adding new SQLPage functions, document them using a SQL migrations. Example structure: - -```sql --- Function Definition -INSERT INTO sqlpage_functions ( - "name", - "introduced_in_version", - "icon", - "description_md" -) -VALUES ( - 'your_function_name', - '1.0.0', - 'function-icon-name', - 'Description of what the function does. - -### Example - - select ''text'' as component, sqlpage.your_function_name(''parameter'') as result; - -Additional markdown documentation, usage notes, and examples go here. -'); - --- Function Parameters -INSERT INTO sqlpage_function_parameters ( - "function", - "index", - "name", - "description_md", - "type" -) -VALUES ( - 'your_function_name', - 1, - 'parameter_name', - 'Description of what this parameter does and how to use it.', - 'TEXT|BOOLEAN|INTEGER|JSON' -); -``` - -Key elements to include in function documentation: - -- Clear description of the function's purpose -- Version number where the function was introduced -- Appropriate icon -- Markdown-formatted documentation with examples -- All parameters documented with clear descriptions and types -- Security considerations if applicable -- Example usage scenarios - -## Pull Request Process - -1. Create a new branch for your feature/fix: +Rust tests default to in-memory SQLite; `DATABASE_URL` selects another database. +For example: ```bash -git checkout -b feature/your-feature-name +docker compose up --wait mssql +DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test ``` -2. Make your changes, ensuring: - -- All tests pass -- Code is properly formatted -- New features are documented -- tests cover new functionality -- `CHANGELOG.md` has an entry for any user-visible change - -3. Push your changes and create a Pull Request - -4. CI Checks - Our CI pipeline will automatically: - - Run Rust formatting and clippy checks - - Execute all tests across multiple platforms (Linux, Windows) - - Build Docker images for multiple architectures - - Run frontend linting, typechecking and unit tests (`npm test`) - - Test against SQLite, PostgreSQL, MySQL, Microsoft SQL Server, Oracle via ODBC, and DuckDB via ODBC - -## Release Process - -Releases are automated when pushing tags that match the pattern `v*` (e.g., `v1.0.0`). The CI pipeline will: +Use the [CI matrix](./.github/workflows/ci.yml) for supported backends, connection +strings, and driver setup. On Linux and macOS, `cargo test --features odbc-static` +matches CI's driver-manager linking. Oracle and DuckDB need host ODBC drivers even +when the database runs in a container. Check affected matrix results for SQL +execution changes; SQLite alone cannot establish portability. -- Build and test the code -- Create Docker images for multiple architectures -- Push images to Docker Hub -- Create GitHub releases +## Submitting a change -## Questions? +Follow nearby code and tests, add regression coverage, and describe the resulting +behavior and validation in the pull request. Keep changes and formatting focused. +Architectural constraints and documentation maintenance rules are in +[AGENTS.md](./AGENTS.md). -If you have any questions, feel free to open an issue or discussion on GitHub. +User documentation lives in [the official-site SQL pages and migrations](./examples/official-site/). +Edit existing documentation in place: deployment recreates the site database. +Use nearby entries as examples rather than copying schemas into this guide. Update +[configuration.md](./configuration.md) for configuration changes and +[CHANGELOG.md](./CHANGELOG.md) for user-visible changes in an unreleased version. diff --git a/src/webserver/database/execute_queries.rs b/src/webserver/database/execute_queries.rs index 500da529a..475380d44 100644 --- a/src/webserver/database/execute_queries.rs +++ b/src/webserver/database/execute_queries.rs @@ -938,8 +938,9 @@ mod tests { .unwrap(); let file = SqlFile::new( &request.app_state.db, + // Text markers keep this ordering test independent of driver numeric representations. "SELECT n, sqlpage.fetch(url) AS value - FROM (SELECT 1 AS n, NULL AS url UNION ALL SELECT 2 AS n, 'invalid URL' AS url) source_rows + FROM (SELECT 'row1' AS n, NULL AS url UNION ALL SELECT 'row2' AS n, 'invalid URL' AS url) source_rows ORDER BY n; SET after_query = 'unexpected'", Path::new("execution-test.sql"), @@ -951,12 +952,12 @@ mod tests { let Some(DbItem::Row(row)) = stream.next().await else { panic!("Expected the first row before the later function error"); }; - assert_eq!(row, json!({"n": 1, "value": null})); + assert_eq!(row, json!({"n": "row1", "value": null})); } assert!(!request.set_variables.borrow().contains_key("after_query")); let file = SqlFile::new( &request.app_state.db, - "SELECT n FROM (SELECT 7 AS n) source_rows", + "SELECT n FROM (SELECT 'reused' AS n) source_rows", Path::new("execution-test.sql"), ); let stream = stream_query_results_with_conn(&file, &request, &mut connection); @@ -964,7 +965,7 @@ mod tests { let Some(DbItem::Row(row)) = stream.next().await else { panic!("Expected a row from the reused connection"); }; - assert_eq!(row, json!({"n": 7})); + assert_eq!(row, json!({"n": "reused"})); while let Some(item) = stream.next().await { assert!(matches!(item, DbItem::FinishedQuery)); }