New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated_at not fully qualified, creates ambiguous column errors #13909

Closed
bryceray1121 opened this Issue Jun 7, 2016 · 31 comments

Comments

Projects
None yet
@bryceray1121
Copy link
Contributor

bryceray1121 commented Jun 7, 2016

When attempting to use delete() or update() on an eloquent model you can not use any joins because you will get an ambiguous column name error on updated_at. This is because the Eloquent/Builder.php does not prefix updated_at with the proper table name. I attempted to do a pull request for this, but the issue is further complicated by Arr::add (used in addUpdatedAtColumn) doing automatic dot notation exploding. This prevents me from adding a table.columnName string similar to the operation of SoftDeletes trait.

Laravel 5.2

@srmklive

This comment has been minimized.

Copy link
Contributor

srmklive commented Jun 8, 2016

You can disable $timestamps on specific models by doing this: public $timestamps=false.

One question though, why are you using join when updating or deleting a row? Can you describe in detail the exact scenario?

@themsaid

This comment has been minimized.

Copy link
Member

themsaid commented Jun 17, 2016

There was an attempt to fix a similar issue: #13519
But end up causing problems with Postgres and also sqlite: #13707

So the fix was reverted: d12c4bb

I don't think there'll be an easy solution for this as using a fully qualified column name table.column has different rules between drivers which requires having different implementations.

@troygilbert

This comment has been minimized.

Copy link

troygilbert commented Jun 23, 2016

This seems like a pretty major issue. Maybe I'm using Eloquent wrong, but with just a simple use of Eloquent like this: $model->childRelationship()->update(['some_column' => $value]); fails.

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Jul 15, 2016

Yeah it is definitely an issue. The commit referenced by @themsaid was mine, and while I haven't had the time to revisit this issue I am going to devote the time to look for a fix regardless. Anyone who wants to work together to find a solution, you can find contact info on my profile. Really determined to get this resolved.

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Jul 15, 2016

The only thing that comes to the top of my head is in the Eloquent Model's getQualifiedUpdatedAtColumn method, checking which driver is being used and determine the value that way.

The more long-term way would probably be defining the Eloquent Builder as an abstract class and then each driver has its own implementation of the addUpdatedAtColumn method. Of course, that would require Laravel to determine the proper Builder at runtime.

@themsaid

This comment has been minimized.

Copy link
Member

themsaid commented Sep 11, 2016

@samrap Did you get the chance to open a PR?

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Sep 12, 2016

@themsaid Unfortunately not but I'd be happy to work together with you in order to develop a solution. From what I've discovered with this bug it's a much larger issue than it sounds.

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Sep 12, 2016

@themsaid I don't have experience outside of MySQL but if I get a list of the different qualified column name rules between drivers I think I can handle fixing this issue on my own.

@baona95

This comment has been minimized.

Copy link

baona95 commented Oct 12, 2016

This seems to be an edge case and there hasn't been a fix yet.
But we can always attempt to solve with the Query Builder. Join the tables and manually set deleted_at and updated_at.

@themsaid

This comment has been minimized.

Copy link
Member

themsaid commented Oct 13, 2016

@trandaihung that's what I personally do, I tried to fix before but every database driver has different rules for the qualified column name which makes this change pretty major.

@makoru-hikage

This comment has been minimized.

Copy link

makoru-hikage commented Jan 3, 2017

Aw, I've to use Carbon::now()

@iateadonut

This comment has been minimized.

Copy link

iateadonut commented Jan 18, 2017

Can a method ->withoutTimestamps() be added for this particular case?

@JordanDalton

This comment has been minimized.

Copy link

JordanDalton commented Feb 3, 2017

I've ran into this issue today as well.

@walterdeboer

This comment has been minimized.

Copy link

walterdeboer commented Jun 1, 2017

@srmklive the join in my case is generated because I have a belongsToMany relation in my model. When soft-deleting the model, the relation also gets soft-deleted.

@samrap I noticed however the deleted_at column is properly qualified in the delete query, where the updated_at column isn't. Is this the also the case for other scenario's? Why isn't updated_at qualified the same way as the deleted_at column?

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Jun 1, 2017

@walterdeboer My PR to fix the issue explains what is going on in-depth: #13519

Unfortunately, as @themsaid mentioned above, the PR caused a problem when using the Postgres and Sqlite drivers.

@decadence

This comment has been minimized.

Copy link
Contributor

decadence commented Jul 27, 2017

