Behaviour of parseFilter("on" | "where" | "having") seems restrictive

[ggallovalle]

Hi I am loving this project and for a little bit of context I am writing type safe functions which are wrappers around sql functions without the use of sql``string literal, but the restriction on parseFilter with just one argument are making it so that I can not use an Expression expressed as a tree other than a Raw node.

// src/parser/binary-operation-parser.ts

// 144
function parseFilter(type: FilterExpressionType, args: any[]): OperationNode {
  if (args.length === 3) {
    return parseValueComparison(args[0], args[1], args[2])
  }

  if (args.length === 1) {
    return parseOneArgFilterExpression(type, args[0])
  }

  throw createFilterExpressionError(type, args)
}

// 190
function parseOneArgFilterExpression(
  type: FilterExpressionType,
  arg: any
): OperationNode {
  if (isFunction(arg)) {
    return CALLBACK_PARSERS[type](arg)
  } else if (isOperationNodeSource(arg)) {
    const node = arg.toOperationNode()

    if (RawNode.is(node)) {
      return node
    }
  }

  throw createFilterExpressionError(type, arg)
}

For example I can do something like this with my wrapper

const expressionBoolean = string("column").trim().lower().isEqualTo("john").compile(db)

// expressonBoolean is:
{
  parameters: [ 'john' ],
  sql: 'lower(btrim("column")) = $1',
  query: {
    kind: 'BinaryOperationNode',
    leftOperand: {
      kind: 'FunctionNode',
      func: 'lower',
      arguments: [
        {
          kind: 'FunctionNode',
          func: 'btrim',
          arguments: [
            {
              kind: 'ColumnNode',
              column: { kind: 'IdentifierNode', name: 'column' }
            }
          ]
        }
      ]
    },
    operator: { kind: 'OperatorNode', operator: '=' },
    rightOperand: { kind: 'ValueNode', value: 'john' }
  }
}

which produces the following Query

lower(btrim("column")) = $1

But even though that is a legitimate piece of code that could go into a where or an on clause it throws an error when used as a one argument to functions where and on so I have to do some hacks to make it work.

// throws error even thought the generated sql would be valid
// select * from table where lower(btrim("column")) = $1
db.selectFrom("table")
    .selectAll()
    .where(expressionBoolean)

// hacks
db.selectFrom("table")
    .selectAll()
    .where(eb => expressionBoolean)

// OR
db.selectFrom("table")
    .selectAll()
    .where(sql`${expressionBoolean}`)

// throws error even thought the generated sql would be valid
// select * from table left join other_table on lower(btrim("column")) = $1
db.selectFrom("table")
    .selectAll()
    .leftJoin("other_table", jb => jb.on(expressionBoolean))

// hack
db.selectFrom("table")
    .selectAll()
    .leftJoin("other_table", jb => jb.on((oeb) => expressionBoolean))

// OR
db.selectFrom("table")
    .selectAll()
    .leftJoin("other_table", jb => jb.on(sql`{expressionBoolean}`)

In am requesting that parseFilter to be more lax and not check RawNode.is, just let it be when is an isOperationNodeSource. Or if is too much of a breaking change make it so that a node can inform is raw without actually being kind === "RawNode in the sense that a "RawNode" is a way of telling kysely QueryCompiler that "you know what you are doing" .

Or any other suggestion that you all have.

[igalklebanov]

Hey 👋

Kudos for digging into the codebase and figuring things out! 👏

There's actually a much easier way to achieve what you're looking for:

const rows = await kysely
  .selectFrom("user")
  .where(
    ({ cmpr, fn }) => cmpr(fn('lower', [fn('btrim', ['first_name'])]), '=', 'Johnny')
  )
  .selectAll()
  .execute()

Playground Link

We won't change library internals due to external requirements. There has to be a very good reason for that - e.g. it's a serious blocker for some 3rd party dialect.

[koskimas]

This has been fixed a while ago.