From 1369e2df79265bcdd60621721a5ca50a7e777ac8 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 15 Mar 2023 14:19:38 -0400 Subject: [PATCH 1/4] Minor: Add Documentation and Examples to `TableReference` --- datafusion/common/src/table_reference.rs | 84 +++++++++++++++++++----- 1 file changed, 67 insertions(+), 17 deletions(-) diff --git a/datafusion/common/src/table_reference.rs b/datafusion/common/src/table_reference.rs index 257073681934f..a50dfb0d7d4a9 100644 --- a/datafusion/common/src/table_reference.rs +++ b/datafusion/common/src/table_reference.rs @@ -35,7 +35,38 @@ impl<'a> std::fmt::Display for ResolvedTableReference<'a> { } } -/// Represents a path to a table that may require further resolution +/// [`TableReference`]s represent a multi part identifier (path) to a +/// table that may require further resolution. +/// +/// # Creating [`TableReference`] +/// +/// When converting strings to [`TableReference`]s, the string is +/// parsed as though it were a SQL identifier, normalizing (convert to +/// lowercase) any unquoted identifiers. +/// +/// See [`TableReference::bare`] to create references without applying +/// normalization semantics +/// +/// # Examples +/// ``` +/// # use datafusion_common::TableReference; +/// // Get a table reference to 'mytable' +/// let table_reference = TableReference::from("mytable"); +/// assert_eq!(table_reference, TableReference::bare("mytable")); +/// +/// // Get a table reference to 'mytable' (note the capitalization) +/// let table_reference = TableReference::from("MyTable"); +/// assert_eq!(table_reference, TableReference::bare("mytable")); +/// +/// // Get a table reference to 'MyTable' (note the capitalization) using double quotes +/// // (programatically it is better to use `TableReference::bare` for this) +/// let table_reference = TableReference::from(r#""MyTable""#); +/// assert_eq!(table_reference, TableReference::bare("MyTable")); +/// +/// // Get a table reference to 'myschema.mytable' (note the capitalization) +/// let table_reference = TableReference::from("MySchema.MyTable"); +/// assert_eq!(table_reference, TableReference::partial("myschema", "mytable")); +///``` #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum TableReference<'a> { /// An unqualified table reference, e.g. "table" @@ -61,6 +92,16 @@ pub enum TableReference<'a> { }, } +/// This is a [`TableReference`] that has 'static lifetime (aka it +/// owns the underlying string) +/// +/// To convert a [`TableReference`] to an [`OwnedTableReference`], use +/// +/// ``` +/// # use datafusion_common::{OwnedTableReference, TableReference}; +/// let table_reference = TableReference::from("mytable"); +/// let owned_reference = table_reference.to_owned_reference(); +/// ``` pub type OwnedTableReference = TableReference<'static>; impl std::fmt::Display for TableReference<'_> { @@ -80,14 +121,18 @@ impl std::fmt::Display for TableReference<'_> { } impl<'a> TableReference<'a> { - /// Convenience method for creating a `Bare` variant of `TableReference` + /// Convenience method for creating a [`TableReference::Bare`] + /// + /// As described on [`TableReference`] this does *NO* parsing at + /// all -- so "Foo.Bar" stays as a reference to the table named + /// "Foo.Bar" (rather than "foo"."bar") pub fn bare(table: impl Into>) -> TableReference<'a> { TableReference::Bare { table: table.into(), } } - /// Convenience method for creating a `Partial` variant of `TableReference` + /// Convenience method for creating a [`TableReference::Partial`] pub fn partial( schema: impl Into>, table: impl Into>, @@ -98,7 +143,7 @@ impl<'a> TableReference<'a> { } } - /// Convenience method for creating a `Full` variant of `TableReference` + /// Convenience method for creating a [`TableReference::Full`] pub fn full( catalog: impl Into>, schema: impl Into>, @@ -136,12 +181,12 @@ impl<'a> TableReference<'a> { } } - /// Compare with another `TableReference` as if both are resolved. + /// Compare with another [`TableReference`] as if both are resolved. /// This allows comparing across variants, where if a field is not present /// in both variants being compared then it is ignored in the comparison. /// - /// e.g. this allows a `TableReference::Bare` to be considered equal to a - /// fully qualified `TableReference::Full` if the table names match. + /// e.g. this allows a [`TableReference::Bare`] to be considered equal to a + /// fully qualified [`TableReference::Full`] if the table names match. pub fn resolved_eq(&self, other: &Self) -> bool { match self { TableReference::Bare { table } => table == other.table(), @@ -189,7 +234,8 @@ impl<'a> TableReference<'a> { } } - /// Converts directly into an [`OwnedTableReference`] + /// Converts directly into an [`OwnedTableReference`] by copying + /// the underlying data. pub fn to_owned_reference(&self) -> OwnedTableReference { match self { Self::Full { @@ -212,6 +258,16 @@ impl<'a> TableReference<'a> { } /// Forms a string where the identifiers are quoted + /// + /// # Example + /// ``` + /// # use datafusion_common::TableReference; + /// let table_reference = TableReference::partial("myschema", "mytable"); + /// assert_eq!(table_reference.to_quoted_string(), r#""myschema"."mytable""#); + /// + /// let table_reference = TableReference::partial("MySchema", "MyTable"); + /// assert_eq!(table_reference.to_quoted_string(), r#""MySchema"."MyTable""#); + /// ``` pub fn to_quoted_string(&self) -> String { match self { TableReference::Bare { table } => quote_identifier(table), @@ -231,14 +287,8 @@ impl<'a> TableReference<'a> { } } - /// Forms a [`TableReference`] by attempting to parse `s` as a multipart identifier, - /// failing that then taking the entire unnormalized input as the identifier itself. - /// - /// Will normalize (convert to lowercase) any unquoted identifiers. - /// - /// e.g. `Foo` will be parsed as `foo`, and `"Foo"".bar"` will be parsed as - /// `Foo".bar` (note the preserved case and requiring two double quotes to represent - /// a single double quote in the identifier) + /// Forms a [`TableReference`] by parsing `s` as a multipart + /// identifier. See docs on [`TableReference`] for more details. pub fn parse_str(s: &'a str) -> Self { let mut parts = parse_identifiers_normalized(s); @@ -260,7 +310,7 @@ impl<'a> TableReference<'a> { } } -/// Parse a `String` into a OwnedTableReference +/// Parse a `String` into a OwnedTableReference as a SQL identifier. impl From for OwnedTableReference { fn from(s: String) -> Self { TableReference::parse_str(&s).to_owned_reference() From fd68dc0572a4e2d3d306b53f22453aa73c635a9a Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 15 Mar 2023 15:28:08 -0400 Subject: [PATCH 2/4] mention no parsing --- datafusion/common/src/table_reference.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/datafusion/common/src/table_reference.rs b/datafusion/common/src/table_reference.rs index a50dfb0d7d4a9..32a427a029dd9 100644 --- a/datafusion/common/src/table_reference.rs +++ b/datafusion/common/src/table_reference.rs @@ -124,7 +124,7 @@ impl<'a> TableReference<'a> { /// Convenience method for creating a [`TableReference::Bare`] /// /// As described on [`TableReference`] this does *NO* parsing at - /// all -- so "Foo.Bar" stays as a reference to the table named + /// all, so "Foo.Bar" stays as a reference to the table named /// "Foo.Bar" (rather than "foo"."bar") pub fn bare(table: impl Into>) -> TableReference<'a> { TableReference::Bare { @@ -132,7 +132,9 @@ impl<'a> TableReference<'a> { } } - /// Convenience method for creating a [`TableReference::Partial`] + /// Convenience method for creating a [`TableReference::Partial`]. + /// + /// As described on [`TableReference`] this does parsing at all. pub fn partial( schema: impl Into>, table: impl Into>, @@ -144,6 +146,8 @@ impl<'a> TableReference<'a> { } /// Convenience method for creating a [`TableReference::Full`] + /// + /// As described on [`TableReference`] this does parsing at all. pub fn full( catalog: impl Into>, schema: impl Into>, From 7e7586b7c81d27233428eddc499077bea8099243 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 15 Mar 2023 15:29:41 -0400 Subject: [PATCH 3/4] more cleanups --- datafusion/common/src/table_reference.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/datafusion/common/src/table_reference.rs b/datafusion/common/src/table_reference.rs index 32a427a029dd9..4451a702c9de5 100644 --- a/datafusion/common/src/table_reference.rs +++ b/datafusion/common/src/table_reference.rs @@ -238,7 +238,7 @@ impl<'a> TableReference<'a> { } } - /// Converts directly into an [`OwnedTableReference`] by copying + /// Converts directly into an [`OwnedTableReference`] by cloning /// the underlying data. pub fn to_owned_reference(&self) -> OwnedTableReference { match self { @@ -291,7 +291,7 @@ impl<'a> TableReference<'a> { } } - /// Forms a [`TableReference`] by parsing `s` as a multipart + /// Forms a [`TableReference`] by parsing `s` as a multipart SQL /// identifier. See docs on [`TableReference`] for more details. pub fn parse_str(s: &'a str) -> Self { let mut parts = parse_identifiers_normalized(s); @@ -314,7 +314,7 @@ impl<'a> TableReference<'a> { } } -/// Parse a `String` into a OwnedTableReference as a SQL identifier. +/// Parse a `String` into a OwnedTableReference as a multipart SQL identifier. impl From for OwnedTableReference { fn from(s: String) -> Self { TableReference::parse_str(&s).to_owned_reference() From 990c1c5e2e079ad97cb630dbc5aaaef21da5288e Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 17 Mar 2023 06:23:09 -0400 Subject: [PATCH 4/4] Apply suggestions from code review --- datafusion/common/src/table_reference.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datafusion/common/src/table_reference.rs b/datafusion/common/src/table_reference.rs index fdee38cb61c42..d0829702d07fa 100644 --- a/datafusion/common/src/table_reference.rs +++ b/datafusion/common/src/table_reference.rs @@ -139,7 +139,7 @@ impl<'a> TableReference<'a> { /// Convenience method for creating a [`TableReference::Partial`]. /// - /// As described on [`TableReference`] this does parsing at all. + /// As described on [`TableReference`] this does *NO* parsing at all. pub fn partial( schema: impl Into>, table: impl Into>, @@ -152,7 +152,7 @@ impl<'a> TableReference<'a> { /// Convenience method for creating a [`TableReference::Full`] /// - /// As described on [`TableReference`] this does parsing at all. + /// As described on [`TableReference`] this does *NO* parsing at all. pub fn full( catalog: impl Into>, schema: impl Into>,