Have this issue as well

@jtwiegand

This comment has been minimized.

Copy link

jtwiegand commented Aug 21, 2017

I'm able to sidestep the ambiguity of the updated_at columns in Laravel 5.3 with a toBase() call on the Eloquent builder before doing an update.

@Turaylon

This comment has been minimized.

Copy link

Turaylon commented Sep 1, 2017

Same problem here. As a workaround had to pluck the ids for the result that i want to update with a select and then update the model with the given ids.

    $ids = User::filter($filter)->pluck('id'); //in filter i have  a join with an other model that has timestamps
    User::whereIn('id',$ids)->update(['field' => 'value']);
@pr4xx

This comment has been minimized.

Copy link

pr4xx commented Sep 21, 2017

Same issue here

@themsaid themsaid added the bug label Oct 26, 2017

@iget-esoares

This comment has been minimized.

Copy link
Contributor

iget-esoares commented Oct 30, 2017

This issue is affecting be on L5.4.36, and also should affect L5.5

$query->join(/* join some table with updated_at */)
->delete()

This will bring ambiguous column error for updated_at

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Oct 31, 2017

We're all well aware this issue exists. Can we refrain from unconstructive comments of the type "same thing happening here" from this point forward? There are lots of people subscribed to these issue threads that don't need to be getting emails every time someone reconfirms this is a bug 😬

@f4usto

This comment has been minimized.

Copy link

f4usto commented Jan 30, 2018

I am using the L5.4.36 and the workaround provided from @Turaylon is not feasible as it could get on the millions of ids on the current project and also the update values come from other tables that will be deprecated

I ended up using a ReflectionClass to "disable" the timestamps for the model on the source model for the query builder:

$instance = new \App\Models\Model;
$reflection = new ReflectionClass(\App\Models\Model::class);
$reflection->getProperty('timestamps')->setValue($instance, null);

$instance->/* wheres(), joins() and so on */->update([/* update values */])

This is not a long term approach or solution, but it bypass the timestamp modification. by not updating that field at all.

A driver aware implementation (or an alternative to the array splitting) of the addUpdatedAtColumn would be appreciated.

@Polokij

This comment has been minimized.

Copy link

Polokij commented Feb 22, 2018

I've used method toBase() before update
$someMode->relations()->someScope()->toBase()->update(['relationTable.deleted_at'=> now(), 'relationTable.updated_at' => now()]);
And it works for me.

@slaughter550

This comment has been minimized.

Copy link
Contributor

slaughter550 commented Apr 19, 2018

@samrap I'm happy to help you solve this problem if you're still looking to solve it. I would also be hesitant about the function getUpdatedAtColumn knowing about the driver as that seems like an implementation detail that each driver should know how to FQ a property. Something that might be of note in this case is how each driver references properties on a join. I could see putting something like the addUpdatedAtColumn method a few levels deeper either in the Query Builder or at the Grammar level even. I would have to dig more but I would be keen on solving this issue with you if you're interested?

@samrap

This comment has been minimized.

Copy link
Contributor

samrap commented Apr 19, 2018

@slaughter550 I would definitely be interested in working together on a solution. I'll shoot you an email.

@tnatanael

This comment has been minimized.

Copy link

tnatanael commented Sep 13, 2018

Its still happening, with Lumen 5.5 any news?

@staudenmeir

This comment has been minimized.

Copy link
Contributor

staudenmeir commented Sep 29, 2018

#22366 updated the SQLiteGrammar to remove the table name from qualified columns when an UPDATE query is compiled.

If we apply the same behavior to the PostgresGrammar, couldn't we resubmit @samrap 's PR (#13519)?

@amenk

This comment has been minimized.

Copy link
Contributor

amenk commented Oct 8, 2018

Sound's good to me, @staudenmeir can you make such a PR ?

@staudenmeir

This comment has been minimized.

Copy link
Contributor

staudenmeir commented Oct 10, 2018

Will be fixed in Laravel 5.8: #26031

@laurencei

This comment has been minimized.

Copy link
Member

laurencei commented Oct 12, 2018

Thanks @staudenmeir

@laurencei laurencei closed this Oct 12, 2018

@patieru12

This comment has been minimized.

Copy link

patieru12 commented Feb 11, 2019

I'm able to sidestep the ambiguity of the updated_at columns in Laravel 5.3 with a toBase() call on the Eloquent builder before doing an update.

This works very nice for me. I am using Laravel 5.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment