Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

supporting "where in" with multiple columns #367

Closed
tgriesser opened this issue Mar 18, 2023 · 5 comments · Fixed by #611
Closed

supporting "where in" with multiple columns #367

tgriesser opened this issue Mar 18, 2023 · 5 comments · Fixed by #611
Labels
api Related to library's API enhancement New feature or request mysql Related to MySQL postgres Related to PostgreSQL sqlite Related to sqlite

Comments

@tgriesser
Copy link
Contributor

Looking for a way to support a multi-column where in clause like this:

const cols = ['colA', 'colB']
const values = [[1, 2], [3, 4]]

qb.where(cols, 'in', values)

Which gives a TypeError: exp.toOperationNode is not a function

This seems to work instead:

const whereCol = Array.isArray(whereInCol)
  ? sql`(${sql.join(whereInCol.map((c) => sql.ref(c)))})`
  : whereInCol;
const whereVals = Array.isArray(whereInCol)
  ? sql`(${sql.join(whereInIds.map((ids) => sql`(${sql.join(ids)})`))})`
  : whereInIds;

qb = qb.where(whereCol, "in", whereVals);

But wanted to see if there was either a simpler approach I was missing that would be better here, or if this is something that could added to be be handled by kysely if not.

@igalklebanov igalklebanov added api Related to library's API mysql Related to MySQL postgres Related to PostgreSQL enhancement New feature or request sqlite Related to sqlite labels Mar 18, 2023
@igalklebanov
Copy link
Member

igalklebanov commented Mar 18, 2023

Hey 👋

Your implementation works, but it is not type-safe.

Personally, I support having this functionality in Kysely's core, as it is supported by all built-in dialects.

For now, something like this should do the trick.

@koskimas
Copy link
Member

koskimas commented Mar 19, 2023

Since we are about to add a more comprehensive expression builder, at least something like this could be easily supported in a type-safe way:

.where(({ bin, ref, tuple }) => bin(
  tuple(ref('colA'), ref('colB')),
  'in', 
  [tuple(1, 2),  tuple(3, 4)]
)) 

I'm not loving the readability though.

@igalklebanov igalklebanov added blocked Blocked waiting for another feature and removed blocked Blocked waiting for another feature labels Mar 22, 2023
@igalklebanov
Copy link
Member

I like the idea of eb.tuple.
We should provide a helper, roughly like this:

function inTuples<DB, TB extends keyof DB>(items: any[] /*should be records with all possible columns from context as keys, and values ValueExpression?*/) {
  return ({ bin, ref, tuple }: ExpressionBuilder<DB, TB>) => {
	const keys = resolveKeys(items)

    return bin(
      tuple(...keys.map((key) => ref(key))),
      'in',
      items.map((item => tuple(...keys.map((key) => isUndefined(item[key]) ? null : item[key])))),
    )
  }
}

Pretty close to how we handle InsertQueryBuilder.values().

And usage:

.where(inTuples(items))

@koskimas
Copy link
Member

@tgriesser In the next version you can say:

.where(({ eb, refTuple, tuple }) => eb(
  refTuple('colA', 'colB'),
  'in', 
  [tuple(1, 2),  tuple(3, 4)]
))

and

.where(({ eb, refTuple, selectFrom }) => eb(
  refTuple('colA', 'colB'),
  'in', 
  selectFrom('some_table')
    .select(['colC', 'colD'])
    .$asTuple('colC', 'colD') // <-- Necessary evil because typescript
))

@vladshcherbin
Copy link
Contributor

@koskimas any insight on release date of the next version?

I'm also using where in in codebase and this addition is awesome, will help simplify writing queries a lot 😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to library's API enhancement New feature or request mysql Related to MySQL postgres Related to PostgreSQL sqlite Related to sqlite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants