Tapioca v0.17.9 generates incorrect signatures for ActiveRecord `#upsert` / `#upsert_all`

[olivier-thatch]

#2421 updated the ActiveRecord relations compiler to generate signatures for #upsert and #upsert_all (as well as many other methods). Howver, the signatures are incorrect:

    sig do
      params(
        attributes: Hash,
        returning: T.nilable(T.any(T::Array[Symbol], FalseClass)),
        unique_by: T.nilable(T.any(T::Array[Symbol], Symbol))
      ).returns(ActiveRecord::Result)
    end
    def upsert(attributes, returning: nil, unique_by: nil); end

    sig do
      params(
        attributes: T::Array[Hash],
        returning: T.nilable(T.any(T::Array[Symbol], FalseClass)),
        unique_by: T.nilable(T.any(T::Array[Symbol], Symbol))
      ).returns(ActiveRecord::Result)
    end
    def upsert_all(attributes, returning: nil, unique_by: nil); end

There are multiple issues:

• The signatures are missing some valid arguments: on_duplicate, update_only, and record_timestamps (cf. Rails docs and source)

• returning is typed as T.nilable(T.any(T::Array[Symbol], FalseClass)), but it can also be a String or a T::Array[String]. Some examples:

  Commodity.upsert_all(
    [
      { id: 2, name: "Copper", price: 4.84, created_at: Time.now, updated_at: Time.now  },
      { id: 4, name: "Gold", price: 1380.87, created_at: Time.now, updated_at: Time.now  },
      { id: 8, name: "Steel", price: 1.35, created_at: Time.now, updated_at: Time.now  }
    ],
    returning: Arel.sql("id, (xmax = '0') as inserted, name as new_name") # Arel.sql returns a String
  )
  
  Commodity.upsert_all(
    [
      { id: 2, name: "Copper", price: 4.84, created_at: Time.now, updated_at: Time.now  },
      { id: 4, name: "Gold", price: 1380.87, created_at: Time.now, updated_at: Time.now  },
      { id: 8, name: "Steel", price: 1.35, created_at: Time.now, updated_at: Time.now  }
    ],
    on_duplicate: Arel.sql("price = GREATEST(commodities.price, EXCLUDED.price)"),
    returning: ["price"] # Need to use String to reference dynamic SQL "columns"
  )

I'm also curious why these signatures were added to the compiler in the first place. Since they don't reference the model class, it seems like they could be omitted from the compiler and handled via annotations for ActiveRecord instead.

[paracycle]

Hi @olivier-thatch. Yes, you are correct on both points.

I was fixing insert related methods and realized that insert_all and friends were never being generated, but didn't pay attention to the fact that neither the inputs nor the output was making a reference to the model it is defined on.

So, yes, upsert_all signature was incorrect, since I copied it from insert_all (which takes less arguments) and, also, neither of them need to be generated by this compiler.

We will remove them from the compiler for the next release. Apologies for the noise.

[lasseebert]

I'm not sure if it's related, but with the recent update, we also get delete_all on CommonRelationMethods. I think this is fine, but PrivateCollectionProxy should override it to also accept an argument.

The CommonRelationMethods method is (correctly) defined as

sig { returns(Integer) }
def delete_all; end

The one on PrivateCollectionProxy accepts a dependent argument:

def delete_all(dependent = nil)

Ref the docs.

Since PrivateCollectionProxy includes CommonRelationMethods calls to delete_all(dependent) are now marked as type errors.

Is this related or should I create another issue?

Thanks!

[crespire]

Is there a manual fix I can apply in the meantime?

[lasseebert]

Is there a manual fix I can apply in the meantime?

I only had the problem in a few places, so I just ignored it with T.unsafe and a comment with a link to this issue 😁

I guess you could add some shims to cover multiple similar issues in one go.

[crespire]

Yeah, I'll have to T.unsafe for now then. Thanks!