laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.21k stars 10.9k forks source link

[Request] Allow Eloquent to save multiple records at once #1295

Closed rogierborst closed 11 years ago

rogierborst commented 11 years ago

I'd love to be able to insert multiple records into the database in one fell swoop. I know you can do that with something like User::insert([array of multiple user detail arrays]); but insert is not handled by Eloquent, but by Fluent instead.

One of the disadvantages of that, is that Fluent doesn't set timestamps on these records.

abishekrsrikaanth commented 11 years ago

+1. I feel User::create([array of multiple user detail arrays]) would be a more appropriate function to handle this

robclancy commented 11 years ago

But that isn't what an ORM does?

jaketoolson commented 11 years ago

+3 (pun intended get it? I just bulk voted...)

crynobone commented 11 years ago

What's wrong with

foreach ($users as $user)
{
    User::create($user);
}
Anahkiasen commented 11 years ago

It executes as much queries as you have entries to add ?

jaketoolson commented 11 years ago

I'd hope that with this request, Eloquent would (behind the scenes) create a single bulk extended insert, rather than multiple insert statements as a foreach() would.

crynobone commented 11 years ago

@jaketoolson Eloquent would still need to run the insert one by one. How would you expect the event to be fired if it's a bulk insert? As mention by @robclancy this isn't really a task for ORM.

Anahkiasen commented 11 years ago

Well no that's the point, it doesn't need to run them one by one. Most SQL languages support bulk inserts, I'm pretty sure having one query instead of, say, 50, is significant enough to be added. And if you can insert multiple related models with saveMany I don't see why inserting multiple models in itself isn't in the scope.

crynobone commented 11 years ago

Then why not DB::table('users')->saveMany($users);

rogierborst commented 11 years ago

saveMany() only works for relations (I believe many-to-many relations specifically), so something like this: $user->saveMany($roles) That's not what we're going for here.

taylorotwell commented 11 years ago

It would be relatively easy to do this; however, events would not be fired. The only thing it would be buying you is the timestamp insertion.

helmut commented 11 years ago

Just an idea but perhaps when performing multiple creates, deletes and updates the system could throw some 'creating.many' or 'updating.many' events that instead of passing along the actual object it passed an array of affected primary key ids. So you could handle that use case in your event handlers.

crynobone commented 11 years ago

@helmut then what if I would need to know certain attributes (added or changed) on the event? I would still need to fetch the row from database (and compared it against old value). Example:

static::updating(function ($role)
{
     Acl::renameRole($role->getOldAttributes('name'), $role->name);
});

With DB Query Builder, it would be an expected behaviour that event wouldn't be fired (at all).

rogierborst commented 11 years ago

If timestamp insertion is the only thing it'd bring, I'm okay with setting those myself.

foreach( $users as $user ) {
    $user['created_at'] = new DateTime;
    $user['updated_at'] = $user['created_at'];
}
User::insert($users);

Something like that...

hasokeric commented 11 years ago

+1 i was just trying to do multi-array insert in L3 you know something you would think would happen by design and then when i got the error i researched old L3 docs and i needed to make a foreach loop :)

drakakisgeo commented 11 years ago

+1 I really need to insert/update many thousand records, as fast as possible

robclancy commented 11 years ago

Then use the DB:: methods.

drakakisgeo commented 11 years ago

Can I have something like |LOAD DATA |(Mysql) with DB::method? Topic here is to do as little queries as you can to insert/update many thousand records.

On 2/7/2013 9:14 πμ, Robert Clancy (Robbo) wrote:

Then use the |DB::| methods.

— Reply to this email directly or view it on GitHub https://github.com/laravel/framework/issues/1295#issuecomment-20328423.

jfloff commented 11 years ago

+1

pushpak commented 11 years ago

+1 our users upload products in bulk (csv)

taylorotwell commented 11 years ago

Honestly just use the query builder for this. Will be much more efficient and makes more sense.

jianfengye commented 9 years ago

Eloquent does not have multcreate...

hope Eloqent can add this function...

tonglil commented 9 years ago

While I would be completely satisfied with the query builder method, I found that the qb requires all fields to be filled out in order to perform a proper insert.

For example:

