Manage composite primary key

[Arkanys]

Overriding of ActiveRecord method "quote_column_name" to manage multiple column names. When the project integrate the gem "composite-primary-keys" [1], the returned primary key is an array. (refs #13)

[1] https://github.com/composite-primary-keys/composite_primary_keys

[vjt]

My concerns about this are that we are overriding here in ChronoModel a piece of ActiveRecord due to a requirement of another gem. Wouldn't it be more correct that this quote_column_name override be done in composite_primary_key itself?

@codeodor, @darrinholst what do you think?

[codeodor]

I think it would belong in CPK more than it would belong here. But I'm not sure why CPK doesn't have it in the first place.

@Arkanys what happens if you add that to CPK and try to run the specs?

I'm not saying it'd be accepted as a patch, because I don't know why it's not there (and I only contributed a couple of patches to the project myself), but IMO it would belong w/ CPK.

[vjt]

@codeodor thanks! I included you and @darrinholst because you were the two top commit contributors to CPK :smiley:

[Arkanys]

In CPK, the method quote_column_name is already done in AbstractAdapter, and PostgresqlAdapter is overrided too. But as we define the adapter: chronomodel in the config file, it seems none of these changes are applied.

[codeodor]

Wow, I had no idea. :smile: I think it's probably because when I updated it to work for Rails 4, we should have rebased all my commits into one before merging it, but I didn't.

[codeodor]

@Arkanys Note that method is plural, quote_column_names not quote_column_name

Do both gems override insert_sql and sql_for_insert ?

[codeodor]

Let me ask a follow up question: can you give the stack trace of what it looks like without this patch? That may give an insight into another way to fix it.

[vjt]

@codeodor FYI, ChronoModel does not override insert_sql :smiley_cat:

EDIT: and neither sql_for_insert

[codeodor]

I really should have just looked via search before I asked. :smile:

But I wanted to know basically where is it failing.

It looks like CPK only use that pluralized quote_column_names in the two insert methods I mentioned above. I wondered where ChronoModel is failing due to CPK, to see if there's a way to fix it in CPK without overriding the singular quote_column_name

(since I don't know why it wasn't done in the first place, I just want a backup plan ... I'm assuming there was a reason it's not overridden)

[tagliala]

While we appreciate the contribution of this feature, we've decided to close this pull request at this time. Our current focus is on maintaining a minimal footprint for Chronomodel to minimize the risk of unexpected issues.

However, we're still open to considering this feature in the future. If there's continued interest, feel free to submit a new pull request with detailed specifications that address our concerns about code size and potential complexity. Please remember to add new specs