Support direct interpolation

[lorefnon]

It is great that the *FragmentExpression.sql functions default to converting interpolated values in tagged templates to sql parameters. However, some cases are simplified by enabling direct interpolation.

I understand that this enables sql injection if injected value is not sanitized, but I think having the option is good similar to how most template engines sanitize HTML by default but provide a construct for raw injection if needed.

One common convention in many template based libraries (eg leafac/sqlite) is to use $${...} for raw sql interpolation.

This would enable usages like:

const duration = 10;
conn
    .update(tJoinCode)
    .set({ isActive: false })
    .where(tJoinCode.deactivatedAt.lessThan(
        conn
            .fragmentWithType('localDateTime', 'required')
            .sql`now() - interval '$${duration} minute'`)
    ))

So basically in this proposal the sql function will look at the last character before the interpolation and will use direct interpolation if its $.

[juanluispaz]

Hi

as far I know that is possible to do with regular parameters; are you finding an issue or limitation of the DB here? The only reason why you want this right now is the cosmetic of the generated query; but, as usually here, I can be surprised.

Be aware, when you inline values as SQL, you destroy the performance of the caching system in your database.

Let me know why do you want this?

I can try to provide some methods here like:

• intValueAsInlineSql(number): InstValueSource
• doubleValueAsInlineSql(number): DoubleValueSource
• unsafeInlineSqlFragmentWithType(...).inlineSql(string): ValueSource

I don't want to use the $$ notation because that will require parse the template string; and I will really prefer to avoid; and, as well, I cannot make it TypeSafe in TS, additionally, I will want to make clear that operation is unsafe.

This is related to #83

[lorefnon]

Yes, the difference is mostly cosmetic. I understand that everything can be expressed in terms of params eg. in case of above I can write now() - (${conn.const(duration, 'int')} || ' minute')::interval.

My particular use case is I have a collection of infrequently run reporting queries which currently use (liquid) templates. I am converting to use ts-sql-query (to take advantage of the excellent return type inference) but rewriting them to be parameterizable needs substantial refactoring. I am not very concerned about the parse time performance because they don't need to run often and they run on replica.

It is ok if you don't consider this to be within the scope of this library. I don't understand the concern about type safety though because my understanding is that when we use rawFragment anyways the developer is responsible for ensuring the right type.

[juanluispaz]

Hi,

I always try to understand what the people are trying to do; in that way, I can try to provide good abstractions; I'm pleased to add functionalities to ts-sql-query that are useful for each one in their own case; and each time I receive feedback I got surprised.

now() - ('${conn.const(duration, 'int')} || ' minute')::interval

I will not be happy with this solution because it is not the same; in this way, you will need to add extra processing to the database because of a stupid limitation of the library. I tested using the parameter syntax now() - ? minute (my error, it needs to be quoted), but, unfortunately, PostgreSQL doesn't admit a parameter in that position; now I start understanding your motivation behind this.

my understanding is that when we use rawFragment anyways the developer is responsible for ensuring the right type.

This is true, but even using your own rawFragment right now (if you don't bypass TS types), there is no possibility to add SQL injections; I have no issue adding the additional functions to the library, and as well I see some possible advantages to have that functions in our projects.

As well, I already identified that I need to improve the fragment creation to allow build patterns where the fragment needs to be built dynamically (like if, for, etc.); that appears in one of our projects, but, in the end, we didn't need it; but I saw the limitation of the current implementation.

My initial thought on this is to create a general function in a similar way this.fragmentWithType(...).sql...` works; something like this:

• unsafeInlineSqlFragmentWithType(...).inlineSql(string): ValueSource

Another option that came to my mind reading your comment will be to have a general function that doesn't provide information of the type, but you must always use as a parameter in a SQL fragment to define the type:

• unsafeInlineSql(string): InlineSql

And then use it directly in your SQL fragment as:

this.fragmentWithType('int', 'required').sql`${this.unsafeInlineSql('my SQL code')}`

Additionally, I can add two shortcuts functions already validated to deal with number values (not sure about naming):

• inlineIntValue(number): IntValueSource
• inlineDoubleValue(number): DoubleValueSource

Maybe it could be useful to add the other additional types as well over time; I will need to think about how to do it in all supported databases; for example, dates in Sqlite are a headache, or boolean doesn't exist in SqlServer or Oracle. In the worst case, I can always add features that target one specific database.

With these shortcut functions, your code will look like this:

const duration = 10;
conn
    .update(tJoinCode)
    .set({ isActive: false })
    .where(tJoinCode.deactivatedAt.lessThan(
        conn
            .fragmentWithType('localDateTime', 'required')
            .sql`now() - interval '${conn.inlineIntValue(duration)} minute'`)
    ))

I'm trying to figure out how I must follow here; for this reason, I want to have your feedback.

Let me know your thoughts and what will make the tasks you are trying to archive easier.

Side note: in our projects, we prefer to use the reusable fragments because this often needs to be reused several times.

[lorefnon]

Thank you for the detailed reply, Juan. I think the unsafeInlineSql fn would be a great addition. The shortcut functions are also nice to have to prevent sql injection but I can survive without them.

Also thanks for pointing to the reusable fragments, I should use that more often.