[CALCITE-4385] Extracts and eliminates/merges the condition that has common expressions (Jiatao Tao)

[Aaaaaaron]

Simplifies AND/OR condition that has common expressions, extract and eliminate/merge them.

1. (a || b) && (a || b || c || d) => (a || b)

2. (a || b || c || d) && (a || b || e || f) => (a || b) || ((c || d) && (e || f))

3. (a && b) || (a && b && c) => (a && b)

4. (a && b && c && d) || (a && b && e && f) => (a && b) && ((c && d) || (e && f))

The difference between "implifyAnd" is that "simplifyAnd" mainly simplifies expressions whose answer can be determined without evaluating both sides, like: FALSE AND (xxx) => FALSE

[julianhyde]

I have a bad feeling about this change. It adds a lot of new code and does not leverage existing code (e.g. reasoning based on predicates). It is going to be a nightmare to maintain.

[Aaaaaaron]

I have a bad feeling about this change. It adds a lot of new code and does not leverage existing code (e.g. reasoning based on predicates). It is going to be a nightmare to maintain.

Hi, @julianhyde, what do you mean "does not leverage existing code(e.g. reasoning based on predicates)"? Current I just add a utility to simplifies AND/OR condition that has common expressions, it belongs to CALCITE-4375, and later we can think to add this logic to RexSimplify's main process or just used to simplify the join condtition.

In fact, the code is not so much, the real code is no more than 20 lines and there's UT to guarantee this logic.

[Aaaaaaron]

Recently I'm really busy, sorry for the slow progress, will restart soon.

[Aaaaaaron]

@rubenada Hi, could you help review it after updating~

[Aaaaaaron]

@rubenada I've updated my PR, could you take a look, thanks a lot?

[rubenada]

@Aaaaaaron thanks, great work. I have left some minor final details, otherwise LGTM

[Aaaaaaron]

@Aaaaaaron thanks, great work. I have left some minor final details, otherwise LGTM

@rubenada All revised, thanks for your careful review, really appreciate it!

[rubenada]

You're welcome @Aaaaaaron , thanks for your work. @vlsi you reviewed this PR some time ago, I think now it is in a pretty good shape, do you want to take a final look?

[Aaaaaaron]

@vlsi Thanks for your nice review, I update my PR, here a separate commit for your to review again, thanks again! https://github.com/apache/calcite/pull/2253/commits/b22496b3536a4c789a676688d94b2c98be011604 @rubenada May you also review again? Thanks!

[Aaaaaaron]

Hi @rubenada, could we merge this PR if there are no more comments?

[vlsi]

Can we please set a hard limit on the maximum number of back-and-forth, and close/reject PRs that exceed it?

I see the PR does not meet code/performance quality, and I have no time to review it. I unsubscribe.

[Aaaaaaron]

Can we please set a hard limit on the maximum number of back-and-forth, and close/reject PRs that exceed it?

I see the PR does not meet code/performance quality, and I have no time to review it. I unsubscribe.

@vlsi @julianhyde very sad to hear this, really aggressive, some of your review comment is great and I've worked hard to revise them, no one's code is perfect, if you think it is still not meet code/performance, show the evidence, and please show more respect!

[julianhyde]

@Aaaaaaron, We are showing respect. But we are volunteers, and reviewing changes takes time away from other work we could be doing. This conversation has 84 entries. Every one of them is 10 - 15 minutes of someone's time.

Maybe this would benefit from a reboot. Re-title the JIRA, re-state your value proposition, create a new PR.

[Aaaaaaron]

@Aaaaaaron, We are showing respect. But we are volunteers, and reviewing changes takes time away from other work we could be doing. This conversation has 84 entries. Every one of them is 10 - 15 minutes of someone's time.

Maybe this would benefit from a reboot. Re-title the JIRA, re-state your value proposition, create a new PR.

@julianhyde Thanks, I'll create a new PR.

[Aaaaaaron]

@Aaaaaaron, We are showing respect. But we are volunteers, and reviewing changes takes time away from other work we could be doing. This conversation has 84 entries. Every one of them is 10 - 15 minutes of someone's time.

Maybe this would benefit from a reboot. Re-title the JIRA, re-state your value proposition, create a new PR.

@julianhyde Really appreciate your work about code review, thanks, the main point is I welcome actual advice, but not just complaining, what's the point of telling you are wrong but don't tell you what's your mistake? These are total useless information and all negative:

---

---

@vlsi If you have no time to review, then do not review, if you want to review, I really appreciate it, but please do leave the valuable info.

[vlsi]

@Aaaaaaron ,

1. I have no time here. No details, sorry.
2. The comments (e.g. "code/performance", "checkDuplicateSubset") are helpful for other reviewers and committers so they don't accidentally commit the change without a proper review.

[Aaaaaaron]

@Aaaaaaron ,

1. I have no time here. No details, sorry.
2. The comments (e.g. "code/performance", "checkDuplicateSubset") are helpful for other reviewers and committers so they don't accidentally commit the change without a proper review.

If you leave this and not update no more, it's also irresponsible, this means this will not get merged no matter what the author does. If you have no time, just quit, other committers/PMCs do have their own judgment and thoughts, you are not the only one who knows how to coding; If you want to leave something, please give details and follow them.