Skip to content

Fix: glue with nested multi values#96

Merged
lucassalesn merged 5 commits into
masterfrom
glue-with-nested-multi-values
Mar 8, 2022
Merged

Fix: glue with nested multi values#96
lucassalesn merged 5 commits into
masterfrom
glue-with-nested-multi-values

Conversation

@lucassalesn

@lucassalesn lucassalesn commented Mar 3, 2022

Copy link
Copy Markdown
Contributor

Fixes #94

In this issue, we catch a bug when one of the arguments, except the last one in SQL.glue(args) has nested values.
Example:

const ids = [1, 2, 3]
const name = 'foo'
const moreIds = [4, 5, 6]
const inIds = SQL.glue(
  ids.map((id) => SQL`${id}`),
  ' , '
)
const inMoreIds = SQL.glue(
  moreIds.map((id) => SQL`${id}`),
  ' , '
)
const condition = SQL`tsd.id IN (${inIds})`
const anotherCondition = SQL`tsd.id IN (${inIds})`

//this works because the nested condition is the last one
// const filters = [
//   SQL`tsd.name = ${name}`,
//   condition,
// ]

// Error, result is SELECT tsd.* FROM data tsd WHERE tsd.id IN (? , ? , ?)?tsd.name = ?
const filters = [
  condition,
  SQL`tsd.name = ${name}`,
]

// Error, result is SELECT tsd.* FROM data tsd WHERE tsd.name = ? AND tsd.id IN (? , ? , ?) ?, ?, ?tsd.in (?)
// const filters = [
//   SQL`tsd.name = ${name}`,
//   condition,
//   anotherCondition
// ]

const sql = SQL`SELECT tsd.* FROM data tsd WHERE ${SQL.glue(filters, ' AND ')}`

the cause is in this piece of code

    for (let i = 0; i < pieces.length; i++) {
      const strings = Array.from(pieces[i].strings)
      if (typeof carryover === 'string') {
        strings[0] = carryover + separator + strings[0]
        carryover = null
      }

      if (strings.length > pieces[i].values.length) {
        carryover = strings.splice(-1)[0]
      }
      result.strings.push.apply(result.strings, strings)
      result.values.push.apply(result.values, pieces[i]._values)
    }

In the second if "strings.length > pieces[i].values.length" we check if strings.length if higher than pieces[I].values.length, and only if this condition is true we change the value of carryover variable, but we only add the separator if the variable carryover type is a string.

So every time that one of the pieces (except the last one because we don't need the separator in this case) doesn't match with the second "if" we will not have the addition of the separator.

@lucassalesn lucassalesn force-pushed the glue-with-nested-multi-values branch from 865eac1 to 35c5b6c Compare March 3, 2022 15:24
Comment thread SQL.test.js Outdated
Comment thread SQL.js Outdated
Comment thread SQL.js
Comment thread SQL.test.js
@lucassalesn lucassalesn requested a review from simoneb March 4, 2022 13:53

@ramonmulia ramonmulia left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lucassalesn lucassalesn merged commit 615946f into master Mar 8, 2022
@lucassalesn lucassalesn deleted the glue-with-nested-multi-values branch March 8, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: wrong character for IN condition

4 participants