Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

pluck() performance issues (specifically Query\Builder::pluck) #98

Closed
vlakoff opened this issue May 26, 2016 · 11 comments
Closed

pluck() performance issues (specifically Query\Builder::pluck) #98

vlakoff opened this issue May 26, 2016 · 11 comments

Comments

@vlakoff
Copy link

vlakoff commented May 26, 2016

I encountered performance issues using Query\Builder::pluck(). This is because it uses Arr::pluck(), which calls data_get() on each row. data_get() easily becomes a bottleneck when used in loops.

On a tiny 5000 rows test database, Builder::pluck() takes 184 ms. Right now, I'm using a custom method which takes 40 ms.

// CustomQueryBuilder
public function fastPluck($column, $key = null)
{
    $results = $this->get($key === null ? [$column] : [$column, $key]);

    return object_column($results, $column, $key);
}

// custom helper
function object_column($data, $column, $index = null)
{
    $result = [];

    if ($index === null) {
        foreach ($data as $row) {
             $result[] = $row->$column;
        }
    } else {
        foreach ($data as $row) {
             $result[$row->$index] = $row->$column;
        }
    }

    return $result;
}

I can use this code, because I know:

  • I'm manipulating an array of objects (config/database.php 'fetch' => PDO::FETCH_CLASS)
  • There is no need for "dot access"
  • There is no missing value


Maybe this could be of inspiration for implementing a general-purpose optimized method.

@franzliedke
Copy link

I like the idea, but can you really rely on your fetch mode? After all, it is configurable in config/database.php.

That said, maybe we can use either array_get or object_get depending on the fetch mode instead?

@vlakoff
Copy link
Author

vlakoff commented Jun 12, 2016

Arr::get() wouldn't add value (we don't need dot access nor support for ArrayAccess). Same for object_get().

Indeed, the code would have to support both array and object fetch modes.

My main concern is about the missing values. Could this only happen if the user asked for a nonexistent DB column, or could there be other cases?

@franzliedke
Copy link

I'm fairly positive this would only happen for invalid (nonexistent) column names, yeah...

@vlakoff
Copy link
Author

vlakoff commented Jun 14, 2016

So basically, the only question is: what should happen if the user queries a nonexistent column (which is clearly wrong). Currenty it would return an array of nulls… I suppose it would be okay if the user gets a shitload of "index/property undefined", so he would just fix its code nonetheless.

@vlakoff
Copy link
Author

vlakoff commented Jun 17, 2016

That's even simpler: querying a nonexistent column would throw a sql error beforehand…

So, I can't think of any technical limitation for optimizing Builder::pluck().

@vlakoff
Copy link
Author

vlakoff commented Jun 18, 2016

The main (and only) concern about implementing the optimization, is proper handling of PDO fetch mode.

As it may be of interest when working on this, refs laravel/framework#12171.

@vlakoff
Copy link
Author

vlakoff commented Jun 22, 2016

Just as a reminder, one case to keep in mind: if the user has set the connection to PDO::FETCH_CLASS, CustomClass and this class modifies the properties when building.

Maybe an edge case, but I suppose here we'd want to pluck the class-modified properties, not the original properties from DB.

@taylorotwell
Copy link
Member

I'm open to improvements here if possible.

@franzliedke
Copy link

PR cross-reference: laravel/framework#23482

@manjuuu
Copy link

manjuuu commented Oct 24, 2018

pluk is there for laravel 5

@manjuuu
Copy link

manjuuu commented Oct 24, 2018

pluck is changed as value()

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

No branches or pull requests

4 participants