@alamb for your consideration.

I don't like that datafusion-physical-optimizer needs to use datafusion crate as a dev-dependency. This was required as SanityChecker tests were tightly coupled with the helper functions in datafusion::physical_optimizer::test_utils. Perhaps we can consider moving these helper functions somewhere outside, say, to test-utils crate?

I don't like that datafusion-physical-optimizer needs to use datafusion crate as a dev-dependency.

Thanks @mnpw -- I agree this is not good and we shouldn't merge it in like that (and in fact I think it failed the circular dependency check)

I think one of the major sources of this dependency is the use of ParquetExec which is still defined in the core module...

Since datafusion already depends on phsyical-optimizer, what would you think about moving as many of the physical optimizer utils from

• https://github.com/apache/datafusion/blob/main/datafusion/core/src/physical_optimizer/test_utils.rs
  To
• https://github.com/apache/datafusion/tree/main/datafusion/physical-optimizer/src/test_utils.rs
  As possible?

Converting to draft given the dependency situation we have uncovered

@alamb

Here's my understanding of the issue:

• datafusion crate already depends on datafusion-physical-optimizer.
• SanityChecker's tests depend on datafusion::datasources::{listing, physical_plan, stream} (via datafusion::physical_optimizer::test_utils).
• Moving SanityChecker, with its tests, to datafusion-physical-optimizer makes datafusion-physical-optimizer depend on datafusion::datasources.
• This creates a circular dependency between datafusion and datafusion-physical-optimizer.

In this case, best solution seems to be to move SanityChecker to datafusion-physical-optimizer while keeping its tests in datafusion/core/tests.

I have re-worked the PR and made the following changes:

• Move SanityChecker to datafusion-physical-optimizer
• Move SanityChecker's tests to datafusion/core/tests
• Move independent componenets from datafusion::physical_optimizer::test_utils to datafusion-physical-optimizer::test_utils

I will update the datafusion-cli cargo file as well

Thanks @alamb for proactively reviewing and adding the fix commits. Much appreciated 🙏

Thanks @alamb for proactively reviewing and adding the fix commits. Much appreciated 🙏

Thank you for helping push this along ❤️