update_columns doesn't support an empty hash

[Michael Mahemoff]

The following will return an error:

    > Post.first.update_columns({})

    ArgumentError: Empty list of attributes to change

I think that's surprising, because I see update_columns as an analogue to update_attributes, just without callbacks happening. An empty hash for the latter is fine:

    > Post.first.update_attributes({})

    true

So shouldn't update_columns support an empty hash too? In the rare cases where callbacks should be avoided and this is needed, it would save having to make a special-case check to prevent the error.

[Rafael Mendonça França]

Are not both cases invalid input? Why should we accept empty hashes?

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Michael Mahemoff]

Basically because http://en.wikipedia.org/wiki/Null_Object_pattern. It's often the case that systems have to deal with "N" possible changes, where N is between 0 and 1. e.g. I might have a procedure to gradually build up a list of things to change, starting with {} and appending to it if certain conditions are met. It wouldn't be an exceptional situation if none of those conditions are met.

[Michael Mahemoff]

Bah I mean "N is between 0 and <many>" (rather than 1 and <many>).

[Xavier Noria]

It makes sense to me, you get passed a hash with the attributes to update, which could be none.

This is a no-op edge-case similar to append an empty list to a list, etc., and could have use cases where the hash is built programatically.

The implementation generally uses iterators, which is the normal way to support these edge-cases with no explicit handling, maybe the exception is raised by update_all(attributes)

    https://github.com/rails/rails/blob/master/activerecord/lib/active_record/persistence.rb#L312

?

[Rafael Mendonça França]

And what happen if users expect N to be between 1 and <many> and we silently do nothing when 0 is sent to these methods?

In my opinion if you don't have changes to be made in the database, just don't call the method.

[Rafael Mendonça França]

This behavior was added at https://github.com/rails/rails/pull/7133.

Also don't assume that `update_attributes` and `update_columns` are analogues. The later is dealing directly with the database without any kind of interaction with the object instance, so they will behaves differently in a lot of ways, like type casting for instance.

[Rafael Mendonça França]

Yes. It is raised by `update_all(attributes)`.

[James Coleman]

I think the error makes sense with update_columns and not with update_attributes. Since the former is essentially a pass through to the database, and the database would raise an error without any columns to update, I'd expect an error when the method is used that way as well.