Composable SQL templates

[brandonchinn178]

If I have a WHERE clause to conditionally include in the query, it would be nice to do something like

const whereClause = condition ? sql`name = ${name}` : sql`TRUE`
return db.query(sql`SELECT * FROM "foo" WHERE ${whereClause}`)

or even

const whereClause = condition ? sql`WHERE name = ${name}` : ''
return db.query(sql`SELECT * FROM "foo" ${whereClause}`)

Thoughts?

[andywer]

Hey @brandonchinn178. Absolutely – thanks for sharing!

I thought about it before, but didn't create an issue yet. Just one addition to the second code snippet:

Passing an empty string into the query (whereClause = "") is problematic, since squid will escape values automatically and there would be no way to tell that the empty string is supposed to be a query fragment and not a value.

The most forward solution would be to use sql.raw(): whereClause = sql.raw("")

[brandonchinn178]

hm so i implemented this and tried using it, but postguard isn't happy, since we now have sql templates that aren't valid SQL queries by themselves. One solution is to provide hints to postguard, but I can't even imagine what that would look like.

I actually think this goes against the philosophy of squid + postguard, since the whole point is to manually write out the full SQL query instead of building it up. As an example, postguard doesn't even like

const query = sql`SELECT * FROM user ${sql.raw('WHERE')} id = 1`

which makes sense (and seems rather difficult to do), since trying to account for composable templates in postguard means needing to evaluate each branch and check that the final sql result is valid.

Thoughts?

[andywer]

Ohh, man, I didn't think about postguard when I wrote that, tbh.

Makes perfect sense, but I don't have an immediate approach to solve that at hand either…

[brandonchinn178]

@andywer In thinking about how I want this to work in my project, what do you think about the following:

sql`
  SELECT * FROM "song"
  WHERE
    ${sql.and(
      query && sql.fmt`song.title ILIKE ${'%' + query + '%'}`,
      bpm !== null ? sql.fmt`song.bpm = ${bpm}` : null,
      timeSig && sql.fmt`
        song.time_signature_top = ${timeSig[0]} AND
        song.time_signature_bottom = ${timeSig[1]}
      `
    )}
`

There are a couple new features being suggested here. Keeping postguard in mind, I'm thinking of these semantics:

1. Seeing sql.fmt should act the same way as sql.raw at runtime, but in postguard, would interpolate the SQL instead of replacing with $n as with sql.raw. e.g.

   sql`SELECT * FROM foo WHERE ${sql.fmt`bar = ${1}`}`

   is exactly equivalent to

   sql`SELECT * FROM foo WHERE bar = ${1}`

   at runtime and postguard.

2. sql.and is a helper that just stitches everything together with ANDs, ignoring any null or undefined values. If there are no null/undefined values, returns TRUE. I'm thinking that only sql.fmt, &&, ||, and ternary operators are allowed in sql.and, if one is using postguard.

[andywer]

Thanks for sharing! A few thoughts of mine:

1. What do we need sql.fmt for – what would be the reason to not just use the sql tag? In my head your example would look like this:

   sql`SELECT * FROM foo WHERE ${sql`bar = ${1}`}`

   is exactly equivalent to

   sql`SELECT * FROM foo WHERE bar = ${1}`

   We would just interpolate those query objects that the sql tag function returns.

2. I am a bit concerned that with something like sql.and() we would actually start diverting towards a query builder instead of focussing on the "it's really just SQL" approach.

   I might be missing something, but for me looks like the only benefit of using it would be to be able to use null values in there instead of sql.raw("TRUE"). In that case a convenience helper for the latter might be the better option.

[brandonchinn178]

1. Hm I was thinking that it would be easier for postguard if it were a separate function. Would it be possible to "remember" that we're currently in a sql template?

2. Yeah, sql.and() definitely steers more towards a query builder approach, although the spread functions currently kinda subvert that. Do you see a difference between the spread functions and this function?

   Either way, I'm down for any solution that doesn't involve this. The problem is trying to avoid making postguard exponential-time in number of branches in the template. One alternative I was thinking of is doing

   sql`
     SELECT * FROM "song"
     WHERE
       ${sql.if(query, sql`song.title ILIKE %{'%' + query + '%'}`)}
       AND ${sql.if(bpm, sql`song.bpm = ${bpm}`)}
       AND ${sql.if(timeSig, sql`
         song.time_signature_top = ${timeSig[0]} AND
         song.time_signature_bottom = ${timeSig[1]}
       `)}
   `

   The problem with this is that sql.if doesn't generalize without running into the exponential problem. It works in an AND chain because postguard can just assume all of them are true. But how do we prevent people from using sql.if outside the AND chain? And what if you need an else branch (I guess we can handle that if we get there, but still)?

[andywer]

1. Postguard walks the AST, so we can handle all kinds of structures and also use information from parent expressions. So I think it really just comes down to a new expression that does the same vs. nesting the existing expression. Both should be easy to handle with postguard ;)

2. Yeah, sql.if() is the same thing in a different dress, essentially. Now that I think about it… Why not do it like that:

   When we find an interpolation in the sql-tagged template string…

   • if it's another sql-tagged template, then statically interpolate it into the current query
   • if it's some sql.*() helper call, then apply our existing logic as always
   • if it's a ternary expression, duplicate our current query, interpolating both copies of the query with one of the ternary's result expressions, according to these rules here
   • if it's any other kind of expression, assume it is a value (maybe allow overriding this behavior via a /* postguard:interpolate */ comment or similar?)

   Biggest issue here is that I have not touched the postguard code in a long time and don't really have time for it right now either and it feels like quite a bit of work…

[brandonchinn178]

if it's a ternary expression, duplicate our current query, interpolating both copies of the query with one of the ternary's result expressions, according to these rules here

Yeah, we could do that, but then we hit the problem I mentioned earlier: The problem is trying to avoid making postguard exponential-time in number of branches in the template.

The query I put above has 3 branches, which would generate 8 queries that postguard would need to analyze, when in fact, postguard really only needs to analyze 1 query (all of the branches being interpolated) because the three branches are all independent of each other. I guess we could check if one side is null and if so, ignore that case.