Skip to content

[CALCITE-7549] Add Programs.of overload that accepts RelOptListener#4968

Open
shikhae6 wants to merge 1 commit into
apache:mainfrom
shikhae6:CALCITE-7549
Open

[CALCITE-7549] Add Programs.of overload that accepts RelOptListener#4968
shikhae6 wants to merge 1 commit into
apache:mainfrom
shikhae6:CALCITE-7549

Conversation

@shikhae6
Copy link
Copy Markdown

@shikhae6 shikhae6 commented May 25, 2026

Jira Link

CALCITE-7549

Changes Proposed

When multiple HEP phases are composed via Programs.sequence, each
phase constructs its HepPlanner internally with no hook to attach a
RelOptListener. This prevents observing rule firings across sequenced
optimization phases — a requirement for query optimizer tracing and
observability tooling.

This PR adds a new overload to Programs:

public static Program of(HepProgram, boolean noDag,
    RelMetadataProvider, @Nullable RelOptListener listener)
                                            
Behavior:                               
- If listener is non-null, it is attached via HepPlanner.addListener()
before setRoot / findBestExp.                                                                                                                            
- If listener is null, the method delegates directly to the existing
Programs.of(HepProgram, boolean, RelMetadataProvider) — zero behavior                                                                                    
change, zero overhead for current callers.  
                                        
Use case:
Production query optimizers built on Calcite structure optimization as                                                                                   
multiple HEP phases via Programs.sequence. To build a unified trace of
which rules fire, in which phase, and in what order, a shared                                                                                            
RelOptListener must be attached to each HepPlanner. Without this
overload the only alternatives are bypassing Programs.of entirely
(losing its metadata/materialization/lattice setup) or using reflection.                                                                                 
                                        
Backward compatibility:                                                                                                                                  
- The existing three-argument Programs.of is unchanged.                                                                                                  
- No changes to the Program interface or HepPlanner.   
- Null listener path delegates to the existing method.                                                                                                   
                
Summary                                     
                                        
- Added Programs.of(HepProgram, boolean, RelMetadataProvider, RelOptListener)
overload in core/src/main/java/org/apache/calcite/tools/Programs.java.                                                                                   
- The new method mirrors the existing Programs.of behavior exactly, with
a single addition: attaching the provided listener to the HepPlanner                                                                                     
before execution.                           
- Expanded star import org.apache.calcite.plan.* to explicit imports to                                                                                  
satisfy checkstyle.                                                                                                                                      
- Added @Nullable annotation (from org.checkerframework.checker.nullness.qual)                                                                           
to the listener parameter.                                                                                                                               
- Added @SuppressWarnings("deprecation") consistent with the existing                                                                                    
Programs.of method (for registerMetadataProviders).
                                                                                                                                                         
Testing                                 
                                                                                                                                                         
Two new tests in HepPlannerTest:                                                                                                                         
                                                                                                                                                         
- testProgramsOfWithListenerverifies the listener receives                                                                                            
ruleAttempted events when rules fire through Programs.of.
- testProgramsOfWithNullListenerverifies null listener is safely
ignored and produces a valid result.    

Reference XML (HepPlannerTest.xml) updated with new test case entries.                                                                                   
                                        
./gradlew :core:build passes.                                                                                                                            
                                                                                                                                                         
---
                                                                                                                                                         
**Commit message:**
[CALCITE-7549] Programs.of should allow attaching a RelOptListener to the internal HepPlanner

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@mihaibudiu mihaibudiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably merge this after the 1.43 release

* <p>If {@code listener} is not null, it is
* {@linkplain HepPlanner#addListener(RelOptListener) attached}
* to the planner before execution. */
@SuppressWarnings("deprecation")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be done without calling deprecated functions?

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label May 25, 2026
@julianhyde
Copy link
Copy Markdown
Contributor

I raised some concerns in the jira case.

@mihaibudiu mihaibudiu added discussion-in-jira There's open discussion in JIRA to be resolved before proceeding with the PR and removed LGTM-will-merge-soon Overall PR looks OK. Only minor things left. labels May 26, 2026
@xiedeyantu
Copy link
Copy Markdown
Member

@shikhae6 Do you think this PR is still needed?

@shikhae6
Copy link
Copy Markdown
Author

shikhae6 commented Jun 5, 2026

@xiedeyantu I have proposed an alternate way in Jira , awaiting @julianhyde view on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion-in-jira There's open discussion in JIRA to be resolved before proceeding with the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants