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

Fix documentation warnings, make CsvExecBuilder and Unparsed pub #11729

Merged
merged 4 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ jobs:
rust-version: stable
- name: Run cargo doc
run: |
export RUSTDOCFLAGS="-D warnings -A rustdoc::private-intra-doc-links"
export RUSTDOCFLAGS="-D warnings"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this makes all doc warnings an error and does not allow links to private items

Copy link
Member

Choose a reason for hiding this comment

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

RUSTDOCFLAGS in ci/scripts/rust_docs.sh should be updated as well
i am doing this in #11890

cargo doc --document-private-items --no-deps --workspace
cd datafusion-cli
cargo doc --document-private-items --no-deps
Expand Down
3 changes: 2 additions & 1 deletion datafusion/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ impl From<DataFusionError> for io::Error {
}

impl DataFusionError {
const BACK_TRACE_SEP: &'static str = "\n\nbacktrace: ";
/// The separator between the error message and the backtrace
pub const BACK_TRACE_SEP: &'static str = "\n\nbacktrace: ";

/// Get deepest underlying [`DataFusionError`]
///
Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/src/datasource/physical_plan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub use self::parquet::{ParquetExec, ParquetFileMetrics, ParquetFileReaderFactor

pub use arrow_file::ArrowExec;
pub use avro::AvroExec;
pub use csv::{CsvConfig, CsvExec, CsvOpener};
pub use csv::{CsvConfig, CsvExec, CsvExecBuilder, CsvOpener};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this struct, added by @connec recently, actually wasn't publically exported which was flagged by one of the doc tests

pub use file_groups::FileGroupPartitioner;
pub use file_scan_config::{
wrap_partition_type_in_dict, wrap_partition_value_in_dict, FileScanConfig,
Expand Down
4 changes: 2 additions & 2 deletions datafusion/core/src/datasource/physical_plan/parquet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ pub use writer::plan_to_parquet;
/// # Execution Overview
///
/// * Step 1: [`ParquetExec::execute`] is called, returning a [`FileStream`]
/// configured to open parquet files with a [`ParquetOpener`].
/// configured to open parquet files with a `ParquetOpener`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since ParquetOpener isn't public let's not try to make a link to it

///
/// * Step 2: When the stream is polled, the [`ParquetOpener`] is called to open
/// * Step 2: When the stream is polled, the `ParquetOpener` is called to open
/// the file.
///
/// * Step 3: The `ParquetOpener` gets the [`ParquetMetaData`] (file metadata)
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/string_agg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.

//! [`StringAgg`] and [`StringAggAccumulator`] accumulator for the `string_agg` function
//! [`StringAgg`] accumulator for the `string_agg` function

use arrow::array::ArrayRef;
use arrow_schema::DataType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ impl<S: SimplifyInfo> ExprSimplifier<S> {
self
}

/// Should [`Canonicalizer`] be applied before simplification?
/// Should `Canonicalizer` be applied before simplification?
///
/// If true (the default), the expression will be rewritten to canonical
/// form before simplification. This is useful to ensure that the simplifier
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-plan/src/joins/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use parking_lot::Mutex;
/// E.g. 1 -> [3, 6, 8] indicates that the column values map to rows 3, 6 and 8 for hash value 1
/// As the key is a hash value, we need to check possible hash collisions in the probe stage
/// During this stage it might be the case that a row is contained the same hashmap value,
/// but the values don't match. Those are checked in the [`equal_rows_arr`](crate::joins::hash_join::equal_rows_arr) method.
/// but the values don't match. Those are checked in the `equal_rows_arr` method.
///
/// The indices (values) are stored in a separate chained list stored in the `Vec<u64>`.
///
Expand Down
5 changes: 3 additions & 2 deletions datafusion/sql/src/unparser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,8 +522,9 @@ impl Unparser<'_> {
}
}

/// This function can convert more [`Expr`] types than `expr_to_sql`, returning an [`Unparsed`]
/// like `Sort` expressions to `OrderByExpr` expressions.
/// This function can convert more [`Expr`] types than `expr_to_sql`,
/// returning an [`Unparsed`] like `Sort` expressions to `OrderByExpr`
/// expressions.
pub fn expr_to_unparsed(&self, expr: &Expr) -> Result<Unparsed> {
match expr {
Expr::Sort(Sort {
Expand Down
2 changes: 2 additions & 0 deletions datafusion/sql/src/unparser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub use plan::plan_to_sql;
use self::dialect::{DefaultDialect, Dialect};
pub mod dialect;

pub use expr::Unparsed;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unparsed was also not pub, though maybe it does't need to be


/// Convert a DataFusion [`Expr`] to [`sqlparser::ast::Expr`]
///
/// See [`expr_to_sql`] for background. `Unparser` allows greater control of
Expand Down
Loading