villebro commented on code in PR #2819:
URL:
https://github.com/apache/datafusion-ballista/pull/2819#discussion_r4507175056
##########
ballista/scheduler/src/test_utils.rs:
##########
@@ -129,7 +104,12 @@ impl TableProvider for PanickingTableProvider {
_filters: &[Expr],
_limit: Option<usize>,
) -> datafusion::common::Result<Arc<dyn ExecutionPlan>> {
- panic!("PanickingTableProvider just panics!")
+ match self {
+ Self::Error => Err(DataFusionError::Plan(
+ "ExplodingTableProvider just throws an error!".to_owned(),
+ )),
+ Self::Panic => panic!("PanickingTableProvider just panics!"),
Review Comment:
Agreed, renamed it to `"ExplodingTableProvider just panics!"` in 7929296a1,
along with the assertion that checks it.
##########
ballista/scheduler/src/state/aqe/test/physical_submission.rs:
##########
@@ -22,43 +22,23 @@
//! must keep those shuffles as stage boundaries and must not make decisions
//! that assume it knows what an extension operator does with its input.
-use crate::state::aqe::adaptive_rules::{AdaptiveRule, AdaptiveRules};
-use crate::state::aqe::execution_plan::ExchangeExec;
use crate::state::aqe::planner::AdaptivePlanner;
-use crate::state::aqe::test::coalesce_rule::partitions_with_byte_sizes;
-use crate::state::aqe::test::mock_batch;
+use crate::state::aqe::test::{
+ Embedder, Seen, coalesce_context, mock_batch, partitions_with_byte_sizes,
+};
+use crate::test_utils::PassThroughExec;
use ballista_core::assert_plan;
-use ballista_core::extension::SessionConfigExt;
-use datafusion::common::tree_node::{TreeNode, TreeNodeRecursion};
-use datafusion::config::ConfigOptions;
-use datafusion::execution::{SessionStateBuilder, TaskContext};
-use datafusion::physical_expr::PhysicalExpr;
use datafusion::physical_expr::expressions::Column;
-use datafusion::physical_optimizer::PhysicalOptimizerRule;
use datafusion::physical_plan::repartition::RepartitionExec;
-use datafusion::physical_plan::{
- DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PlanProperties,
- SendableRecordBatchStream,
-};
-use datafusion::prelude::{SessionConfig, SessionContext};
-use std::fmt::Formatter;
+use datafusion::physical_plan::{ExecutionPlan, Partitioning};
+use datafusion::prelude::SessionContext;
use std::sync::Arc;
-use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
+use std::sync::atomic::Ordering;
-/// A session with Ballista's config extension installed, partition
-/// coalescing switched on or off, and a small coalesce target so the
-/// bin-pack is hand-traceable.
+/// A coalesce-tuned session with partition coalescing switched on or off and
+/// the table `t` registered.
fn physical_context(coalesce_enabled: bool) -> SessionContext {
- let config = SessionConfig::new_with_ballista()
- .with_target_partitions(8)
- .with_round_robin_repartition(false)
- .with_ballista_coalesce_enabled(coalesce_enabled)
- .with_ballista_coalesce_target_partition_bytes(200);
- let state = SessionStateBuilder::new()
- .with_config(config)
- .with_default_features()
- .build();
- let ctx = SessionContext::new_with_state(state);
+ let ctx = coalesce_context(8, coalesce_enabled);
Review Comment:
Good catch. Rather than a note, 766f92755 keeps the join knobs out of the
shared helper: `coalesce_config` holds the coalesce knobs, `coalesce_context`
adds the sort-merge join knobs on top, and `physical_context` builds on
`coalesce_config`, so it's back to the config it had before. The sort-merge
coalesce test now uses `coalesce_config` too.
--
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]