Model:
- field_a
- field_b
- field_c
$instance_a = [
    'field_a' => 'filled',
    'field_b' => 'filled',
    // 'field_c' => not included in the array,
];
$instance_b = [
    'field_a' => 'filled',
    // 'field_b' => not included in the array,
    'field_c' => 'filled',
];
// This will insert "field_c" of $instance_b as "field_b"
Model::insert([$instance_a, $instance_b]);

The only work around is to populate the fields left out with null or ''. For example, missing (optional) inputs.

chimit commented 9 years ago

If I'm not mistaken, Eloquent's create method returns inserted object (with primary key), but Query Builder's insert method returns BOOLEAN. That's the point.

kinologik commented 9 years ago

+1 for "::bulkCreate". I have a few conversion methods in my Eloquent model, and they work fine using Create, but obviously get bypassed when using Insert. They just "feel" at the right place in my Model, getting triggered at the moment values are attributed. Having to code my converters twice for Eloquent and QueryBuilder seems a bit messy. It can be done, but not very elegant... and on a project bigger than mine, could impact maintenance.

legshooter commented 8 years ago

+1

Agree with points made by @rogierborst @Anahkiasen @tonglil @kinologik

I want to add on a more general level, that resorting to the query builder for such a simple task doesn't make sense to me. Eloquent should enable us to do as much of the work as humanly possible (and in the most correct and efficient way). It should be the framework's/ORM's aspiration that users would have to use the query builder or even manually write their own queries, only when it's something so complex, that 90% of us would never go about doing anyway. Inserting multiple records is definitely not one of those cases.

mpmurph commented 8 years ago

I would like to upvote this suggestion too. Right now I am using MySql's INSERT... ON DUPLICATE KEY UPDATE functionality with DB::statement() to do batch insert/updates but am struggling to figure out how to trigger an update to created_at or updated_at as appropriate. It would be very helpful if Eloquent had this functionality built in.

bgarrison25 commented 8 years ago

+1

I am actually falling into this same issue http://stackoverflow.com/questions/34885575/laravel-5-2-updating-multiple-rows-with-one-query?noredirect=1#comment57509405_34885575

mwaa commented 8 years ago

+1

Would be helpful if this(bulk creates) can be implemented in eloquent

jcperez2405 commented 8 years ago

Just use: DB::table('users')->insert([ ['email' => 'taylor@example.com', 'votes' => 0], ['email' => 'dayle@example.com', 'votes' => 0] ]);

cabloo commented 8 years ago

This is what I'm currently using (warning: does not generate events):

abstract class Model extends BaseModel
{
    /**
     * @return string
     */
    public static function table()
    {
        return with(new static)->table;
    }

    /**
     * Insert each item as a row. Does not generate events.
     *
     * @param  array  $items
     *
     * @return bool
     */
    public static function insertAll(array $items)
    {
        $now = \Carbon\Carbon::now();
        $items = collect($items)->map(function (array $data) use ($now) {
            return $this->timestamps ? array_merge([
                'created_at' => $now,
                'updated_at' => $now,
            ], $data) : $data;
        })->all();

        return \DB::table(static::table())->insert($items);
    }
}

Example:

Administrator::insertAll([
    ['name' => 'Zane'],
    ['name' => 'Rob'],
]);
bgarrison25 commented 8 years ago

@cabloo is their a reason that you take an array and turn it into a collection just to turn it back into an array again? I can see doing that if a collection has a method on it that you want to use that php doesn't have inherently (which is many) but php has a function for this (array_map) so your just introducing overhead. Other than that....this is a great idea and ive implemented it.

cabloo commented 8 years ago

@bgarrison25 just because I like the way it looks more. The overhead you mention will only be significant when $items is sufficiently large (i.e. many rows or large amounts of data in the columns), which will also take a second hit during array_merge since $data is copied into the first array. I'm not dealing with either of those use cases in my code, so I didn't bother implementing accordingly. I would definitely recommend modifying the above code to fix both of those issues if you find yourself working with larger inserts.

crzzano commented 7 years ago

The solution is to use createMany method.

robclancy commented 7 years ago

^ no it isn't

TwanoO67 commented 7 years ago

