bgultekin / laravel4-datatables-package

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

[ Bug ?] Filtering on prefixed table. #26

Open awsp opened 11 years ago

awsp commented 11 years ago

I got a column(s) not found on prefixed table whenever I preform filtering on dataTables. I dig into the code and I think I found a little bug that caused this problem.

at Datatables.php:401, from

$doctrine_column = DB::getDoctrineColumn($table, $target_column);

to,

$doctrine_column = DB::getDoctrineColumn($db_prefix . $table, $target_column);

at Datatables.php:409, from

if(Config::get('datatables.search.case_insensitive', false)) {
    $column = $db_prefix . $column;
    $query->orwhere(DB::raw('LOWER(CAST('.$column.' as CHAR('.$column_max_length.')))'), 'LIKE', $keyword);
} else {
    $query->orwhere(DB::raw('CAST('.$column.' as CHAR('.$column_max_length.'))'), 'LIKE', $keyword);
}

to

$column = $db_prefix . $column;
if(Config::get('datatables.search.case_insensitive', false)) {
    $query->orwhere(DB::raw('LOWER(CAST('.$column.' as CHAR('.$column_max_length.')))'), 'LIKE', $keyword);
} else {
    $query->orwhere(DB::raw('CAST('.$column.' as CHAR('.$column_max_length.'))'), 'LIKE', $keyword);
}
tortuetorche commented 11 years ago

:+1:

bgultekin commented 11 years ago

Thanks for your contribution :+1: I am editing the code.

swilla commented 11 years ago

I think this broke join table aliases.

swilla commented 11 years ago
{"error":{"type":"Doctrine\\DBAL\\Schema\\SchemaException","message":"There is no column with name 'name' on table 'subcat'.","file":"\/var\/www\/myimsreport\/code\/vendor\/doctrine\/dbal\/lib\/Doctrine\/DBAL\/Schema\/SchemaException.php","line":85}}

Here is the Fluent query:

$programs = Program::select('programs.id', 'programs.title','companies.name as marketingCompany','categories.name as category_name','subcat.name as subcategory_name')
                        ->whereNull('master_id')
                        ->where('language_id', 1)   
                        ->leftJoin('categories','programs.category_id','=','categories.id')
                        ->leftJoin(DB::raw('categories as subcat'),'programs.sub_category_id','=','subcat.id')
                        ->leftJoin('companies', 'programs.marketing_company_id','=','companies.id')
                        ->orderBy('programs.title');
bgultekin commented 11 years ago

Can you share sql query which is tried to run?

awsp commented 11 years ago

@bllim Thanks for the update!

@swilla I am doing aliases with join tables too, but got no problems. What would happen if you do that with just a ->get()? I did both, got no problem on filtering

->leftJoin('statuses as stat', 'stats_id', '=', 'bookings.status_id')

and

->leftJoin(DB::raw('statuses as stat'), 'stats.id', '=', 'bookings.status_id');

I have tried re-migrate my tables without prefix once, and it was working.

------ Update ----- I think there is an error on table alias, but I think this occurs even before this update. So I guess for now just do the whole table until there is a fix? :)

Regards

swilla commented 11 years ago

Here is the actual query I want to run, it doesn't actually run it because of the doctrine error:

SELECT programs.id, programs.title, companies.name as marketingCompany, categories.name as category_name, subcat.name as subcategory_name
FROM programs
left join categories on programs.category_id = categories.id
left join categories as subcat on programs.sub_category_id = subcat.id
left join companies on programs.marketing_company_id = companies.id
ORDER BY programs.title

This worked before the recent changes, but the original bug described in this issue did not work.

awsp commented 11 years ago

I think the issue is that DataTables.php lines 380 and 390 are trying to preg_match the alias on query's select and parse it as the table name and column, as a result it would call your subcat table instead of the actually alias made on the table itself.

If it is possible to get the correct joins table alias, I agree line 401 as well as 409 will need to have some checks to ensure when they contain alias, then table prefix should not be added.

bgultekin commented 11 years ago

@swilla I wonder the query which datatable package trying to run (and also fail). @awsp can you share the fluent/eloquent code which causes error at first (before update)?

awsp commented 11 years ago

Here is the code I used. But I am using the whole table name before.

What I am thinking is, would that be possible when @swilla did composer update it actually pulls some other packages that are incapable with Laravel or other packages such as Doctrine.

Here is the original code I was using. Prefix is 'lh_'

$reservations = Reservation::select(array(
    'reservations.id as reservation_id',
    'customers.lastname',
    'customers.firstname',
    'customers.email_address',

    'reservations.num_of_customers',
    'courses.name as course',

    'statuses.label as status',
    'statuses.id as statusid',
))
->leftJoin('courses', 'courses.id', '=', 'reservations.course_id')
->leftJoin('customers', 'customers.id', '=', 'reservations.customer_id')
->leftJoin('statuses', 'statuses.id', '=', 'reservations.status_id')
;

