[Multi-stage] Clean up unnecessary checks in rules

[Jackie-Jiang]

• Remove unnecessary checks in matches()
• Use copy() instead of create() if we are modifying a RelNode to preserve the hints

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 93.33333% with 5 lines in your changes missing coverage. Please review.

Project coverage is 65.02%. Comparing base (59551e4) to head (a740dc8).
Report is 1080 commits behind head on master.

Files with missing lines	Patch %	Lines
...alcite/rel/rules/PinotExchangeEliminationRule.java	33.33%	2 Missing ⚠️
...rel/rules/PinotSingleValueAggregateRemoveRule.java	71.42%	2 Missing ⚠️
...ite/rel/rules/PinotJoinExchangeNodeInsertRule.java	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14066      +/-   ##
============================================
+ Coverage     61.75%   65.02%   +3.27%     
- Complexity      207     1533    +1326     
============================================
  Files          2436     2564     +128     
  Lines        133233   140692    +7459     
  Branches      20636    21592     +956     
============================================
+ Hits          82274    91482    +9208     
+ Misses        44911    42491    -2420     
- Partials       6048     6719     +671     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (+99.99%)	⬆️
integration	100.00% <ø> (+99.99%)	⬆️
integration1	100.00% <ø> (+99.99%)	⬆️
integration2	0.00% <ø> (ø)
java-11	64.98% <93.33%> (+3.27%)	⬆️
java-21	64.91% <93.33%> (+3.29%)	⬆️
skip-bytebuffers-false	65.02% <93.33%> (+3.27%)	⬆️
skip-bytebuffers-true	64.89% <93.33%> (+37.16%)	⬆️
temurin	65.02% <93.33%> (+3.27%)	⬆️
unittests	65.01% <93.33%> (+3.27%)	⬆️
unittests1	56.51% <93.33%> (+9.62%)	⬆️
unittests2	35.03% <14.66%> (+7.29%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gortiz]

Nit: I would recommend to start using the new way to instantiate rules instead of this one, which is deprecated. See PinotImplicitTableHintRule in #13943.

[Jackie-Jiang]

Yes, I also noticed that. PinotSortExchangeCopyRule is also using the config in the constructor (I believe the benefit is to allow reusing the same rule to match different trees).
We can use a separate PR to clean this up.

[gortiz]

I've recently ended up reading Hive's CalcitePlanner class, which is pretty similar to our QueryEnvironment class and I think that is something we can get inspiration of once our rule logic starts to be more complex.