agree with @robclancy , this is doing the exact same amount of ->save() call ...

prog-24 commented 7 years ago

@twocatszerokarma that is only if you are inserting related models.

prog-24 commented 7 years ago

using the for loop is the best solution in my opinion because it fire events.

ajcastro commented 7 years ago

What if we can do this:

class Model extends EloquentModel {
    public static function batchCreate($items)
    {
        $models = array_map(function($item) {
            $model = new static($item);
            // Then fire the creating event for the model here.
            // I dont know how to fire it tho.
            // Also, set the created_at and updated_at of the model.
            return $model;
        });

        $this->newQuery()->insert(array_map(function ($model) {
            return $model->getAttributes();
        }, $models));

        // Fire the created event here on each model
    }

    public static function batchUpdate($models)
    {
        $models = $models instanceof Collection ? $models->all() : $models;

        // Using the query builder api proposal batchUpdate() in laravel/internals #490:
        $this->newQuery()->batchUpdate('id', array_map(function ($model) {
            // Fire the updating event here
            // Set the updated_at attribute here
            return $model->getAttributes();
        }));

        // Fire the updated event here on each model
    }
}

This is the link of the batchUpdate() proposal: https://github.com/laravel/internals/issues/490

ajcastro commented 7 years ago

I think it is better if we will create a separate package, which can be called "laravel-unit-of-work". A Unit Of Work Pattern for laravel. Just like doctrine's EntityManager. I am planning to create one.

RicardasRimsevicius commented 7 years ago

+1

DevDaveo commented 7 years ago

+1

joshmanders commented 7 years ago

Github has a thumbs up/down feature, no need to spam us with +1 comments.

imalhasaranga commented 7 years ago

Do you guys see any problem of this, because I feel this is the closest eloquent solution at the moment

        $allintests = [];
        foreach($intersts as $item){
            $intestcat = new User_Category();
            $intestcat->memberid = $item->memberid;
            $intestcat->catid= $item->catid;
            $allintests[] = $intestcat->attributesToArray();
        }
        User_Category::insert($allintests);
atahrijouti commented 6 years ago

@cabloo how come you are not getting this error ?

[Symfony\Component\Debug\Exception\FatalThrowableError]  
  Using $this when not in object context
pixeliner commented 6 years ago

Problem with insert([]) is that it doesn't add any timestamps. Is there a reason to why? thx

Franclaf7 commented 6 years ago

Is there a way to bulk insert with ::insert(array[]) and to tell Laravel to ignore one one the arrays contained in the major array[] if there's any error? For example, if i want to insert the records into the DB: DB::table('products')->insert(array( array("id"=>'1',"name"=>"expertphp","details"=>"Learning Portal"), array("id"=>'2',"name"=>"demo.expertphp.in","details"=>"Find running example"), ); And there's an error with the second array ("id"=2), but I want to ignore it and still insert the first array, is there any way of doing this?

(First comment ever on here, didn't know if I had to start a new thread or what, thanks)

mityukov commented 6 years ago

Any option to make ::saveAll() kind of stuff on an Eloquent collection?

Say, I extract a collection of SMS messages from the database, check statuses of these messages and then update them all using as few DB requests as possible.

public static function checkStatus(string $apiId, Collection $smsLogs)
{
        // 2. check statuses of these messages
        $apiStatuses = self:getStatusesFromApi($apiId, $smsLogs->pluck('resp_id')->toArray());

        // 3. fill-in status-related fields of the models:
        foreach ($smsLogs as $log) {
            $log->status_code = $apiStatuses->{$log->resp_id};
            $log->is_delivered = self::isMessageDelivered($log);
        }
        $smsLogs->saveAll();

        return $smsLogs->count();
}
cabloo commented 6 years ago

@mityukov as far as I am aware, you cannot do that in a single UPDATE query in SQL, you can only update multiple rows at once if you are setting the same values on each row. You could do some smart grouping on the data before-hand to accomplish the minimum number of queries.

mityukov commented 6 years ago

@cabloo in MySQL this can be done via "INSERT … ON DUPLICATE UPDATE …". On other DBs, well…, if not supported on DB level — then group and make several queries. Just hide implementation details from the end-programmer ;-)