You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/05/20 17:47:52 UTC
[PR] Minor: Improve documentation in sql_to_plan example [datafusion]
alamb opened a new pull request, #10582:
URL: https://github.com/apache/datafusion/pull/10582
## Which issue does this PR close?
Follow on to https://github.com/apache/datafusion/pull/10558 from @xinlifoobar (🙏 ) and @edmondop
## Rationale for this change
I noticed a few potential improvements to the sql_to_plan examples
## What changes are included in this PR?
1. Improve the documentation and function names to make the examples better in my opinion
## Are these changes tested?
Existing CI
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
3. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Minor: Improve documentation in sql_to_plan example [datafusion]
Posted by "edmondop (via GitHub)" <gi...@apache.org>.
edmondop commented on code in PR #10582:
URL: https://github.com/apache/datafusion/pull/10582#discussion_r1607146553
##########
datafusion-examples/examples/plan_to_sql.rs:
##########
@@ -22,36 +22,45 @@ use datafusion::sql::unparser::expr_to_sql;
use datafusion_sql::unparser::dialect::CustomDialect;
use datafusion_sql::unparser::{plan_to_sql, Unparser};
-/// This example demonstrates the programmatic construction of
-/// SQL using the DataFusion Expr [`Expr`] and LogicalPlan [`LogicalPlan`] API.
+/// This example demonstrates the programmatic construction of SQL strings using
+/// the DataFusion Expr [`Expr`] and LogicalPlan [`LogicalPlan`] API.
///
///
/// The code in this example shows how to:
-/// 1. Create SQL from a variety of Expr and LogicalPlan: [`main`]`
-/// 2. Create a simple expression [`Exprs`] with fluent API
-/// and convert to sql: [`simple_expr_to_sql_demo`]
-/// 3. Create a simple expression [`Exprs`] with fluent API
-/// and convert to sql without escaping column names: [`simple_expr_to_sql_demo_no_escape`]
-/// 4. Create a simple expression [`Exprs`] with fluent API
-/// and convert to sql escaping column names a MySQL style: [`simple_expr_to_sql_demo_escape_mysql_style`]
+///
+/// 1. [`simple_expr_to_sql_demo`]: Create a simple expression [`Exprs`] with
+/// fluent API and convert to sql suitable for passing to another database
+///
+/// 2. [`simple_expr_to_sql_demo_no_escape`] Create a simple expression
+/// [`Exprs`] with fluent API and convert to sql without escaping column names
+/// more suitable for displaying to humans.
+///
+/// 3. [`simple_expr_to_sql_demo_escape_mysql_style`]" Create a simple
+/// expression [`Exprs`] with fluent API and convert to sql escaping column
+/// names in MySQL style.
+///
+/// 4. [`simple_plan_to_sql_demo`]: Create a simple logical plan using the
+/// DataFrames API and convert to sql string.
+///
+/// 5. [`round_trip_plan_to_sql_demo`]: Create a logical plan from a SQL string, modify it using the
+/// DataFrames API and convert it back to a sql string.
#[tokio::main]
async fn main() -> Result<()> {
// See how to evaluate expressions
simple_expr_to_sql_demo()?;
simple_expr_to_sql_demo_no_escape()?;
simple_expr_to_sql_demo_escape_mysql_style()?;
- simple_plan_to_sql_parquest_dataframe_demo().await?;
- round_trip_plan_to_sql_parquest_dataframe_demo().await?;
+ simple_plan_to_sql_demo().await?;
+ round_trip_plan_to_sql_demo().await?;
Ok(())
}
/// DataFusion can convert expressions to SQL, using column name escaping
/// PostgreSQL style.
fn simple_expr_to_sql_demo() -> Result<()> {
let expr = col("a").lt(lit(5)).or(col("a").eq(lit(8)));
- let ast = expr_to_sql(&expr)?;
- let sql = format!("{}", ast);
+ let sql = expr_to_sql(&expr)?.to_string();
Review Comment:
I don't remember why I had to do it, that totally makes sense
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Minor: Improve documentation in sql_to_plan example [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead merged PR #10582:
URL: https://github.com/apache/datafusion/pull/10582
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Minor: Improve documentation in sql_to_plan example [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10582:
URL: https://github.com/apache/datafusion/pull/10582#discussion_r1607076013
##########
datafusion-examples/examples/plan_to_sql.rs:
##########
@@ -22,36 +22,45 @@ use datafusion::sql::unparser::expr_to_sql;
use datafusion_sql::unparser::dialect::CustomDialect;
use datafusion_sql::unparser::{plan_to_sql, Unparser};
-/// This example demonstrates the programmatic construction of
-/// SQL using the DataFusion Expr [`Expr`] and LogicalPlan [`LogicalPlan`] API.
+/// This example demonstrates the programmatic construction of SQL strings using
+/// the DataFusion Expr [`Expr`] and LogicalPlan [`LogicalPlan`] API.
///
///
/// The code in this example shows how to:
-/// 1. Create SQL from a variety of Expr and LogicalPlan: [`main`]`
-/// 2. Create a simple expression [`Exprs`] with fluent API
-/// and convert to sql: [`simple_expr_to_sql_demo`]
-/// 3. Create a simple expression [`Exprs`] with fluent API
-/// and convert to sql without escaping column names: [`simple_expr_to_sql_demo_no_escape`]
-/// 4. Create a simple expression [`Exprs`] with fluent API
-/// and convert to sql escaping column names a MySQL style: [`simple_expr_to_sql_demo_escape_mysql_style`]
+///
+/// 1. [`simple_expr_to_sql_demo`]: Create a simple expression [`Exprs`] with
+/// fluent API and convert to sql suitable for passing to another database
+///
+/// 2. [`simple_expr_to_sql_demo_no_escape`] Create a simple expression
+/// [`Exprs`] with fluent API and convert to sql without escaping column names
+/// more suitable for displaying to humans.
+///
+/// 3. [`simple_expr_to_sql_demo_escape_mysql_style`]" Create a simple
+/// expression [`Exprs`] with fluent API and convert to sql escaping column
+/// names in MySQL style.
+///
+/// 4. [`simple_plan_to_sql_demo`]: Create a simple logical plan using the
+/// DataFrames API and convert to sql string.
+///
+/// 5. [`round_trip_plan_to_sql_demo`]: Create a logical plan from a SQL string, modify it using the
+/// DataFrames API and convert it back to a sql string.
#[tokio::main]
async fn main() -> Result<()> {
// See how to evaluate expressions
simple_expr_to_sql_demo()?;
simple_expr_to_sql_demo_no_escape()?;
simple_expr_to_sql_demo_escape_mysql_style()?;
- simple_plan_to_sql_parquest_dataframe_demo().await?;
- round_trip_plan_to_sql_parquest_dataframe_demo().await?;
+ simple_plan_to_sql_demo().await?;
+ round_trip_plan_to_sql_demo().await?;
Ok(())
}
/// DataFusion can convert expressions to SQL, using column name escaping
/// PostgreSQL style.
fn simple_expr_to_sql_demo() -> Result<()> {
let expr = col("a").lt(lit(5)).or(col("a").eq(lit(8)));
- let ast = expr_to_sql(&expr)?;
- let sql = format!("{}", ast);
+ let sql = expr_to_sql(&expr)?.to_string();
Review Comment:
I think it makes it easier to use this example if we don't have to worry about the intermediate `ast` as much, so I combined it into a single statement
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org