timgws / QueryBuilderParser

A simple to use query builder for the jQuery QueryBuilder plugin for use with Laravel.
MIT License
159 stars 65 forks source link

Possibility to use HAVING instead of WHERE everywhere #26

Open royduin opened 6 years ago

royduin commented 6 years ago

I let my users create their own subqueries which can be used as filters in the query builder. But wheres are not working with values from subqueries so I need this one to be $query->having() instead of $query->where(). But in QBPFunctions.php and in JoinSupportingQueryBuilderParser.php there are some more wheres which should change in that case.

Maybe a new parameter in the constructor?

public function __construct(array $fields = null, $useHaving = false)
timgws commented 6 years ago

Hi @royduin,

Firstly, sorry for taking such a long time to get back to you.

I don't think that having $useHaving as a parameter on the constructor is necessarily the best way to go about this.

Potentially, using something like the Strategy pattern (https://github.com/domnikl/DesignPatternsPHP/tree/master/Behavioral/Strategy) to define how the queries are built might be a little better then an if statement.

This way, a strategy can be included by default by the constructor method, and then if a new one is desired, it can be set (by calling setWhereStrategy or something similar).

This removes the requirement to have a large amount of parameters inside the constructor (which is good, IMHO. We should not have too many parameters, because it means the class does not really have a single responsibility).

Wheres might still be able to work with your subquery, if you select them into a temporary table and perform the where on that, however, depending on the size of your table, this could potentially be slow.

ie, for a simplistic example something like:

select * from my_table where price < 2400 and item_name like '%macbook pro%'

would be turned into

select * from (
    select * from my_table where price < 2400 and item_name like '%macbook pro%'
) as my_query_table

where your queries could then be added to the select * query.

royduin commented 6 years ago

Hi @timgws, thanks for your response. I'm currently using your suggestion but in comparison with "having" it's very slow. For other people coming by here, I'm using this to do that in Laravel 5.5:

$query = DB::table(DB::raw("({$query->toSql()}) as t"))->mergeBindings($query);

I'll update my PR asap with your suggestion and hope I find some time shortly to implement "having" on all places.

mijaelsaban commented 2 years ago

Hi, I am encountering same need,

Is there any update on this topic?

royduin commented 2 years ago

See https://github.com/timgws/QueryBuilderParser/pull/27

mijaelsaban commented 2 years ago

I used this as a solution as you mentioned
https://github.com/timgws/QueryBuilderParser/issues/26#issuecomment-363074369

$query = DB::table(DB::raw("({$query->toSql()}) as t"))->mergeBindings($query);

I tried to override the package with your code #27 but It doesn't work as expected when using nested groups, for example trying with a whereIn and having. it just adds the having at the end without respecting the groups. I mean for nested groups I don't think it would work.

Thanks

timgws commented 2 years ago

@mijaelsaban @royduin

Could you please email me (or include in this issue) a good example of how you are using the QueryBuilder, and why having (heh!) support for HAVING over WHERE would be better?

Just trying to work out a good way that this can be integrated into this project. As I understand MySQL, the HAVING part of the query will only work after the query has been run by MySQL, so switching everything to HAVING could mean a massive performance hit if not done correctly.

Are the HAVING queries to be always processed on particular columns? Will those columns always be fetched by the QBP queries? Would be good to see example use cases. Happy to spend some time this weekend looking at it if some good usecases can be documented 🚀

mijaelsaban commented 2 years ago

Hi

My question was related on having an alias on my query then I cannot use WHERE anymore but HAVING.

My solution was like @royduin suggested to put the query inside another query and then I can use WHERE again.

Example query:

In this case cannot use WHERE width > 0

select manufacturers.markup * countries.markup * countries.markup_currency * countries.markup_shipping *
             countries.markup_payment * countries.markup_ddp            as markup,
             (select sum(if(product_variation.quantity > 0, 1, 0)) / count(*) * 100
              from `product_variation`
              where `product_variation`.`product_id` = `products`.`id`) as `width`,
             manufacturer_id
      from `products`
               left join `manufacturers` on `manufacturers`.`id` = `products`.`manufacturer_id`
               left join `countries` on `iso_code` = 'AF'

Solution like mentioned above:

select *
from (select manufacturers.markup * countries.markup * countries.markup_currency * countries.markup_shipping *
             countries.markup_payment * countries.markup_ddp            as markup,
             (select sum(if(product_variation.quantity > 0, 1, 0)) / count(*) * 100
              from `product_variation`
              where `product_variation`.`product_id` = `products`.`id`) as `width`,
             manufacturer_id
      from `products`
               left join `manufacturers` on `manufacturers`.`id` = `products`.`manufacturer_id`
               left join `countries` on `iso_code` = 'AF')
         as t
where `manufacturer_id` in (98, 137)
  and width > 20
mijaelsaban commented 2 years ago

Hi guys,

Unfortunately, the last solution is not good at all

It loops for each record and builds tons of rows which makes it impossible,

Trying to find other solutions...

If someone had same issue, please let me know

image

mijaelsaban commented 2 years ago

Got a not a clean solution but works more efficiently.

The idea is in the were using the subselect instead of the alias in the HAVING statement.

STEPS:

  1. First of all, put the expected query in the id (check image) of the JqueryBuilder Element.

  2. Second, need to remove the backticks that are created on each dot(.) of the query with this. $queryWithNoBackTicks = str_replace(array("",chr(96)), '', $query);`

And works.

Definitely, a Short-term solution, looking for something cleaner for the future.

image