waynexia commented on code in PR #4645:
URL: https://github.com/apache/arrow-datafusion/pull/4645#discussion_r1050778244


##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -54,33 +55,53 @@ pub trait OptimizerRule {
     fn try_optimize(
         &self,
         plan: &LogicalPlan,
-        optimizer_config: &mut OptimizerConfig,
+        config: &dyn OptimizerConfig,
     ) -> Result<Option<LogicalPlan>> {
-        self.optimize(plan, optimizer_config).map(Some)
+        self.optimize(plan, config).map(Some)
     }
 
     /// Rewrite `plan` to an optimized form. This method will eventually be 
deprecated and
     /// replace by `try_optimize`.
     fn optimize(
         &self,
         plan: &LogicalPlan,
-        optimizer_config: &mut OptimizerConfig,
+        config: &dyn OptimizerConfig,
     ) -> Result<LogicalPlan>;
 
     /// A human readable name for this optimizer rule
     fn name(&self) -> &str;
 }
 
 /// Options to control the DataFusion Optimizer.
+pub trait OptimizerConfig {
+    /// Return the time at which the query execution started. This
+    /// time is used as the value for now()
+    fn query_execution_start_time(&self) -> DateTime<Utc>;
+
+    /// Returns false if the given rule should be skipped
+    fn rule_enabled(&self, name: &str) -> bool;
+
+    /// The optimizer will skip failing rules if this returns true
+    fn skip_failing_rules(&self) -> bool;
+
+    /// How many times to attempt to optimize the plan
+    fn max_passes(&self) -> u8;
+
+    /// Return a unique ID
+    ///
+    /// This is useful for assigning unique names to aliases
+    fn next_id(&self) -> usize;
+}
+
+/// A standalone [`OptimizerConfig`] that can be used independently
+/// of DataFusion's config management
 #[derive(Debug)]
-pub struct OptimizerConfig {
+pub struct OptimizerContext {
     /// Query execution start time that can be used to rewrite
     /// expressions such as `now()` to use a literal value instead
     query_execution_start_time: DateTime<Utc>,
     /// id generator for optimizer passes
-    // TODO this should not be on the config,
-    // it should be its own 'OptimizerState' or something)
-    next_id: usize,
+    next_id: AtomicUsize,

Review Comment:
   Looks good 👍 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to