return Datatables::of($reservations)->make();

Exception I get,

/Applications/MAMP/htdocs/bookings/vendor/doctrine/dbal/lib/Doctrine/DBAL/Schema/SchemaException.php
Doctrine \ DBAL \ Schema \ SchemaException
There is no column with name 'id' on table 'reservations'.
static public function columnDoesNotExist($columnName, $table)
    {
        return new self("There is no column with name '$columnName' on table '$table'.", self::COLUMN_DOESNT_EXIST);
    }

I also tried this using simple get() with alias and, --- This is not working ----

$reservations = Reservation::select(array(
        'reservations.id as reservation_id',
        'cust.lastname',
        'cust.firstname',
        'cust.email_address',

        'reservations.num_of_customers',
        'courses.name as course',

        'statuses.label as status',
        'statuses.id as statusid',
    ))
    ->leftJoin('courses', 'courses.id', '=', 'reservations.course_id')
    ->leftJoin('customers as cust', 'cust.id', '=', 'reservations.customer_id')
    ->leftJoin('statuses', 'statuses.id', '=', 'reservations.status_id')
    ;

echo '<pre>'; var_dump($reservations->get()); echo '</pre>'; die();
bgultekin commented 11 years ago

By the way, I have just merged @tortuetorche 's pull-request. Can you try again with updated code?

Lastly, sorry I didn't have an opportunity to test so that I couldn't care. I will care as soon as possible.

swilla commented 11 years ago

I still get the same error after a composer update:

{"error":{"type":"Doctrine\\DBAL\\Schema\\SchemaException","message":"There is no column with name 'name' on table 'subcat'.","file":"\/vagrant\/vendor\/doctrine\/dbal\/lib\/Doctrine\/DBAL\/Schema\/SchemaException.php","line":85}}
awsp commented 11 years ago

@bllim

on prefix table, with table alias: Fail on prefx table, without table alias: Success

no prefix table, with table alias: Fail no prefix table, without table alias: Success

Doctrine \ DBAL \ Schema \ SchemaException
There is no column with name 'lastname' on table 'lh_cust'.
awsp commented 11 years ago

When it comes to prefix table with alias, event without DataTables, it looks like the actual query will throw a column not found exception.

I wrote a simple quick checks, What I actually did was, try to observe the join table to see if there is an alias set for each joins and re-correct it before it gets inserted to DB::getDoctrineColumn().

But it turns out no matter what I put, the alias with prefix just doesn't work. for example, I did

->leftJoin('customers as cust')

I tried, for DB::getDoctrineColumn(pesudo),

Caution: this is un-tested code, just an Example only


private function filtering()
{
    $joinTables = array();
    foreach($this->query->getQuery()->joins as $joinTable) {
        preg_match('#^(\S*?)\s+as\s+(\S*?)$#si',$joinTable->table,$tmpTable);
        if( ! empty($tmpTable)) {
            $joinTables[$tmpTable[2]] = $tmpTable[1];
        }
    }

    if (Input::get('sSearch','') != '')
    {
        $copy_this = $this;

        $this->query->where(function($query) use ($copy_this, $joinTables) {

            $db_prefix = $copy_this->database_prefix();

            for ($i=0,$c=count($copy_this->columns);$i<$c;$i++)
            {
                if (Input::get('bSearchable_'.$i) == "true")
                {
                    preg_match('#^(\S*?)\s+as\s+(\S*?)$#si',$copy_this->columns[$i],$matches);
                    $column = empty($matches) ? $copy_this->columns[$i] : $matches[1];
                    $keyword = '%'.Input::get('sSearch').'%';

                    if(Config::get('datatables.search.use_wildcards', false)) {
                        $keyword = $copy_this->wildcard_like_string(Input::get('sSearch'));
                    }

                    // Check the current $column type
                    // If it isn't a string/text/blob, cast it to a string of 255 characters
                    $cast_begin = null;
                    $cast_end = null;
                    $column_max_length = 255;
                    preg_match('#([^.]+)\.(.+)$#si', $column, $table_infos);
                    if(empty($table_infos)) {
                        throw new \Exception("Invalid table and column names format for '".$column."'");
                    } else {
                        if(empty($table_infos[1])){
                            throw new \Exception("Empty table for '".$column."'");
                        } elseif (empty($table_infos[2])) {
                            throw new \Exception("Empty column for '".$column."'");
                        }
                        $table = $table_infos[1];
                        $target_column = $table_infos[2];

                        if( ! array_key_exists($table, $joinTables)) {
                            $doctrine_column = DB::getDoctrineColumn($db_prefix . $table, $target_column);
                            $column = $db_prefix . $column;
                        }

                        $type = $doctrine_column->getType()->getName();
                        if( !in_array($type, array('string', 'text', 'blob')) ) {
                            $cast_begin = "CAST(";
                            $cast_end = " as CHAR(".$column_max_length."))";
                        }
                    }

                    if(Config::get('datatables.search.case_insensitive', false)) {
                        $query->orwhere(DB::raw('LOWER('.$cast_begin.$column.$cast_end.')'), 'LIKE', $keyword);
                    } else {
                        $query->orwhere(DB::raw($cast_begin.$column.$cast_end), 'LIKE', $keyword);
                    }
                }
            }
        });

    }

    $db_prefix = $this->database_prefix();

    for ($i=0,$c=count($this->columns);$i<$c;$i++)
    {
        if (Input::get('bSearchable_'.$i) == "true" && Input::get('sSearch_'.$i) != '')
        {
            $keyword = '%'.Input::get('sSearch_'.$i).'%';

            if(Config::get('datatables.search.use_wildcards', false)) {
                $keyword = $copy_this->wildcard_like_string(Input::get('sSearch_'.$i));
            }

            if(Config::get('datatables.search.case_insensitive', false)) {
                $column = $db_prefix . $this->columns[$i];
                $this->query->where(DB::raw('LOWER('.$column.')'),'LIKE', $keyword);
            } else {
                $this->query->where($this->columns[$i], 'LIKE', $keyword);
            }
        }
    }
}
swilla commented 11 years ago

