Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 2 partitions in planner tests #17

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

andygrove
Copy link
Member

Use 2 partitions so that we have distributed plans with multiple query stages

@andygrove andygrove changed the title Use 2 partitions Use 2 partitions in planner tests Oct 4, 2024
@@ -394,8 +394,8 @@ mod test {

let file = format!("testdata/queries/q{n}.sql");
let sql = fs::read_to_string(&file)?;
let config = SessionConfig::new().with_target_partitions(1);
let ctx = SessionContext::with_config(config);
let config = SessionConfig::new().with_target_partitions(2);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main change

@@ -145,26 +145,6 @@ export TPCH_DATA_PATH=`pwd`/data
cargo test
```

Tests compare plans with expected plans, which unfortunately contain the
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is not needed anymore, can we also remove the script ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was removed in my previous PR, but I missed this documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooops, my previous review was not deep enough

@andygrove andygrove merged commit b91705c into apache:main Oct 4, 2024
1 check passed
@andygrove andygrove deleted the use-2-partitions branch October 4, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants