feat: Make parquet_encryption a non-default feature

[miroim]

Which issue does this PR close?

• ClosesMake parquet_encryption a non-default feature #16777.

Rationale for this change

as discussion from @alamb on: #16649 (comment)

What changes are included in this PR?

• Removed the parquet_encryption feature from the default features.
• Added documentation to ParquetEncryptionOptions explaining that the parquet_encryption feature flag must be enabled to use Parquet encryption.
• Enabled the parquet_encryption feature in the datafusion-cli.
• Enabled the parquet_encryption feature in the datafusion/sqllogictest.
• Enabled the parquet_encryption feature in the datafusion-examples.
• Updated the Crate features in README.

Are these changes tested?

doc test

Are there any user-facing changes?

Change parquet_encryption feature from the default feature to non-default

[alamb]

CI failure tracked by

• security_audit CI check is failing on main #17144

[alamb]

Updating to get clean CI run

[alamb]

This makes sense to me -- thank you @miroim

I think we need to document the new config as well as add a new CI test to make sure the crate works without the feature. For example:

datafusion/.github/workflows/rust.yml

Lines 207 to 208 in 5c370fa

	- name: Check datafusion (parquet)
	run: cargo check --profile ci --no-default-features -p datafusion --features=parquet

And compile datafusion with the parquet and parquet_encryption features.

cc @adamreeve and @corwinjoy I would be interested to hear your opinion on this change

[corwinjoy]

This change looks reasonable to me.

[adamreeve]

👍 makes sense to me too.

There is already a cargo check run in CI that just enables the parquet_encryption feature:

datafusion/.github/workflows/rust.yml

Line 222 in 5c370fa

run: cargo check --profile ci --no-default-features -p datafusion --features=parquet_encryption

I think we need to enable parquet_encryption for the cargo test run here though:

datafusion/.github/workflows/rust.yml

Line 297 in 5c370fa

--features serde,avro,json,backtrace,integration-tests

[miroim]

Should we also enable parquet_encryption for the cargo test (macos-aarch64) here?

datafusion/.github/workflows/rust.yml

Line 558 in 5c370fa

run: cargo test --profile ci --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,integration-tests

[alamb]

Should we also enable parquet_encryption for the cargo test (macos-aarch64) here?

datafusion/.github/workflows/rust.yml

Line 558 in 5c370fa

run: cargo test --profile ci --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,integration-tests

I don't have any strong opinions -- I don't think it is necessary

[alamb]

I think we need to enable parquet_encryption for the cargo test run here though:

I agree this is needed to ensure the feature keeps working

We also need to check that the flags work in isolation as described in #17137 (review)

[adamreeve]

This seems like it should be redundant but then I noticed that the parquet_encryption feature doesn't actually depend on the parquet feature. Maybe we should remove this extra check run as the one above already tests the parquet_encryption feature, and update datafusion/core/Cargo.toml so that the parquet_encryption feature depends on the parquet feature rather than just dep:parquet?

[miroim]

Thank you @adamreeve for review

[alamb]

Thank you again @miroim and @adamreeve

[alamb]

I merged up from main and minimized this diff for clarity about what actually changed.

[alamb]

Thanks again @miroim