[minor] make recursive package dependency optional

[buraksenn]

Which issue does this PR close?

Closes #13766

Rationale for this change

Adding recursive package causes issues for downstream projects

What changes are included in this PR?

Made recursive package optional in packages that use it instead of top level dependency

Are these changes tested?

Existing tests. Especially this one that fails when recursive is off:

    #[test]
    fn test_large_tree() {
        let mut item = TestTreeNode::new_leaf("initial".to_string());
        for i in 0..3000 {
            item = TestTreeNode::new(vec![item], format!("parent-{}", i));
        }

        let mut visitor =
            TestVisitor::new(Box::new(visit_continue), Box::new(visit_continue));

        item.visit(&mut visitor).unwrap();
    }

[blaginin]

should we add it to the docs?

[buraksenn]

should we add it to the docs?

Thanks. Added to README.md file

[alamb]

Thanks @buraksenn

I wonder if there is some way to test this (aka add a CI test that will fail with recursive protection -- perhaps the WASM compile test 🤔 ) that doesn't fail when the feature is disabled?

That way we can ensure we have the configuration settings hooked up right

[findepi]

thanks for working on this!

[findepi]

unrelated change?

[buraksenn]

I think this was caused by cargo fmt when I've added feature above

[findepi]

I wonder if there is some way to test this (aka add a CI test that will fail with recursive protection -- perhaps the WASM compile test 🤔 ) that doesn't fail when the feature is disabled?

@alamb would you want to see this in this PR, or in a follow-up?

[alamb]

I wonder if there is some way to test this (aka add a CI test that will fail with recursive protection -- perhaps the WASM compile test 🤔 ) that doesn't fail when the feature is disabled?

@alamb would you want to see this in this PR, or in a follow-up?

Yes sorry I should have been more explicit. I think this PR should have a test (maybe even manually at first?) to both

1. ensure that this change actually does allow the relevant subcrates to be compiled without #recursive
2. ensure we don't break the feature in the future (for example breaking code into new crates but not bringing the #cfg flags

[alamb]

I spent some time playing around with this locally, I think it does the right thing

Enabled by default:

$ cargo test  -p datafusion-common --lib 2>&1  | grep test_large_tree
test tree_node::tests::test_large_tree ... ok

Disabled without default features:

$ cargo test --no-default-features  -p datafusion-common --lib 2>&1  | grep test_large_tree
$

Can enable explicitly:

$ cargo test --no-default-features  -p datafusion-common --features=recursive-protection --lib 2>&1  | grep test_large_tree
test tree_node::tests::test_large_tree ... ok
$

Unfortunately, I can't really think of any feasible way to add this test to CI

Sorry for the delay reviewing / approving this @buraksenn and thank you for the reviews @findepi

I merged up from main and plan to merge it once the tests pass