fix: keep SQL templates self-contained, use .replace() for filter placeholders

Address review feedback on PR #432:

1. Restore GROUP BY, ORDER BY, and LIMIT clauses to SQL template constants
   so each template is a complete, readable query. Only the optional WHERE
   filter placeholders ({warehouse_filter}, {user_filter}, etc.) are
   replaced at the call site via .replace() — safe because they are
   hardcoded SQL fragments, not user input.

2. Fix incorrect comment in query-history.ts claiming Postgres driver does
   not support parameterized binds. The actual limitation is that our
   connector wrapper (packages/drivers/src/postgres.ts) does not yet pass
   the _binds parameter through to pg. LIMIT is still rendered inline via
   Math.floor(Number(limit)) until the driver is updated.

3. Double-newline issue from empty filters is resolved by the .replace()
   approach — replacing an empty placeholder leaves at most a blank line,
   which SQL ignores.

Files: credit-analyzer.ts, query-history.ts, role-access.ts, tags.ts

Co-Authored-By: Vijay Yadav <vijay@studentsucceed.com>

Author: VJ-yadav

packages/opencode/src/altimate/native/finops/credit-analyzer.ts

@@ -27,6 +27,10 @@ SELECT
     AVG(credits_used) as avg_credits_per_query
 FROM SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_METERING_HISTORY
 WHERE start_time >= DATEADD('day', ?, CURRENT_TIMESTAMP())
+{warehouse_filter}
+GROUP BY warehouse_name, DATE_TRUNC('day', start_time)
+ORDER BY usage_date DESC, credits_used DESC
+LIMIT ?
 `
 
 const SNOWFLAKE_CREDIT_SUMMARY_SQL = `
@@ -194,8 +198,7 @@ function buildCreditUsageSql(
     const whF = warehouseFilter ? (binds.push(warehouseFilter), "AND warehouse_name = ?") : ""
     binds.push(limit)
     return {
-      sql: SNOWFLAKE_CREDIT_USAGE_SQL +
-        `${whF}\nGROUP BY warehouse_name, DATE_TRUNC('day', start_time)\nORDER BY usage_date DESC, credits_used DESC\nLIMIT ?\n`,
+      sql: SNOWFLAKE_CREDIT_USAGE_SQL.replace("{warehouse_filter}", whF),
       binds,
     }
   }

packages/opencode/src/altimate/native/finops/query-history.ts

@@ -33,6 +33,10 @@ SELECT
     credits_used_cloud_services
 FROM SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY
 WHERE start_time >= DATEADD('day', ?, CURRENT_TIMESTAMP())
+{user_filter}
+{warehouse_filter}
+ORDER BY start_time DESC
+LIMIT ?
 `
 
 const POSTGRES_HISTORY_SQL = `
@@ -55,6 +59,7 @@ SELECT
     calls as execution_count
 FROM pg_stat_statements
 ORDER BY total_exec_time DESC
+LIMIT {limit}
 `
 
 const BIGQUERY_HISTORY_SQL = `
