Allow relations with different SQL comments in the or method

[sinsoku]

An error occurs when you pass a relation with SQL comments to the or method.

class Post
  scope :active, -> { where(active: true).annotate("active posts") }
end

Post.where("created_at > ?", Time.current.beginning_of_month)
  .or(Post.active)
#=> ArgumentError (Relation passed to #or must be structurally compatible. Incompatible values: [:annotate])

In order to work without ArgumentError, it changes the or method to ignore SQL comments in the argument.

Ref: #38145 (comment)

[kamipo]

How do you think optimizer_hints should behave?

[sinsoku]

Sorry, I haven't used optimizer_hints yet, so I'm not very familiar with it.
I don't know how to behave in case of conflicting conditions.

Post.optimizer_hints("MAX_EXECUTION_TIME(50000)").or(
  Post.optimizer_hints("MAX_EXECUTION_TIME(20000)"))
#=> ?

[sinsoku]

I found the following description in the MySQL documentation.

When a hint comment contains multiple hints, the possibility of duplicates and conflicts exists. The following general guidelines apply. For specific hint types, additional rules may apply, as indicated in the hint descriptions.

• Duplicate hints: For a hint such as /*+ MRR(idx1) MRR(idx1) */, MySQL uses the first hint and issues a warning about the duplicate hint.
• Conflicting hints: For a hint such as /*+ MRR(idx1) NO_MRR(idx1) */, MySQL uses the first hint and issues a warning about the second conflicting hint.

ref: https://dev.mysql.com/doc/refman/5.7/en/optimizer-hints.html

The query when the condition conflicts is valid (although there is a warning), so I think that it is no problem to simply combine the values.
I would like to support optimizer_hints in this PR.

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[sinsoku]

I don't come up with a use case for using optimizer_hints with the or method, so this PR only covers annotate.
If someone wants to use optimizer_hints with the or method, they will make a PR.

[kamipo]

To work or without ArgumentError in that case, is not enough just ignore annotate/optimizer_hints?

or is focused to combine where-like clauses by not AND but OR.
Allowing (and defining) to combine non where-like clauses is the notable design change for or to me.

[sinsoku]

Sure, that's enough.
I'll fix it.

[kamipo]

I'm not excited to talk about the annotate and the optimizer_hints in the same breath with the where and the having.

We had hit the structually incompatible error on several occasions in the past (#29461, ea61391), in that time, we just relaxed the limitation to address the error, but didn't mention about what values are allowed/ignored.

I agree that there is room for improvement in the doc, but how about discussing it in another PR about how to improve it?

[sinsoku]

Thank you for telling me related pull requests.
I understood.

I reverted the changes in doc and force-pushed.

[kamipo]

The quoted_table_name should be escaped by Regexp.escape, since it is qouted as [posts] in SQLServer.

Suggested change

	quoted_posts = Post.quoted_table_name
	quoted_posts = Regexp.escape(Post.quoted_table_name)

[sinsoku]

Thanks for your review. I updated.

[kamipo]

Thanks!