Should it reuse values?

[Janpot]

SQL`INSERT into table(id, text) VALUES (${1}, ${'x'}), (${2}, ${'x'}), (${3}, ${'x'}), (${4}, ${'x'}), (${5}, ${'x'})`

currently results into

{
  text: 'INSERT into table(id, text) VALUES ($1, $2), ($3, $4), ($5, $6), ($7, $8), ($9, $10)',
  values: [1, 'x', 2, 'x', 3, 'x', 4, 'x', 5, 'x']
}

Could this be optimized to return

{
  text: 'INSERT into table(id, text) VALUES ($1, $2), ($3, $2), ($4, $2), ($5, $2), ($6, $2)',
  values: [1, 'x', 2, 3, 4, 5]
}

?

[XeCycle]

I did consider this at the beginning; this would of course reduce amount of data transferred on the wire, but there's also time added on comparing values. Comparing may be O(N^2), so a detailed benchmark is needed. I have not yet got enough time to do that.

[Janpot]

Can't you use a Map to store values as you interpolate them? That should take care of the complexity right?

[XeCycle]

I plan to add a version that reuses values side-by-side with the current version. If it turned out faster in most use cases I'll consider changing the default, but both versions will be provided.

[XeCycle]

A quick-and-dirty implementation is now on branch dedup. Please check how it works out; I will continue to improve it, especially on how it should interoperate with the non-reusing version.

[blakeembrey]

I recently went and built this feature in one of my own modules, but decided it was a premature optimisation since I wanted to support mysql also (can't dedupe with the mysql module unfortunately). If you'd like to implement it simpler, you can just use Map to keep track of found values:

export type Value = string | number | boolean | object
export type RawValue = Value | Sql

export class Sql {

  text: string
  values: Value[]

  constructor (
    protected rawStrings: ReadonlyArray<string>,
    protected rawValues: ReadonlyArray<RawValue>
  ) {
    this.values = []
    this.text = this.getText(this.values, new Map())
  }

  getText (values: Value[], cache: Map<Value, number>): string {
    return this.rawStrings.reduce((text, part, index) => {
      const child = this.rawValues[index - 1]

      if (child instanceof Sql) return `${text}${child.getText(values, cache)}${part}`

      if (!cache.has(child)) cache.set(child, values.push(child))

      return `${text}$${cache.get(child)}${part}`
    })
  }

}

[pauldraper]

Fixed by #8