Skip to content

doc: add dynamic filtering examples #1128

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

doc: add dynamic filtering examples #1128

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f06e6fc
doc: add dynamic filtering examples
JakobJingleheimer Oct 13, 2025
8f8a1d7
fixup!: apply suggestions from code review
JakobJingleheimer Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,37 @@ select * from users
select * from users where user_id = $1
```

Use a combination of `sql()` and ` sql`` ` to access fields in `json` or `jsonb`:

```js
const filter = someCondition
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I guess the condition is not required to illustrate this method of accessing a property in JSONB, and distracts from the example
  2. Maybe you can try inlining the SQL in the interpolation, rather than creating another variable - this would probably also make it easier to follow

Copy link
Author

Choose a reason for hiding this comment

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

I kept it separated because that is likely how it would be written IRL, and because this library is so finicky with string templates and what utils are needed when (which is literally the problem necessitating this example).

Perhaps a better way to write it would be

const field = someCondition
  ? 'notifications'
  : '2FA'

await sql`
  select * from users
    where ${sql`${sql(field)}->>'email' = TRUE`};
`

Copy link
Contributor

@karlhorky karlhorky Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What my point is: the ternary is not needed for dynamic field access and introduces some business logic in the example which is only a distraction

But maybe keeping 2 variables would be illustrative, to show that both can be dynamic:

const field = 'notifications';
const subField = 'email';

await sql`
  select * from users
    where ${sql`${sql(field)}->>${sql(subField)} = TRUE`};
`

Copy link
Author

Choose a reason for hiding this comment

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

I think that sql(subField) is actually wrong (I think it needs to be sql`subField`), but otherwise, sure, that looks fine to me :)

Copy link
Contributor

@karlhorky karlhorky Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, try it out to confirm that it works - I thought that would be the syntax but maybe I'm wrong.

? sql`${sql('notifications')}->>'email' = TRUE`
: sql`${sql('2FA')}->>'email' = TRUE`;

await sql`
select * from users
where ${filter};
`

// Which results in:
select * from users where "notifications"->>'email' = TRUE;
// OR
select * from users where "2FA"->>'email' = TRUE;
```

To dynamically combine multiple conditions, use [Array `reduce`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce); [Array `join`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/join) does not work because its output is a string (an interpolated value) instead of a `Fragment`:
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to exhaustively list the things that don't work

Suggested change
To dynamically combine multiple conditions, use [Array `reduce`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce); [Array `join`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/join) does not work because its output is a string (an interpolated value) instead of a `Fragment`:
To dynamically combine multiple conditions, use [`Array.prototype.reduce()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce):

Copy link
Author

Choose a reason for hiding this comment

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

I disagree since this is what almost everyone will try, and it mysteriously fails despite looking perfectly fine in the output from postgres.js's debug. That nonsense cost me 1.5 days of wtf, and you yourself did not realise it was the problem 😜

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, others may try something else - we won't be able to cover all possible incorrect cases / methods, and it blows up the size of the docs.

Copy link
Author

Choose a reason for hiding this comment

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

They may well try something else. This will catch the vast majority and also, most importantly, explains why join doesn't work and reduce does.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think either of us can decide what the majority would be - I disagree with having these notes in the docs

Copy link
Owner

@porsager porsager Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks a lot for the improvements. I think an addition of something like this to the docs is a smell because the real fix is to add a proper solution for joining fragments. My wish for the join method is for it to look like this:

sql` and `.join(...)

The current pr to add join is close, but tests aren't passing and the solution is too verbose sql.join(sql` and `, fragments). I'll follow up there too to get it moving

Copy link
Author

@JakobJingleheimer JakobJingleheimer Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Mm, I agree. I was thinking of this as an interim solution (and caveat) since sql.join() isn't available yet—I would much prefer sql.join() 🙂

That syntax though initially looks like Yoda 🤪 I think something like this would be much more intuitive:

await sql`
  SELECT * FROM "Example"
    WHERE ${sql.join(
      [
        sql`foo = ${foo}`,
        sql`bar = ${bar}`,
      ],
      'OR', // default to AND?
    )};
`

Copy link
Owner

Choose a reason for hiding this comment

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

Haha :P Yeah I can see your point. The trouble is that we need to use a fragment for the joiner if we want to keep the safety. That makes yours and the PRs example into sql.join(..., sql` or `).