I just tested this code and it works for me.


Edit: Filtering works, but now column sorting does not (with changes above).

Kampfbereit commented 11 years ago

When searching a tabla, with a prefix with a dash, like "bla-bla_" the resulting query only searches for "bla". the prefix and column need to be encompassed by grave accent ( ` ) or similar like in the FROM statement:

select count(*) as aggregate from `rydbergs-gourmet_tips` where (rydbergs-gourmet_tips.id LIKE ? or rydbergs-gourmet_tips.content LIKE ? or rydbergs-gourmet_tips.filename LIKE ? or rydbergs-gourmet_tips.created_at LIKE ?)

I can only get it to work if I change line 402 in Datatables/Datatables.php from:

$column = $db_prefix . $column;
if(Config::get('datatables.search.case_insensitive', false)) { 
    $query->orwhere(DB::raw('LOWER('.$cast_begin.$column.$cast_end.')'), 'LIKE', $keyword);
} else {
    $query->orwhere(DB::raw($cast_begin.$column.$cast_end), 'LIKE', $keyword);
}

to:

/* $column = $db_prefix . $column; */
if(Config::get('datatables.search.case_insensitive', false)) {
    $query->orwhere($column, 'LIKE', $keyword);
} else {
    $query->orwhere($column, 'LIKE', $keyword);
}
tortuetorche commented 11 years ago

@Kampfbereit I think your workaround breaks PostgreSQL compatibility. Why did you remove the DB::raw($cast_begin.$column.$cast_end) part ?

Kampfbereit commented 11 years ago

I know, I just tried to narrow it down to se where the problem was. I just found another related problem.

This select:

Tip::select('tips.id', 'tips.content', 'tips.filename', 'tips.created_at', DB::raw('WEEK('. DB::getTablePrefix() .'tips.created_at)') );

Generated this faulty SQL:

select count(*) as aggregate from `rydbergs_gourmet_tips` where (rydbergs_gourmet_tips.id LIKE ? or rydbergs_gourmet_tips.content LIKE ? or rydbergs_gourmet_tips.filename LIKE ? or rydbergs_gourmet_tips.created_at LIKE ? or rydbergs_gourmet_WEEK(rydbergs_gourmet_tips.created_at) LIKE ?

Again it's the prefix and DB::raw that doesn't like eachother.

Ended up putting it in ->add_column instead.

ktunkiewicz commented 10 years ago

This issue will be reviewed to ensure that new code is working fine with prefixing. Added to TO DO list for 1.4.1 release #122

fheckl commented 9 years ago

I also have this issue when using Datatables. When a filter is set on my table it tries to execute this SQL:

select count(*) as aggregate from (select '1' as row from `pfreg_view_registrations` where (pfreg_id LIKE %heckl% or pfreg_vorname LIKE %heckl% or pfreg_name LIKE %heckl% or pfreg_geburtstag LIKE %heckl% or pfreg_countdown LIKE %heckl% or pfreg_dioezese LIKE %heckl%)) AS count_row_table)

which leads to an error message:"SQLSTATE[42S22]: Column not found: 1054 Unknown column 'pfreg_id'..." because the query really should be:

select count(*) as aggregate from (select '1' as row from `pfreg_view_registrations` where (id LIKE %heckl% or vorname LIKE %heckl% or name LIKE %heckl% or geburtstag LIKE %heckl% or countdown LIKE %heckl% or dioezese LIKE %heckl%)) AS count_row_table)

The error lies in line 610 of Datatables.php (as of version 1.4.4) which prefixes column names with the database prefix:

$column = $db_prefix . $columns_clean[$i];

Why would you want to prefix column names in that situation?? Removing the prefix fixes my problem but does it break something else?