Reuse repeated variable bindings for numbered placeholders

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## unreleased
 
+- Restored parameter reuse for repeated variables on SQL Server, PostgreSQL, and SQLite, fixing SQL Server and PostgreSQL queries that repeat a variable-dependent expression in `SELECT` and `GROUP BY`.
+
 ## v0.47.0 (2026-10-15)
 - Redesigned the official website homepage with live SQLPage examples, an interactive database sculpture, flowing section transitions, responsive layouts, and direct links to documentation and deployment guides.
 - For sites that store their pages in a database, checking for page updates no longer makes other pages wait when the database is busy. In a test, another page loaded in under 1 ms instead of about 250 ms.

src/webserver/database/sql.rs

@@ -681,6 +681,120 @@ mod tests {
         assert_eq!(query.bindings.len(), 1);
     }
 
+    #[test]
+    fn repeated_variables_follow_placeholder_style() {
+        use SupportedDatabase::{Mssql, MySql, Postgres, Sqlite};
+        for (db, kind, expected_sql, reuse) in [
+            (
+                Postgres,
+                AnyKind::Postgres,
+                "SELECT $1, $2, $1 FROM t",
+                true,
+            ),
+            (Mssql, AnyKind::Mssql, "SELECT @p1, @p2, @p1 FROM t", true),
+            (
+                Sqlite,
+                AnyKind::Sqlite,
+                "SELECT CAST(?1 AS TEXT), CAST(?2 AS TEXT), CAST(?1 AS TEXT) FROM t",
+                true,
+            ),
+            (MySql, AnyKind::MySql, "SELECT ?, ?, ? FROM t", false),
+            (Mssql, AnyKind::Odbc, "SELECT ?, ?, ? FROM t", false),
+            (
+                Postgres,
+                AnyKind::Odbc,
+                "SELECT CAST(? AS TEXT), CAST(? AS TEXT), CAST(? AS TEXT) FROM t",
+                false,
+            ),
+            (
+                Sqlite,
+                AnyKind::Odbc,
+                "SELECT CAST(? AS TEXT), CAST(? AS TEXT), CAST(? AS TEXT) FROM t",
+                false,
+            ),
+        ] {
+            let mut info = database(db);
+            info.kind = kind;
+            let dialect = dialect::parser_dialect(db);
+            let FileStatement::Query(Query {
+                body: QueryBody::Database(query),
+                ..
+            }) = parse_sql(&info, dialect.as_ref(), "select $b, $a, $b from t")
+                .unwrap()
+                .next()
+                .unwrap()
+            else {
+                panic!("expected database query for {db:?} via {kind:?}");
+            };
+            assert_eq!(query.sql, expected_sql, "{db:?} via {kind:?}");
+            let mut expected_bindings = vec![variable("b"), variable("a")];
+            if !reuse {
+                expected_bindings.push(variable("b"));
+            }
+            assert_eq!(
+                query.bindings.as_ref(),
+                expected_bindings,
+                "{db:?} via {kind:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn repeated_variables_preserve_lookup_source() {
+        let query = rewrite_database("select $b, :b, $b, :b from t");
+        assert_eq!(query.sql, "SELECT $1, $2, $1, $2 FROM t");
+        assert_eq!(
+            query.bindings.as_ref(),
+            [
+                variable("b"),
+                SqlPageExpr::Variable(VariableRef {
+                    name: "b".into(),
+                    source: VariableSource::SetOrForm
+                }),
+            ]
+        );
+    }
+
+    #[test]
+    fn mssql_group_by_reuses_variable() {
+        let FileStatement::Query(Query {
+            body: QueryBody::Database(query),
+            ..
+        }) = one_mssql(
+            "SELECT n % CAST($b AS int) AS bucket, COUNT(*) AS cnt FROM (VALUES (1),(2),(3),(4),(5),(6)) t(n) GROUP BY n % CAST($b AS int);",
+        )
+        else {
+            panic!("expected database query");
+        };
+        assert_eq!(
+            query.sql,
+            "SELECT n % CAST(@p1 AS INT) AS bucket, COUNT(*) AS cnt FROM (VALUES (1), (2), (3), (4), (5), (6)) t (n) GROUP BY n % CAST(@p1 AS INT);"
+        );
+        assert_eq!(query.bindings.as_ref(), [variable("b")]);
+    }
+
+    #[test]
+    fn reused_variables_preserve_function_binding_order() {
+        let query = rewrite_database(
+            "select $b from t where sqlpage.random_string(1) <> $b and sqlpage.random_string(1) <> $b",
+        );
+        assert_eq!(query.sql, "SELECT $1 FROM t WHERE $2 <> $1 AND $3 <> $1");
+        assert_eq!(
+            query.bindings.as_ref(),
+            [
+                variable("b"),
+                call(
+                    SqlPageFunctionName::random_string,
+                    [SqlPageExpr::Literal(1.into())]
+                ),
+                call(
+                    SqlPageFunctionName::random_string,
+                    [SqlPageExpr::Literal(1.into())]
+                ),
+            ]
+        );
+    }
+
     #[test]
     fn effectful_bindings_are_not_deduplicated() {
         let FileStatement::Query(Query {

src/webserver/database/sql/rewrite/database_lowering.rs

@@ -40,9 +40,21 @@ impl<'a> DatabaseLowerer<'a> {
     }
 
     fn add_binding(&mut self, value: StandaloneExpr) -> SqlExpr {
-        let sequence = self.bindings.len();
-        self.bindings.push(value);
-        let placeholder = match placeholder_style(self.database.kind) {
+        let style = placeholder_style(self.database.kind);
+        // Variable identity includes its lookup source. Calls may have side effects,
+        // and positional placeholders need a separate argument for each occurrence.
+        let existing = match (&value, style) {
+            (SqlPageExpr::Variable(_), PlaceholderStyle::Numbered { .. }) => {
+                self.bindings.iter().position(|binding| binding == &value)
+            }
+            _ => None,
+        };
+        let sequence = existing.unwrap_or_else(|| {
+            let sequence = self.bindings.len();
+            self.bindings.push(value);
+            sequence
+        });
+        let placeholder = match style {
             PlaceholderStyle::Numbered { prefix } => format!("{prefix}{}", sequence + 1),
             PlaceholderStyle::Positional { .. } => format!("${}", sequence + 1),
         };

tests/end-to-end/official-site.spec.ts

@@ -20,14 +20,22 @@ test("Open documentation", async ({ page }) => {
   }
 });
 
-test("chart", async ({ page }) => {
+async function openChartDocumentation(page: Page) {
   await page.goto("/documentation.sql?component=chart#component");
-  await expect(page.getByText("Loading...")).not.toBeVisible();
+  // This page initializes every documented chart, which can take several seconds.
+  await expect(page.locator('[data-pre-init="chart"]')).toHaveCount(0, {
+    timeout: 15_000,
+  });
+}
+
+test("chart", async ({ page }) => {
+  await openChartDocumentation(page);
+  await expect(page.getByText("Loading...")).toHaveCount(0);
   await expect(page.locator(".apexcharts-canvas").first()).toBeVisible();
 });
 
 test("chart supports hiding legend", async ({ page }) => {
-  await page.goto("/documentation.sql?component=chart#component");
+  await openChartDocumentation(page);
 
   const expensesChart = page.locator(".card", {
     has: page.getByRole("heading", { name: "Expenses" }),
@@ -55,7 +63,7 @@ const drawnPoints = (card: Locator, series: string) =>
 test("stacked chart draws every series at every x of the chart", async ({
   page,
 }) => {
-  await page.goto("/documentation.sql?component=chart#component");
+  await openChartDocumentation(page);
   const powerChart = chartCard(page, "Power draw");
   await expect(powerChart.locator(".apexcharts-canvas")).toBeVisible();
 
@@ -69,7 +77,7 @@ test("stacked chart draws every series at every x of the chart", async ({
 test("stacked chart raises a series only where it has a value", async ({
   page,
 }) => {
-  await page.goto("/documentation.sql?component=chart#component");
+  await openChartDocumentation(page);
   const powerChart = chartCard(page, "Power draw");
   await expect(powerChart.locator(".apexcharts-canvas")).toBeVisible();
 
@@ -83,7 +91,7 @@ test("stacked chart raises a series only where it has a value", async ({
 test("chart draws a yline as a line and a yline_end as a band", async ({
   page,
 }) => {
-  await page.goto("/documentation.sql?component=chart#component");
+  await openChartDocumentation(page);
 
   const temperature = page.locator(".card", {
     has: page.getByRole("heading", { name: "CPU temperature" }),
@@ -103,7 +111,7 @@ test("chart draws a yline as a line and a yline_end as a band", async ({
 test("chart draws an xline as a line and an xline_end as a band", async ({
   page,
 }) => {
-  await page.goto("/documentation.sql?component=chart#component");
+  await openChartDocumentation(page);
 
   const latency = page.locator(".card", {
     has: page.getByRole("heading", { name: "Request latency" }),
@@ -123,7 +131,7 @@ test("chart draws an xline as a line and an xline_end as a band", async ({
 test("horizontal chart draws a yline down it and an xline across it", async ({
   page,
 }) => {
-  await page.goto("/documentation.sql?component=chart#component");
+  await openChartDocumentation(page);
 
   const disks = page.locator(".card", {
     has: page.getByRole("heading", { name: "Disk usage" }),

tests/sql_test_files/data/database-specific/mssql/variable_group_by.sql

@@ -0,0 +1,11 @@
+-- Repeated variables must use the same parameter in SELECT and GROUP BY.
+-- https://github.com/sqlpage/SQLPage/issues/1558
+SET b = '3';
+
+SELECT '0:2,1:2,2:2' AS expected,
+    STRING_AGG(CONCAT(bucket, ':', cnt), ',') WITHIN GROUP (ORDER BY bucket) AS actual
+FROM (
+    SELECT n % CAST($b AS int) AS bucket, COUNT(*) AS cnt
+    FROM (VALUES (1), (2), (3), (4), (5), (6)) t(n)
+    GROUP BY n % CAST($b AS int)
+) buckets;

tests/sql_test_files/data/parameter_bound_twice_in_a_projection.sql

@@ -7,3 +7,14 @@ insert into parameter_bound_twice_t (id) values (1);
 select 'It works !' as expected,
     case when $x is not null and $x = '1' then 'It works !' else 'fail' end as actual
 from parameter_bound_twice_t;
+
+-- Reusing a parameter is scoped to one statement, including SET to NULL.
+set x = NULL;
+select 'It works !' as expected,
+    case when $x is null and $x is null then 'It works !' else 'fail' end as actual
+from parameter_bound_twice_t;
+
+set x = '2';
+select 'It works !' as expected,
+    case when $x = '2' and $x = '2' then 'It works !' else 'fail' end as actual
+from parameter_bound_twice_t;