Your sample also adds another question. Do we just willy nilly add whitespace on each side of the join string? That would probably help after few, but also be annoying for someone where the whitespace breaks some intended functionality

Copy link
Owner

Choose a reason for hiding this comment

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

I have also been thinking of using between instead to prevent the ambiguity around js join and sql join. That would also reverse the yodaness you mention.. But I agree that join is the goto keyword someone would think of so we'd loose that..


sql` and `.between(...)

Copy link
Author

Choose a reason for hiding this comment

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

That makes yours and the PRs example into sql.join(..., sql` or `).

Your sample also adds another question. Do we just willy nilly add whitespace on each side of the join string?

I guarantee you requiring users to include whitespace around the joiner arg will trip up almost, if not every, user 😬

annoying for someone where the whitespace breaks some intended functionality

I'm ~95% sure extra whitespace does not create a problem—AFAIK, SQL collapses it under the hood. You can even do something like "TableName" . "FieldName".

I have also been thinking of using between instead to prevent the ambiguity around js join and sql join.

Six of one, half a dozen of the other: SQL also has BETWEEN 😅 So I think on top of still being ambiguous, it's not what anyone would expect. I think users will be working with either a plain key-value object/dictionary or an array, and build their conditions from that.

I'm currently migrating from pg to this library, and the first thing I tried when dynamically building a WHERE clause was sql() (since it handles an object for INSERT). I hoped it would work for WHERE or WHERE … IN. I think it's not much of a mental stretch, so if sql() could also handle this, that would perhaps be the most straightforward for users:

await sql`
  UPDATE "Example"
    SET ${sql({ foo: 1 })}
    WHERE ${sql({ bar: 2 })};
`

sql() does not handle many cases for filter fields though (ex JSON(B) selectors like "SomeJsonField"->>'someSubField'), so without enhancing sql() to handle those, a dictionary can't be what's passed to WHERE's sql() since keys of an object can't themselves be objects, which is what sql`` produces: (invalid js) { [sql`"SomeJsonField"->>'someSubField'`]: 'qux' }.

Buuut, on the flip-side, sql is already quite overloaded, so maybe adding more onto the pile is actually not a good idea 🤔 And also sql() already accepts a second/...rest argument to pluck fields from the first, so it would probably be a huge headache to implement (if even possible—a user could do something insane like name a column AND, so the imp couldn't look for specific needles to determine what it's being passed).

P.S. Whatever the ultimate solution, please don't make it a promise—that makes debugging such a headache.


```js
const conditions = []

if (foo) conditions.push(sql`foo = ${foo}`)

await sql`
select * from users
where ${conditions.reduce((clause, condition) => sql`${clause} AND ${condition}`)};
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve the clarity of the reduce mechanics

Suggested change
where ${conditions.reduce((clause, condition) => sql`${clause} AND ${condition}`)};
where ${conditions.reduce((accumulatedCondition, condition) => sql`${accumulatedCondition} AND ${condition}`)};

Copy link
Author

Choose a reason for hiding this comment

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

I think this just makes it wordier

Copy link
Contributor

Choose a reason for hiding this comment

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

Long variable names are good to improve comprehension and expressiveness - see as an reference some of the variables that Facebook/Meta uses in various React codebases

But if you have a shorter (non-abbreviated) alternative to clause (which doesn't seem expressive enough to me), open to suggestions. I looked at 10 alternatives to clause, and this is the one that looked the best.

When I read clause:

  • I didn't immediately understand how this was being used in your reduce example
  • it also made me think "why is it now called clause, without any reference to condition anymore? everything else was called condition? what is the difference between a clause and a condition?"

Copy link
Author

Choose a reason for hiding this comment

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

I used clause because that's its official name in SQL. A quick search for "sql clause" gives 10 near-identical and succinct definitions. From DuckDuckGo's search assistant:

A SQL clause is a part of a SQL statement that specifies a particular condition or action, such as filtering data with the WHERE clause or sorting results with the ORDER BY clause.

That's exactly what this is, and it even cites the specific filter operator used in this example.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think introducing the new terminology makes it worse. My opinion would be to change it, for the reasons I already expressed above

`
```

### Dynamic ordering

```js
Expand Down