Skip to content
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
44 changes: 44 additions & 0 deletions datafusion/core/tests/sql/joins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
use insta::assert_snapshot;

use datafusion::assert_batches_eq;
use datafusion::catalog::MemTable;
use datafusion::datasource::stream::{FileStreamProvider, StreamConfig, StreamTable};
use datafusion::test_util::register_unbounded_file_with_ordering;
use datafusion_sql::unparser::plan_to_sql;

use super::*;

Expand Down Expand Up @@ -264,3 +266,45 @@ async fn join_using_uppercase_column() -> Result<()> {

Ok(())
}

// Issue #17359: https://github.com/apache/datafusion/issues/17359
#[tokio::test]
async fn unparse_cross_join() -> Result<()> {
let ctx = SessionContext::new();

let j1_schema = Arc::new(Schema::new(vec![
Field::new("j1_id", DataType::Int32, true),
Field::new("j1_string", DataType::Utf8, true),
]));
let j2_schema = Arc::new(Schema::new(vec![
Field::new("j2_id", DataType::Int32, true),
Field::new("j2_string", DataType::Utf8, true),
]));

ctx.register_table("j1", Arc::new(MemTable::try_new(j1_schema, vec![vec![]])?))?;
ctx.register_table("j2", Arc::new(MemTable::try_new(j2_schema, vec![vec![]])?))?;

let df = ctx
.sql(
r#"
select j1.j1_id, j2.j2_string
from j1, j2
where j2.j2_id = 0
"#,
)
.await?;

let unopt_sql = plan_to_sql(df.logical_plan())?;
assert_snapshot!(unopt_sql, @r#"
SELECT j1.j1_id, j2.j2_string FROM j1 CROSS JOIN j2 WHERE (j2.j2_id = 0)
"#);

let optimized_plan = df.into_optimized_plan()?;

let opt_sql = plan_to_sql(&optimized_plan)?;
assert_snapshot!(opt_sql, @r#"
SELECT j1.j1_id, j2.j2_string FROM j1 CROSS JOIN j2 WHERE (j2.j2_id = 0)
"#);

Ok(())
}
7 changes: 0 additions & 7 deletions datafusion/sql/src/unparser/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,13 +696,6 @@ impl Unparser<'_> {
join_filters.as_ref(),
)?;

self.select_to_sql_recursively(
right_plan.as_ref(),
query,
select,
&mut right_relation,
)?;

Comment on lines -699 to -705
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe someone more knowledgeable could provide some context to this repeated call, but since all tests still work there doesn't seem to be a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree -- it seems like a copy/paste bug to me.

My git archeology suggests it came in the initial PR from @backkem

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it came from #13132 actually

let right_projection: Option<Vec<ast::SelectItem>> = if !already_projected
{
Some(select.pop_projections())
Expand Down
1 change: 1 addition & 0 deletions datafusion/sql/tests/cases/plan_to_sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.

use arrow::datatypes::{DataType, Field, Schema};

use datafusion_common::{
assert_contains, Column, DFSchema, DFSchemaRef, DataFusionError, Result,
TableReference,
Expand Down