Add new Rails/OrderById cop

[fatkodima]

This is an implementation of https://rails.rubystyle.guide/#order-by-id

[andyw8]

I'm a little wary of this because there are likely some legitimate uses of ordering by ID - it's just not appropriate when you need chronological ordering.

[fatkodima]

Sure, there are, but i think, and as i saw in the past, almost always the author thought about chronological ordering when used constructions like this.

[koic]

I agree with @andyw8's opinion and it seems like query plan may change. I'm worried about proposing this by default because it seems a bit aggressive.

[fatkodima]

Made it disabled by default and added a note about possible performance changes.

[koic]

Thanks!

[kitsunde]

Sorry if I'm turning this into a discussion I was just going through the changelog and reading the concerns. I want to add context for the future if this comes up again and highlight introducing a subtle bug.

Ordering by id is very common as databases do not guarantee records are returning in any particular order, in fact, the current good example of swapping ORDER BY id to ORDER BY created_at will cause a subtle bug where this inconsistency happens when the created_at is the same for two records, causing them to appear to randomly swap places between queries. Combine that with commonplace operations like updating the .last record and you can see how this can lead to surprises.

So the use case for ORDER BY id isn't ordering records by time, it's stable sorting for a situation like pagination, I think the ruby style guide itself might not understand this issue in depth.

Timestamp columns themselves don't really offer any guarantees of truth either only as a matter of display. Since they are in rails commonly set by application servers not perfectly in sync, db calls arrive and execute at different speeds and NOW() is frozen to the first invocation inside of a transaction across the duration of the active transaction (in pg anyways). I think this style guide referenced is mixing up presentation layer concerns like showing an ordered series of times to the user - which is situational - with general application-level concerns.