Skip to content

Commit 75ade13

Browse files
lovasoacursoragent
andauthored
Refactor request handling to separate data and state (#1111)
* Make URL and POST parameters immutable, separate from SET variables - URL and POST parameters are now immutable after request initialization - SET command creates user-defined variables in separate namespace - Variable lookup: SET variables shadow request parameters - Added sqlpage.variables('set') to inspect user-defined variables - Simplified API: most functions now use &RequestInfo instead of &mut - All tests passing (151 total) * Restore deprecation warning for SET on POST variable names * Restore deprecation warnings for $var accessing POST variables - Warn when both URL and POST have same variable name - Warn when $var is used for POST-only variable (should use :var) * Simplify run_sql: always use clone_without_variables No need to branch on whether variables are provided since we clone in both cases anyway. * Revert "Simplify run_sql: always use clone_without_variables" This reverts commit 60f5a05. * WIP: Add ExecutionContext to separate mutable state from RequestInfo This is a draft refactoring to avoid cloning large immutable data (headers, cookies, body) when creating nested execution contexts in run_sql(). Changes: - RequestInfo now contains only immutable request data - ExecutionContext wraps Rc<RequestInfo> + mutable execution state - Avoids cloning potentially large strings in nested run_sql() calls Status: NOT COMPILING YET - this is work in progress * Refactor: Rename RequestInfo to ExecutionContext Co-authored-by: contact <contact@ophir.dev> * avoid cloning request * improve run_sql invalid variables error message * changelog --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 34f5d7f commit 75ade13

File tree

8 files changed

+105
-87
lines changed

8 files changed

+105
-87
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
- `sqlpage.variables('set')` returns only user-defined SET variables as JSON
2626
- `sqlpage.variables()` returns all variables merged together, with SET variables taking precedence
2727
- **Deprecation warnings**: Using `$var` when both a URL parameter and POST parameter exist with the same name now shows a warning. In a future version, you'll need to explicitly choose between `$var` (URL) and `:var` (POST).
28+
- Improved performance of `sqlpage.run_sql`.
29+
- On a simple test that just runs 4 run_sql calls, the new version is about 2.7x faster (15,708 req/s vs 5,782 req/s) with lower latency (0.637 ms vs 1.730 ms per request).
2830
- add support for postgres range types
2931

3032
## v0.39.1 (2025-11-08)

src/webserver/database/execute_queries.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use super::sql::{
1515
use crate::dynamic_component::parse_dynamic_rows;
1616
use crate::utils::add_value_to_map;
1717
use crate::webserver::database::sql_to_json::row_to_string;
18-
use crate::webserver::http_request_info::RequestInfo;
18+
use crate::webserver::http_request_info::ExecutionContext;
1919
use crate::webserver::single_or_vec::SingleOrVec;
2020

2121
use super::syntax_tree::{extract_req_param, StmtParam};
@@ -44,7 +44,7 @@ impl Database {
4444

4545
pub fn stream_query_results_with_conn<'a>(
4646
sql_file: &'a ParsedSqlFile,
47-
request: &'a RequestInfo,
47+
request: &'a ExecutionContext,
4848
db_connection: &'a mut DbConn,
4949
) -> impl Stream<Item = DbItem> + 'a {
5050
let source_file = &sql_file.source_path;
@@ -131,7 +131,7 @@ pub fn stop_at_first_error(
131131
/// Executes the sqlpage pseudo-functions contained in a static simple select
132132
async fn exec_static_simple_select(
133133
columns: &[(String, SimpleSelectValue)],
134-
req: &RequestInfo,
134+
req: &ExecutionContext,
135135
db_connection: &mut DbConn,
136136
) -> anyhow::Result<serde_json::Value> {
137137
let mut map = serde_json::Map::with_capacity(columns.len());
@@ -161,7 +161,7 @@ async fn try_rollback_transaction(db_connection: &mut AnyConnection) {
161161
/// Returns `Ok(None)` when NULL should be used as the parameter value.
162162
async fn extract_req_param_as_json(
163163
param: &StmtParam,
164-
request: &RequestInfo,
164+
request: &ExecutionContext,
165165
db_connection: &mut DbConn,
166166
) -> anyhow::Result<serde_json::Value> {
167167
if let Some(val) = extract_req_param(param, request, db_connection).await? {
@@ -175,7 +175,7 @@ async fn extract_req_param_as_json(
175175
/// This allows recursive calls.
176176
pub fn stream_query_results_boxed<'a>(
177177
sql_file: &'a ParsedSqlFile,
178-
request: &'a RequestInfo,
178+
request: &'a ExecutionContext,
179179
db_connection: &'a mut DbConn,
180180
) -> Pin<Box<dyn Stream<Item = DbItem> + 'a>> {
181181
Box::pin(stream_query_results_with_conn(
@@ -187,7 +187,7 @@ pub fn stream_query_results_boxed<'a>(
187187

188188
async fn execute_set_variable_query<'a>(
189189
db_connection: &'a mut DbConn,
190-
request: &'a RequestInfo,
190+
request: &'a ExecutionContext,
191191
variable: &StmtParam,
192192
statement: &StmtWithParams,
193193
source_file: &Path,
@@ -223,7 +223,7 @@ async fn execute_set_variable_query<'a>(
223223

224224
async fn execute_set_simple_static<'a>(
225225
db_connection: &'a mut DbConn,
226-
request: &'a RequestInfo,
226+
request: &'a ExecutionContext,
227227
variable: &StmtParam,
228228
value: &SimpleSelectValue,
229229
_source_file: &Path,
@@ -254,7 +254,7 @@ async fn execute_set_simple_static<'a>(
254254
}
255255

256256
fn vars_and_name<'a, 'b>(
257-
request: &'a RequestInfo,
257+
request: &'a ExecutionContext,
258258
variable: &'b StmtParam,
259259
) -> anyhow::Result<(std::cell::RefMut<'a, HashMap<String, SingleOrVec>>, &'b str)> {
260260
match variable {
@@ -274,7 +274,7 @@ fn vars_and_name<'a, 'b>(
274274
async fn take_connection<'a>(
275275
db: &'a Database,
276276
conn: &'a mut DbConn,
277-
request: &RequestInfo,
277+
request: &ExecutionContext,
278278
) -> anyhow::Result<&'a mut PoolConnection<sqlx::Any>> {
279279
if let Some(c) = conn {
280280
return Ok(c);
@@ -349,7 +349,7 @@ fn clone_anyhow_err(source_file: &Path, err: &anyhow::Error) -> anyhow::Error {
349349

350350
async fn bind_parameters<'a>(
351351
stmt: &'a StmtWithParams,
352-
request: &'a RequestInfo,
352+
request: &'a ExecutionContext,
353353
db_connection: &mut DbConn,
354354
) -> anyhow::Result<StatementWithParams<'a>> {
355355
let sql = stmt.query.as_str();
@@ -378,7 +378,7 @@ async fn bind_parameters<'a>(
378378
}
379379

380380
async fn apply_delayed_functions(
381-
request: &RequestInfo,
381+
request: &ExecutionContext,
382382
delayed_functions: &[DelayedFunctionCall],
383383
item: &mut DbItem,
384384
) -> anyhow::Result<()> {
@@ -399,7 +399,7 @@ async fn apply_delayed_functions(
399399
}
400400

401401
async fn apply_single_delayed_function(
402-
request: &RequestInfo,
402+
request: &ExecutionContext,
403403
db_connection: &mut DbConn,
404404
f: &DelayedFunctionCall,
405405
row: &mut serde_json::Map<String, serde_json::Value>,

src/webserver/database/sqlpage_functions/function_definition_macro.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ macro_rules! sqlpage_functions {
5555
pub(crate) async fn evaluate<'a>(
5656
&self,
5757
#[allow(unused_variables)]
58-
request: &'a RequestInfo,
58+
request: &'a $crate::webserver::http_request_info::ExecutionContext,
5959
db_connection: &mut Option<sqlx::pool::PoolConnection<sqlx::Any>>,
6060
params: Vec<Option<Cow<'a, str>>>
6161
) -> anyhow::Result<Option<Cow<'a, str>>> {

src/webserver/database/sqlpage_functions/functions.rs

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::RequestInfo;
1+
use super::{ExecutionContext, RequestInfo};
22
use crate::webserver::{
33
database::{
44
blob_to_data_url::vec_to_data_uri_with_mime, execute_queries::DbConn,
@@ -45,15 +45,15 @@ super::function_definition_macro::sqlpage_functions! {
4545
read_file_as_data_url((&RequestInfo), file_path: Option<Cow<str>>);
4646
read_file_as_text((&RequestInfo), file_path: Option<Cow<str>>);
4747
request_method((&RequestInfo));
48-
run_sql((&RequestInfo, &mut DbConn), sql_file_path: Option<Cow<str>>, variables: Option<Cow<str>>);
48+
run_sql((&ExecutionContext, &mut DbConn), sql_file_path: Option<Cow<str>>, variables: Option<Cow<str>>);
4949

5050
uploaded_file_mime_type((&RequestInfo), upload_name: Cow<str>);
5151
uploaded_file_path((&RequestInfo), upload_name: Cow<str>);
5252
uploaded_file_name((&RequestInfo), upload_name: Cow<str>);
5353
url_encode(raw_text: Option<Cow<str>>);
5454
user_info((&RequestInfo), claim: Cow<str>);
5555

56-
variables((&RequestInfo), get_or_post: Option<Cow<str>>);
56+
variables((&ExecutionContext), get_or_post: Option<Cow<str>>);
5757
version();
5858
request_body((&RequestInfo));
5959
request_body_base64((&RequestInfo));
@@ -549,7 +549,7 @@ async fn request_method(request: &RequestInfo) -> String {
549549
}
550550

551551
async fn run_sql<'a>(
552-
request: &'a RequestInfo,
552+
request: &'a ExecutionContext,
553553
db_connection: &mut DbConn,
554554
sql_file_path: Option<Cow<'a, str>>,
555555
variables: Option<Cow<'a, str>>,
@@ -570,19 +570,12 @@ async fn run_sql<'a>(
570570
.await
571571
.with_context(|| format!("run_sql: invalid path {sql_file_path:?}"))?;
572572
let tmp_req = if let Some(variables) = variables {
573-
let tmp_req = request.clone_without_variables();
574-
let variables: ParamMap = serde_json::from_str(&variables).map_err(|err| {
575-
let context = format!(
576-
"run_sql: unable to parse the variables argument (line {}, column {})",
577-
err.line(),
578-
err.column()
579-
);
580-
anyhow::Error::new(err).context(context)
573+
let variables: ParamMap = serde_json::from_str(&variables).with_context(|| {
574+
format!("run_sql(\'{sql_file_path}\', \'{variables}\'): the second argument should be a JSON object with string keys and values")
581575
})?;
582-
tmp_req.set_variables.replace(variables);
583-
tmp_req
576+
request.fork_with_variables(variables)
584577
} else {
585-
request.clone()
578+
request.fork()
586579
};
587580
let max_recursion_depth = app_state.config.max_recursion_depth;
588581
if tmp_req.clone_depth > max_recursion_depth {
@@ -686,7 +679,7 @@ async fn url_encode(raw_text: Option<Cow<'_, str>>) -> Option<Cow<'_, str>> {
686679

687680
/// Returns all variables in the request as a JSON object.
688681
async fn variables<'a>(
689-
request: &'a RequestInfo,
682+
request: &'a ExecutionContext,
690683
get_or_post: Option<Cow<'a, str>>,
691684
) -> anyhow::Result<String> {
692685
Ok(if let Some(get_or_post) = get_or_post {

src/webserver/database/sqlpage_functions/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ mod url_parameter_deserializer;
66

77
use sqlparser::ast::FunctionArg;
88

9-
use crate::webserver::http_request_info::RequestInfo;
9+
use crate::webserver::http_request_info::{ExecutionContext, RequestInfo};
1010

1111
use super::sql::function_args_to_stmt_params;
1212
use super::syntax_tree::SqlPageFunctionCall;

src/webserver/database/syntax_tree.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::str::FromStr;
1616

1717
use sqlparser::ast::FunctionArg;
1818

19-
use crate::webserver::http_request_info::RequestInfo;
19+
use crate::webserver::http_request_info::ExecutionContext;
2020
use crate::webserver::single_or_vec::SingleOrVec;
2121

2222
use super::{
@@ -112,7 +112,7 @@ impl SqlPageFunctionCall {
112112

113113
pub async fn evaluate<'a>(
114114
&self,
115-
request: &'a RequestInfo,
115+
request: &'a ExecutionContext,
116116
db_connection: &mut DbConn,
117117
) -> anyhow::Result<Option<Cow<'a, str>>> {
118118
let mut params = Vec::with_capacity(self.arguments.len());
@@ -151,7 +151,7 @@ impl std::fmt::Display for SqlPageFunctionCall {
151151
/// Returns `Ok(None)` when NULL should be used as the parameter value.
152152
pub(super) async fn extract_req_param<'a>(
153153
param: &StmtParam,
154-
request: &'a RequestInfo,
154+
request: &'a ExecutionContext,
155155
db_connection: &mut DbConn,
156156
) -> anyhow::Result<Option<Cow<'a, str>>> {
157157
Ok(match param {
@@ -169,21 +169,20 @@ pub(super) async fn extract_req_param<'a>(
169169
Some(Cow::Owned(val.as_json_str().into_owned()))
170170
} else {
171171
let url_val = request.url_params.get(x);
172-
let post_val = request.post_variables.get(x);
173-
if let Some(post_val) = post_val {
174-
if let Some(url_val) = url_val {
172+
if request.post_variables.contains_key(x) {
173+
if url_val.is_some() {
175174
log::warn!(
176-
"Deprecation warning! There is both a URL parameter named '{x}' with value '{url_val}' and a form field named '{x}' with value '{post_val}'. \
177-
SQLPage is using the value from the form submission, but this is ambiguous, can lead to unexpected behavior, and will stop working in a future version of SQLPage. \
178-
To fix this, please rename the URL parameter to something else, and reference the form field with :{x}."
175+
"Deprecation warning! There is both a URL parameter named '{x}' and a form field named '{x}'. \
176+
SQLPage is using the URL parameter for ${x}. Please use :{x} to reference the form field explicitly."
179177
);
180178
} else {
181-
log::warn!("Deprecation warning! ${x} was used to reference a form field value (a POST variable) instead of a URL parameter. This will stop working soon. Please use :{x} instead.");
179+
log::warn!(
180+
"Deprecation warning! ${x} was used to reference a form field value (a POST variable). \
181+
This now uses only URL parameters. Please use :{x} instead."
182+
);
182183
}
183-
Some(post_val.as_json_str())
184-
} else {
185-
url_val.map(SingleOrVec::as_json_str)
186184
}
185+
url_val.map(SingleOrVec::as_json_str)
187186
}
188187
}
189188
StmtParam::Error(x) => anyhow::bail!("{x}"),
@@ -210,7 +209,7 @@ pub(super) async fn extract_req_param<'a>(
210209

211210
async fn concat_params<'a>(
212211
args: &[StmtParam],
213-
request: &'a RequestInfo,
212+
request: &'a ExecutionContext,
214213
db_connection: &mut DbConn,
215214
) -> anyhow::Result<Option<Cow<'a, str>>> {
216215
let mut result = String::new();
@@ -225,7 +224,7 @@ async fn concat_params<'a>(
225224

226225
async fn coalesce_params<'a>(
227226
args: &[StmtParam],
228-
request: &'a RequestInfo,
227+
request: &'a ExecutionContext,
229228
db_connection: &mut DbConn,
230229
) -> anyhow::Result<Option<Cow<'a, str>>> {
231230
for arg in args {
@@ -238,7 +237,7 @@ async fn coalesce_params<'a>(
238237

239238
async fn json_object_params<'a>(
240239
args: &[StmtParam],
241-
request: &'a RequestInfo,
240+
request: &'a ExecutionContext,
242241
db_connection: &mut DbConn,
243242
) -> anyhow::Result<Option<Cow<'a, str>>> {
244243
use serde::{ser::SerializeMap, Serializer};
@@ -276,7 +275,7 @@ async fn json_object_params<'a>(
276275

277276
async fn json_array_params<'a>(
278277
args: &[StmtParam],
279-
request: &'a RequestInfo,
278+
request: &'a ExecutionContext,
280279
db_connection: &mut DbConn,
281280
) -> anyhow::Result<Option<Cow<'a, str>>> {
282281
use serde::{ser::SerializeSeq, Serializer};

src/webserver/http.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,25 +174,26 @@ async fn render_sql(
174174
.clone()
175175
.into_inner();
176176

177-
let req_param = extract_request_info(srv_req, Arc::clone(&app_state), server_timing)
177+
let exec_ctx = extract_request_info(srv_req, Arc::clone(&app_state), server_timing)
178178
.await
179179
.map_err(|e| anyhow_err_to_actix(e, &app_state))?;
180-
log::debug!("Received a request with the following parameters: {req_param:?}");
180+
log::debug!("Received a request with the following parameters: {exec_ctx:?}");
181181

182-
req_param.server_timing.record("parse_req");
182+
exec_ctx.request().server_timing.record("parse_req");
183183

184184
let (resp_send, resp_recv) = tokio::sync::oneshot::channel::<HttpResponse>();
185185
let source_path: PathBuf = sql_file.source_path.clone();
186186
actix_web::rt::spawn(async move {
187+
let request_info = exec_ctx.request();
187188
let request_context = RequestContext {
188-
is_embedded: req_param.url_params.contains_key("_sqlpage_embed"),
189+
is_embedded: request_info.url_params.contains_key("_sqlpage_embed"),
189190
source_path,
190191
content_security_policy: ContentSecurityPolicy::with_random_nonce(),
191-
server_timing: Arc::clone(&req_param.server_timing),
192+
server_timing: Arc::clone(&request_info.server_timing),
192193
};
193194
let mut conn = None;
194195
let database_entries_stream =
195-
stream_query_results_with_conn(&sql_file, &req_param, &mut conn);
196+
stream_query_results_with_conn(&sql_file, &exec_ctx, &mut conn);
196197
let database_entries_stream = stop_at_first_error(database_entries_stream);
197198
let response_with_writer = build_response_header_and_stream(
198199
Arc::clone(&app_state),

0 commit comments

Comments
 (0)