Skip to content
This repository has been archived by the owner on Mar 6, 2020. It is now read-only.

Many-to-many conjunctive (AND operator) queries #16

Closed
Mickael-van-der-Beek opened this issue Mar 3, 2016 · 4 comments
Closed

Many-to-many conjunctive (AND operator) queries #16

Mickael-van-der-Beek opened this issue Mar 3, 2016 · 4 comments
Labels

Comments

@Mickael-van-der-Beek
Copy link

This issue was already reported by @ErisDS on the main TryGhost/Ghost repository, issue reference:

TryGhost/Ghost#6158.

I actually really needed this feature to make my company's use-case work so I went ahead and implemented it in this branch of my fork:

Mickael-van-der-Beek/GQL#many-to-many-conjunctive-queries

And here's the diff compared to the current master branch:

https://github.com/TryGhost/GQL/compare/master...Mickael-van-der-Beek:many-to-many-conjunctive-queries?expand=1

As you can see the implementation is absolutely disgusting and that's also why I'm not trying to submit it as a PR. I just thought that maybe if someone worked on it in the future, they could use it a reference starting point.

A few thoughts about this implementation in no particular order:

  • it uses the slower SQL aggregation way of solving many-to-many queries (as discussed in the original issue as well as the StackOverflow answer)
  • it hard codes the SQL grouping column in the having() call
  • it assumes (as a shitty heuristic) that the WHERE ... IN () query is done on the column that is most used
  • will remove any other mentions of the chosen column from the query
  • has to use a module-global variable to store the having parameters as a workaround to scope limitations in the arguments passed to processFilter() (this one is not too important and can be fixed easily)

But it the end it passes the test suite and seems to work well. For the use-case we had (which was a hack in itself using Ghost's tagging system), neither Ghost nor Ghost's API are public facing so that meant that this solution was sufficient for now.

After thinking about the problem at hand during the day, I don't think that it's possible to automatically generate the SQL query if the parent application doesn't hint at which column specifically will be used for the WHERE ... IN () clause. Also, the HAVING clause has a hard dependency on the GROUPed column(s) which means that for Ghost's use-case where query composition is done it will be hard to implement it in a non-hardcoded way if GQL stays in it's current form.

@ErisDS
Copy link
Member

ErisDS commented Mar 4, 2016

@Mickael-van-der-Beek wow, thanks for taking the time to come and show what you have done. If you'd be up for writing some tests, I think your version is very, very worthy of a PR. Your self-analysis of your code reminds me of myself 😆

Nothing in GQL is particularly pretty, it's all a bit experimental, but in my experience when trying to tackle something this fiddly the nice solutions really don't fall out until you have implementations for all the common use cases and have a full picture to work with.

So yeah, please do submit a PR!

@ErisDS ErisDS changed the title Many-to-many conjunctive (AND operator) queries Many-to-many conjunctive (AND / OR operator) queries Jul 18, 2017
@ErisDS ErisDS changed the title Many-to-many conjunctive (AND / OR operator) queries Many-to-many conjunctive (AND operator) queries Jul 18, 2017
@kirrg001 kirrg001 added the bug label Apr 17, 2018
@bethanyr
Copy link

Hi - I've opened a PR with a possible solution for this issue - after reviewing the changes that Mickael-van-der-Beek made to solve this issue - it didn't appear to support 1 of my tag filter scenarios .

I have 2 tag filtering scenarios that my team uses in our custom Ghost template: filter="tag:sports+tag:football" and filter="tag:sports+tag:-football".

I took the approach of creating sub-queries for each tag filter using knex to build the subquery.

@Toub
Copy link

Toub commented Jul 24, 2018

@ErisDS 2 years later, what is the current status of this one?
is it still hard to implement AND filters?

Edit from https://forum.ghost.org/t/api-how-to-filter-articles-having-tag-a-and-tag-b-both-of-them/2121/2?u=toub

It’s a very tricky one to fix, and one that I am actively working on. No ETA though - if I manage it will be part of a major rewrite and shipped in Ghost 2.0 when that lands.

kirrg001 added a commit to TryGhost/Ghost that referenced this issue Dec 11, 2018
refs #10105, closes #10108, closes #9950, refs #9923, refs #9916, refs #9574, refs #6345, refs #6309, refs #6158, refs TryGhost/GQL#16

- removed GQL dependency
- replaced GQL with our brand new NQL implementation
- fixed all known filter limitations
- GQL suffered from some underlying filter bugs, which NQL tried to fix
- the bugs were mostly in how we query the database for relation filtering
- the underlying problem was caused by a too simple implementation of querying the relations
- mongo-knex has implemented a more robust and complex filtering mechanism for relations
- replaced logic in our bookshelf filter plugin
- we pass the custom, default and override filters from Ghost to NQL, which then are getting parsed and merged into a mongo JSON object. The mongo JSON is getting attached by mongo-knex.

NQL: https://github.com/NexesJS/NQL
mongo-knex: https://github.com/NexesJS/mongo-knex
@kirrg001
Copy link
Contributor

GQL was replaced by NQL. GQL is deprecated now.

This bug was fixed in Ghost 2.8.0 with the help of NQL 0.2.1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants