Skip to content
This repository was archived by the owner on Oct 9, 2025. It is now read-only.

Conversation

soedirgo
Copy link
Member

What kind of change does this PR introduce?

Feature.

What is the current behavior?

Some filter names are very short and hard to understand.

What is the new behavior?

Rename the filters as per #148. Some deviations to the original suggestion (I'm not good at names so if these look bad feel free to chime in):

  • cd becomes containedBy rather than within (closer to the short version)
  • ov becomes overlaps rather than within (same here)
  • fts becomes type: null (default) rather than type: 'tsvector' (tsvector is the type of the text/document rather than the query)
  • plfts becomes type: 'plain' rather than type: 'plainto'
  • phfts becomes type: 'phrase' rather than type: 'phraseto'

Old filter names are kept for back compat but are tagged with @deprecated.

Additional context

Closes #148.

@soedirgo soedirgo requested a review from kiwicopple March 11, 2021 07:01
@kiwicopple
Copy link
Member

Amazing work @soedirgo 💯 . I'm curious about the rangeGt, rangeGte etc - did you choose this over range for a particular reason?

@soedirgo
Copy link
Member Author

👍

did you choose this over range for a particular reason?

Mostly because they operate on different kinds of columns, in contrast to textSearch where each variant is just a different way of interpreting the query.

@kiwicopple
Copy link
Member

kiwicopple commented Mar 11, 2021

Great. I inlined some of the test snapshots so they are easier to read. From a documentation POV, I wouldn't mind putting all the range queries together, but I guess it's consistent with out gt, lt, gte, lte filters

Would be interested to get @thorwebdev and @joshnuss 's (OP) opinion on the final changes:

  • .cs() -> .contains()
  • .cd() -> .containedBy()
  • .sl() -> .rangeLt()
  • .sr() -> .rangeGt()
  • .nxl() -> rangeGte()
  • .nxr() -> rangeLte()
  • .adj() -> .adjacent()
  • .ova() -> .overlaps()
  • .fts() -> .textSearch(column, search, {config: 'english'})
  • .plfts() -> .textSearch(column, search, {config: 'english', type: 'plain'})
  • .phfts() -> .textSearch(column, search, {config: 'english', type: 'phrase'})
  • .wfts() -> .textSearch(column, search, {config: 'english', type: 'websearch'})

@steve-chavez this look ok to you?

@steve-chavez
Copy link
Member

Looks much better!

.adj() -> .adjacent()

Since adjancent(-|-) is only for range types, perhaps the filter should be named rangeAdjacent?

(check with select * from pg_operator where oprname = '-|-';)

.overlaps()

This one looks good. overlaps(&&) can be applied on other types besides ranges(arrays for example).

(check with select * from pg_operator where oprname = '&&';)

@joshnuss
Copy link

This is great! I think it will help a lot of folks. 👍

@kiwicopple
Copy link
Member

OK great, looks like we have enough consensus here.

Since adjancent(-|-) is only for range types, perhaps the filter should be named rangeAdjacent?

@soedirgo perhaps we rename this one as Steve suggested, then we're good to merge yeah?

@soedirgo
Copy link
Member Author

Yup! I've done the rename.

@kiwicopple kiwicopple merged commit 4188925 into master Mar 22, 2021
@kiwicopple kiwicopple deleted the feat/rename-filters branch March 22, 2021 10:53
@github-actions
Copy link

🎉 This PR is included in version 0.28.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename filters

4 participants