alamb commented on code in PR #25688: URL: https://github.com/apache/datafusion/pull/25688#discussion_r4158012389
########## datafusion/physical-optimizer/src/analyzer.rs: ########## @@ -0,0 +1,153 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Physical analyzer + +use std::sync::Arc; + +use crate::ensure_requirements::{EnforceDistribution, EnforceSorting}; +use crate::output_requirements::OutputRequirements; + +// Re-export from this module for convenience. +pub use datafusion_session::PhysicalAnalyzerRule; + +/// A rule-based physical analyzer. +/// +/// Analyzer rules make the plan *valid*: they enforce the invariants every +/// operator declares (distribution, ordering) rather than making the plan +/// faster, mirroring the logical layer's `Analyzer`/`Optimizer` split. The +/// default planner runs the whole analyzer phase before the +/// [`PhysicalOptimizer`](crate::optimizer::PhysicalOptimizer) phase, so every +/// optimizer rule can assume it receives a valid plan; see +/// [`PhysicalAnalyzerRule`] for details. +#[derive(Clone, Debug)] +pub struct PhysicalAnalyzer { + /// All rules to apply + pub rules: Vec<Arc<dyn PhysicalAnalyzerRule + Send + Sync>>, +} + +impl Default for PhysicalAnalyzer { + fn default() -> Self { + Self::new() + } +} + +impl PhysicalAnalyzer { + /// Create a new analyzer using the recommended list of rules + pub fn new() -> Self { + // All enforcement runs here, first, as the analyzer phase: it makes the + // plan *valid* (distribution first, then ordering) before any optimizer + // rule sees it, mirroring the logical Analyzer/Optimizer split. The + // optimizer rules that change these requirements (`JoinSelection`, + // `WindowTopN`, `FilterPushdown`) re-establish validity themselves, so + // enforcement is not repeated as an optimizer pass. The sort + // *optimizations* (`OptimizeSorts`) are not enforcement and stay in the + // optimizer phase. + let rules: Vec<Arc<dyn PhysicalAnalyzerRule + Send + Sync>> = vec![ + // Establish the output-requirement boundary first, so enforcement can + // see it (parallelize top-level scans below it, preserve the query's + // final ordering). The matching remove pass runs late in the optimizer. + Arc::new(OutputRequirements::new_add_mode()), + Arc::new(EnforceDistribution::new()), Review Comment: > JoinSelection and FilterPushdown re-establish distribution only, since neither changes any operator ordering. Maybe to cut down the initial PR scope we could keep them as the same pass (aka `EnforceDistribution`) Then as a follow on PR we could then split them into separate passes (and it would be clear how to measure / evaluate the impact) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
