bgultekin / laravel4-datatables-package

Server-side handler of DataTables Jquery Plugin for Laravel 4
267 stars 108 forks source link

error adding clause "having" to a query #140

Closed ghost closed 10 years ago

ghost commented 10 years ago

Hi, Im trying to pass a query to Datatables::of($data) but I get an error.The var $data contain a query with having clause , at the moment I get an error due the filter I have added to it , Eg. having id =?. The error is located in the query that contains this part of code -> SQL: select count(*) as aggregate from (select '1' as row from table t1 inner join table2 t2 on t1.id=t2.id where field=? having id=?) I easly see the error is due to the missing id on the select fields.

Is there any way to perform a query using having ??

Notice that before I pass the query to the method of($data) I test the query with $data->get() and it retrieves the values as expected.

Thanks

MarkVaughn commented 10 years ago

Are you sure it's not because id is ambiguous? I other words try having t1.id = ? or having t2.id = ?

Mark Vaughn Mobile: +1 (770) 533-2274

On Sat, Sep 13, 2014 at 9:34 AM, dcastrili notifications@github.com wrote:

Hi, Im trying to pass a query to Datatables::of($data) but I get an error.The var $data contain a query with having clause , at the moment I get an error due the filter I have added to it , Eg. having id =?. The error is located in the query that contains this part of code -> SQL: select count(*) as aggregate from (select '1' as row from table t1 inner join table2 t2 on t1.id=t2.id where field=? having id=?) I easly see the error is due to the missing id on the select fields.

Is there any way to perform a query using having ??

Notice that before I pass the query to the method of($data) I test the query with $data->get() and it retrieves the values as expected.

Thanks

— Reply to this email directly or view it on GitHub https://github.com/bllim/laravel4-datatables-package/issues/140.

ghost commented 10 years ago

the query works when it has been executed before calling the method of() , also I have executed the query printing with -> $data->toSql(); copying into mysql and execute it the result return the values correctly.

ghost commented 10 years ago

I see where the problem is when I dig into the source code as the following part

" if (isset($having['column'])) { $myQuery->addSelect($having['column']); } else { " when "havingraw" is used there is no way to catch $having['column'] it does not exists thats why there is an error when my query is perform , the expected column is not set because there is no way to enter to the condition in order to call -> $myQuery->addSelect($having['column']); Havingraw contains a sql string without arguments,thats why the array "$having" does not set any column for this case.

ktunkiewicz commented 10 years ago

If something doesn't work post the source code. How can I help you if I don't know what your code or query is?

ghost commented 10 years ago

Hi this is the part of the query I used with the clause "havingraw" and "orhavingraw",

->havingraw('orders_packages.company_id in (select id from orders_companies c inner join orders_companies_members cm on c.id=cm.company_id where cm.member_id='.$id.')') ->orhavingraw('orders_packages.company_id='.$id)

There is no way from your code to catch the column as I explained before so I had to add a new "having" clause in order to catch the column and pass it to the query, adding "->having('packages.company_id','>',0)" like this is how your script allows to cacth the column company_id and enter to your conditional ---> if (isset($having['column'])) { $myQuery->addSelect($having['column']); } but not with havingraw.

At the end It worked by adding that

->having('packages.company_id','>',0) ->havingraw('orders_packages.company_id in (select id from orders_companies c inner join orders_companies_members cm on c.id=cm.company_id where cm.member_id='.$id.')') ->orhavingraw('orders_packages.company_id='.$id)

By the way your package is great!!

ktunkiewicz commented 10 years ago

When the plugin try to count records it puts your query (with filters) into this query:

SELECT count(*) as aggregate FROM ( SELECT <your_query_goes_here> )

but for performance reasons the columns part in your query is changed like this:

SELECT count(*) as aggregate FROM ( SELECT '1' as row FROM <the_rest_of_your_query_goes_here> )
//                                          ---------

that worked good until you use having clause because mySql requires names of columns you use in having clause in the columns part of SQL query.

So if you want to do something like ->having('packages.company_id','>',0) the counting query must look like this:

SELECT count(*) as aggregate FROM ( SELECT '1' as row, orders_packages.company_id FROM <the_rest_of_your_query_goes_here> ) 
//                                          ---------  --------------------------

The solution is to make the counting function add the required column names.