@@ -122,14 +127,20 @@ function buildHistoryQuery(
     const whF = warehouseFilter ? (binds.push(warehouseFilter), "AND warehouse_name = ?") : ""
     binds.push(limit)
     return {
-      sql: SNOWFLAKE_HISTORY_SQL +
-        `${userF}\n${whF}\nORDER BY start_time DESC\nLIMIT ?\n`,
+      sql: SNOWFLAKE_HISTORY_SQL
+        .replace("{user_filter}", userF)
+        .replace("{warehouse_filter}", whF),
       binds,
     }
   }
   if (whType === "postgres" || whType === "postgresql") {
-    // Postgres driver does not support parameterized binds — render LIMIT inline
-    return { sql: POSTGRES_HISTORY_SQL + `LIMIT ${Math.floor(Number(limit))}\n`, binds: [] }
+    // The Postgres connector (packages/drivers/src/postgres.ts) does not yet
+    // pass binds through to pg — its execute() ignores the _binds parameter.
+    // Render LIMIT inline with Math.floor to prevent injection.
+    return {
+      sql: POSTGRES_HISTORY_SQL.replace("{limit}", String(Math.floor(Number(limit)))),
+      binds: [],
+    }
   }
   if (whType === "bigquery") {
     return { sql: BIGQUERY_HISTORY_SQL, binds: [days, limit] }

packages/opencode/src/altimate/native/finops/role-access.ts

@@ -29,6 +29,11 @@ SELECT
     created_on
 FROM SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES
 WHERE 1=1
+{role_filter}
+{object_filter}
+AND deleted_on IS NULL
+ORDER BY granted_on, name
+LIMIT ?
 `
 
 const SNOWFLAKE_ROLE_HIERARCHY_SQL = `
@@ -52,6 +57,9 @@ SELECT
     created_on
 FROM SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_USERS
 WHERE deleted_on IS NULL
+{user_filter}
+ORDER BY grantee_name, role
+LIMIT ?
 `
 
 // ---------------------------------------------------------------------------
@@ -69,6 +77,9 @@ SELECT
     '' as created_on
 FROM \`region-US.INFORMATION_SCHEMA.OBJECT_PRIVILEGES\`
 WHERE 1=1
+{grantee_filter}
+ORDER BY object_type, object_name
+LIMIT ?
 `
 
 // ---------------------------------------------------------------------------
@@ -86,6 +97,9 @@ SELECT
     '' as created_on
 FROM system.information_schema.table_privileges
 WHERE 1=1
+{grantee_filter}
+ORDER BY table_name
+LIMIT ?
 `
 
 // ---------------------------------------------------------------------------
@@ -117,8 +131,9 @@ function buildGrantsSql(
     const objF = objectName ? (binds.push(objectName), "AND name = ?") : ""
     binds.push(limit)
     return {
-      sql: SNOWFLAKE_GRANTS_ON_SQL +
-        `${roleF}\n${objF}\nAND deleted_on IS NULL\nORDER BY granted_on, name\nLIMIT ?\n`,
+      sql: SNOWFLAKE_GRANTS_ON_SQL
+        .replace("{role_filter}", roleF)
+        .replace("{object_filter}", objF),
       binds,
     }
   }
@@ -127,8 +142,7 @@ function buildGrantsSql(
     const granteeF = role ? (binds.push(role), "AND grantee = ?") : ""
     binds.push(limit)
     return {
-      sql: BIGQUERY_GRANTS_SQL +
-        `${granteeF}\nORDER BY object_type, object_name\nLIMIT ?\n`,
+      sql: BIGQUERY_GRANTS_SQL.replace("{grantee_filter}", granteeF),
       binds,
     }
   }
@@ -137,8 +151,7 @@ function buildGrantsSql(
     const granteeF = role ? (binds.push(role), "AND grantee = ?") : ""
     binds.push(limit)
     return {
-      sql: DATABRICKS_GRANTS_SQL +
-        `${granteeF}\nORDER BY table_name\nLIMIT ?\n`,
+      sql: DATABRICKS_GRANTS_SQL.replace("{grantee_filter}", granteeF),
       binds,
     }
   }
@@ -250,8 +263,7 @@ export async function queryUserRoles(params: UserRolesParams): Promise<UserRoles
     const binds: any[] = []
     const userF = params.user ? (binds.push(params.user), "AND grantee_name = ?") : ""
     binds.push(limit)
-    const sql = SNOWFLAKE_USER_ROLES_SQL +
-      `${userF}\nORDER BY grantee_name, role\nLIMIT ?\n`
+    const sql = SNOWFLAKE_USER_ROLES_SQL.replace("{user_filter}", userF)
 
     const result = await connector.execute(sql, limit, binds)
     const assignments = rowsToRecords(result)

packages/opencode/src/altimate/native/schema/tags.ts

@@ -26,6 +26,9 @@ SELECT
     column_name,
     domain as object_type
 FROM TABLE(INFORMATION_SCHEMA.TAG_REFERENCES_ALL_COLUMNS(?, 'TABLE'))
+{tag_filter}
+ORDER BY tag_name, object_name
+LIMIT ?
 `
 
 const SNOWFLAKE_TAG_LIST_SQL = `
@@ -79,8 +82,7 @@ export async function getTags(params: TagsGetParams): Promise<TagsGetResult> {
         ? (binds.push(params.tag_name), "WHERE tag_name = ?")
         : ""
       binds.push(limit)
-      sql = SNOWFLAKE_TAG_REFERENCES_SQL +
-        `${tagFilter}\nORDER BY tag_name, object_name\nLIMIT ?\n`
+      sql = SNOWFLAKE_TAG_REFERENCES_SQL.replace("{tag_filter}", tagFilter)
     } else {
       // Fall back to listing all tags
       binds = [limit]

packages/opencode/test/altimate/schema-finops-dbt.test.ts

@@ -141,7 +141,7 @@ describe("FinOps: SQL template generation", () => {
     test("builds PostgreSQL history SQL", () => {
       const built = HistoryTemplates.buildHistoryQuery("postgres", 7, 50)
       expect(built?.sql).toContain("pg_stat_statements")
-      // Postgres driver does not support binds — LIMIT is rendered inline
+      // Postgres connector does not yet pass binds — LIMIT rendered inline
       expect(built?.sql).toContain("LIMIT 50")
       expect(built?.binds).toEqual([])
     })