Count on a association of an unsaved item should raise #1856

Closed
gucki opened this Issue Jun 25, 2011 · 16 comments

Projects

None yet

10 participants

@gucki

Consider the following code

class Account
  has_many :dossiers
end

class Dossier
  belongs_to :account
end


a = Account.new
a.dossiers.build

a.dossiers.count
SELECT COUNT(*) FROM "dossiers" WHERE "dossiers"."account_id" IS NULL
=> 0

a.dossiers.size
=> 1

a.dossiers.length
=> 1

I'd suggest that doing a count on an association of an unsaved parent should raise something like "Parent not saved yet!". Otherwise the result seems surpring and can easily lead to errors. Especially when there are them dossiers in the database which have account_id set to nil.

@nragaz

This seems to be a recent change in behaviour.

Since about a week ago I've been seeing this when creating new parent records and then counting their children. That used to return 0, but now it will count any un-associated child records in the database.

(I'm using HEAD on 3-1-stable. I tried to track down the specific commit that caused the change but without success.)

@tenderlove
Ruby on Rails member

I have two thoughts on this:

  1. I'm not sure that we should raise an exception. People could (and probably do) depend on this functionality.

  2. If we know the record is new, we should just return 0 without querying. This is not backwards compatible from the standpoint that someone's database could have NULLable foreign keys, but I don't think we should support that.

@nragaz

@tenderlove Agree. I think that makes more sense.

@gucki

@tenderlove I agree with option 2. :)

@colszowka

I only recently stumbled upon this issue, it's quite hard to diagnose as I had a bit of code that only continued operating when the record did not have any associated records - relying on count.

Even though option 2 might not be backwards compatible, I believe it gives a much more expected behaviour then the current one :)

@isaacsanders

Is this still an issue? Any thoughts on this?

@colszowka

I still think this should be fixed in one way or another, and don't believe it has been already.

@isaacsanders

Does length perform correctly?

@vendethiel

+1 for returning 0 without querying if the primary key is null

@colszowka

@isaacsanders If the behaviour has not changed from the original example posted by @gucki - Yes.

@nsolter

Note that this is a general bug affecting more than just count. See #5215

@colszowka

@nsolter In the light of that ticket I would suggest this to be closed and discussion to be moved to #5215 as it also covers the wider scope of the problem (even though obviously this one here came much earlier to the party :)

@isaacsanders

+1 for closing and focusing on the larger issue.

@henrik

Just ran into this as well. Considering this as a workaround:

class Category
  has_many :items, conditions: "category_id IS NOT NULL"
end
@frodsan

👍 @isaacsanders yup, we should close it and focus on #5215. /cc @carlosantoniodasilva @rafaelfranca

@frodsan frodsan pushed a commit that referenced this issue Oct 3, 2012
Francesco Rodriguez Count returns 0 without querying if parent is not saved
Patches `CollectionAssociation#count` to return 0 without querying
if the parent record is new. Consider the following code:

    class Account
      has_many :dossiers
    end

    class Dossier
      belongs_to :account
    end

    a = Account.new
    a.dossiers.build

    # before patch
    a.dossiers.count
    # SELECT COUNT(*) FROM "dossiers" WHERE "dossiers"."account_id" IS NULL
    # => 0

    # after
    a.dosiers.count # fires without sql query
    # => 0

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