So if your having query is not raw it takes the column name from having function first parameter.

But if your having function is raw its not that easy. The column names are already deleted from the query so it is not that easy to put the required column back in there. The function tries to deduce the column name from your filters definition, but as far as I understand, you don't use filters here, you put the having clause directly into your query so it fails to find the column name.

I have no idea at the moment how to fix this... The counting function works well for having clause in filters because it get get information about column name from filter definition.

But for having in query it is not that easy... The only thing that I think is possible is to detect that some column name was not resolved and completly avoid changing columns definition in that case.

I think of changing this:

     $myQuery->addSelect($found);
}

into this:

     $myQuery->addSelect($found);
} else {
    // column name not resolved - revert all changes and escape the loop
    $myQuery = clone $query;
    break;
}

Can you test it?

I'm not sure if that works, and how that works in terms of performance... Can you also compare SQL timing with this change and without it?

ghost commented 10 years ago

Yes thats right , thats what I figured out when I looked into your code but with the fixed I did it works fine by adding ->having('packages.company_id','>',0) that was my approach and it includes all companies because all are greater than 0 , and I am able to include company_id into the query ;).

I tested it last saturday and it works!!

I have the last question, I have some problems when I have more than 10k records on a table because takes a little while before the ajax call from datatables retrieves all data to the table , is there any way to specify a limit that only bring for instance 10 records the first time the page is loaded??

thanks for your time and support :+1: )

ktunkiewicz commented 10 years ago

Just add paging in your Datatables and the SQL query should have the limit part too.

This is how I do it in Datatables 1.9:

oTable = $('#table').dataTable({
            "iDisplayLength": 50,
            (...)
})

This adds LIMIT 50 to the query. Works fine with 300k rows table.

I think you should also work on your SQL queries. Nested selects affects performance badly.

ghost commented 10 years ago

Hi again , Do you know how can I use "bServerSide": true, with the search ?? When "bServerSide" is activated the search does not work, it load data really fast and I am able to order by the columns or navigate through the pagination but not results after searching . This is my DataTable code var table; table=$('#tablesearch').DataTable( { "bProcessing": true, // "bFilter": true, "bServerSide": true, "iDisplayLength": 10, "sAjaxSource": "{{URL::to('gettable')}}", "aaSorting": [[0, "desc" ]], "fnServerData": function ( sSource, aoData, fnCallback ) { $.getJSON( sSource, aoData, function (json) {console.log(json) // Do whatever additional processing you want on the callback, then tell DataTables fnCallback(json) }); }, aoColumnDefs: [ { "aTargets": [ 0 ],"class":'details-control',"bVisible":true,"bSortable": true,'sWidth': '40px','sClass': 'details-control center'}, { "aTargets": [ 1 ],"bVisible":true, "bSortable": true,'sWidth': '70px', 'sClass': 'center' }, { "aTargets": [ 2 ],"bVisible":false, "bSortable": true,'sWidth': '0px', 'sClass': 'center' }, { "aTargets": [ 3 ],"bVisible":true, "bSortable": true,'sWidth': '30px', 'sClass': 'center' }, { "aTargets": [ 4 ],"bVisible":true, "bSortable": true,'sWidth': '50px', 'sClass': 'center' }, { "aTargets": [ 5 ],"bVisible":true, "bSortable":true,'sWidth': '80px', 'sClass': 'center' }, { "aTargets": [ 6 ],"bVisible":false, "bSortable": false,'sWidth': '0px', 'sClass': 'center' }, { "aTargets": [ 7 ], "bSortable": false,'sWidth': '2px', 'sClass': 'center' }, { "aTargets": [ 8 ], "bSortable": false,'sWidth': '2px', 'sClass': 'center' }, { "aTargets": [ 9 ], "bSortable": false,'sWidth': '2px', 'sClass': 'center' }, { "aTargets": [ 10 ], "bSortable": false,'sWidth': '2px', 'sClass': 'center' }, { "aTargets": [ 11 ], "bSortable": false,'sWidth': '5px', 'sClass': 'center' }, // { "aTargets": [ 12 ], "bSortable": false,'sWidth': '2px', 'sClass': 'center' } ], "sPaginationType": "bootstrap" }); Thanks

ghost commented 10 years ago

sorry I have fixed it :) +1: