Skip to content

BugFix: Introducing Introducing Delim Joins and Delim_Get(s) should respect positionally #11812

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.

Sign up for GitHub

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

Merged
Mytherin merged 9 commits into from
Apr 30, 2024
Merged

BugFix: Introducing Introducing Delim Joins and Delim_Get(s) should respect positionally #11812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ad9562f
add test
Tmonster Apr 12, 2024
2f07b4f
start of fix. need to fix column binding replacer to stop replacing b…
Tmonster Apr 24, 2024
5566b5d
modify cross product to have delim expressions
Tmonster Apr 24, 2024
2936ccb
fix is workingg
Tmonster Apr 24, 2024
60d4258
make format-fix
Tmonster Apr 24, 2024
5355147
Merge branch 'main' into delim_joins_and_reordering
Tmonster Apr 24, 2024
23e0e24
remove unused includes
Tmonster Apr 26, 2024
492a7df
small PR comments
Tmonster Apr 29, 2024
d81bb5d
remove test that doesn't really test anything
Tmonster Apr 29, 2024
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: 0 additions & 2 deletions src/planner/binder/query_node/plan_subquery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "duckdb/planner/expression/bound_columnref_expression.hpp"
#include "duckdb/planner/expression/bound_comparison_expression.hpp"
#include "duckdb/planner/expression/bound_constant_expression.hpp"
#include "duckdb/planner/expression/bound_reference_expression.hpp"
#include "duckdb/planner/expression/bound_subquery_expression.hpp"
#include "duckdb/planner/expression/bound_window_expression.hpp"
#include "duckdb/planner/expression_iterator.hpp"
Expand All @@ -16,7 +15,6 @@
#include "duckdb/planner/subquery/flatten_dependent_join.hpp"
#include "duckdb/common/enums/logical_operator_type.hpp"
#include "duckdb/planner/operator/logical_dependent_join.hpp"
#include "duckdb/planner/expression_binder/lateral_binder.hpp"
#include "duckdb/planner/subquery/recursive_dependent_join_planner.hpp"

namespace duckdb {
Expand Down
38 changes: 29 additions & 9 deletions src/planner/subquery/flatten_dependent_join.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ unique_ptr<LogicalOperator> FlattenDependentJoins::PushDownDependentJoinInternal
idx_t lateral_depth) {
// first check if the logical operator has correlated expressions
auto entry = has_correlated_expressions.find(*plan);
bool exit_projection = false;
unique_ptr<LogicalDelimGet> delim_scan;
D_ASSERT(entry != has_correlated_expressions.end());
if (!entry->second) {
// we reached a node without correlated expressions
Expand All @@ -136,13 +138,21 @@ unique_ptr<LogicalOperator> FlattenDependentJoins::PushDownDependentJoinInternal
}
}

auto left_columns = plan->GetColumnBindings().size();
// create cross product with Delim Join
auto delim_index = binder.GenerateTableIndex();
this->base_binding = ColumnBinding(delim_index, 0);
this->delim_offset = left_columns;
this->data_offset = 0;
auto delim_scan = make_uniq<LogicalDelimGet>(delim_index, delim_types);
return LogicalCrossProduct::Create(std::move(plan), std::move(delim_scan));
base_binding = ColumnBinding(delim_index, 0);

auto left_columns = plan->GetColumnBindings().size();
delim_offset = left_columns;
data_offset = 0;
delim_scan = make_uniq<LogicalDelimGet>(delim_index, delim_types);
if (plan->type == LogicalOperatorType::LOGICAL_PROJECTION) {
// we want to keep the logical projection for positionality.
exit_projection = true;
} else {
auto cross_product = LogicalCrossProduct::Create(std::move(plan), std::move(delim_scan));
return cross_product;
}
}
switch (plan->type) {
case LogicalOperatorType::LOGICAL_UNNEST:
Expand All @@ -166,8 +176,18 @@ unique_ptr<LogicalOperator> FlattenDependentJoins::PushDownDependentJoinInternal
for (auto &expr : plan->expressions) {
parent_propagate_null_values &= expr->PropagatesNullValues();
}
plan->children[0] =
PushDownDependentJoinInternal(std::move(plan->children[0]), parent_propagate_null_values, lateral_depth);

// if the node has no correlated expressions,
// push the cross product with the delim get only below the projection.
// This will preserve positionality of the columns and prevent errors when reordering of
// delim gets is enabled.
if (exit_projection) {
auto cross_product = LogicalCrossProduct::Create(std::move(plan->children[0]), std::move(delim_scan));
plan->children[0] = std::move(cross_product);
} else {
plan->children[0] = PushDownDependentJoinInternal(std::move(plan->children[0]),
parent_propagate_null_values, lateral_depth);
}

// then we replace any correlated expressions with the corresponding entry in the correlated_map
RewriteCorrelatedExpressions rewriter(base_binding, correlated_map, lateral_depth);
Expand Down Expand Up @@ -248,7 +268,7 @@ unique_ptr<LogicalOperator> FlattenDependentJoins::PushDownDependentJoinInternal
}
}
auto left_index = binder.GenerateTableIndex();
auto delim_scan = make_uniq<LogicalDelimGet>(left_index, delim_types);
delim_scan = make_uniq<LogicalDelimGet>(left_index, delim_types);
join->children.push_back(std::move(delim_scan));
join->children.push_back(std::move(plan));
for (idx_t i = 0; i < new_group_count; i++) {
Expand Down
24 changes: 24 additions & 0 deletions test/sql/subquery/exists/test_exists_union_by_name.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# name: test/sql/subquery/exists/test_exists_union_by_name.test
# description: Test exists subquery with union by name
# group: [exists]

statement ok
create table all_types as select * exclude(small_enum, medium_enum, large_enum) from test_all_types() limit 0;

statement ok
SELECT (
EXISTS(
(
SELECT
DISTINCT outer_alltypes."BIGINT", outer_alltypes."INT"
FROM all_types inner_alltypes_1
WHERE inner_alltypes_1."BIGINT" GROUP BY NULL
)
UNION BY NAME
(
SELECT inner2."FLOAT" from all_types inner2
)
) IS DISTINCT FROM outer_alltypes."struct"
)
FROM all_types outer_alltypes GROUP BY ALL;