Skip to content

Detect-and-bail guard for the join_order heuristic in cyclic join queries - #23

Open
jianhongshi123 wants to merge 3 commits into
robust-sql:mainfrom
jianhongshi123:dev/cyclic_join
Open

jianhongshi123 wants to merge 3 commits into
robust-sql:mainfrom
jianhongshi123:dev/cyclic_join

Conversation

@jianhongshi123

Copy link
Copy Markdown
Contributor

Cyclic join graphs can produce equivalence classes that LogicalCreateFilter::CreatePlan can't fully resolve, causing an out-of-bounds access in bound_column_indices at sink time.

This PR adds a detect-and-bail guard for the join_order heuristic. When a cyclic join graph is detected, the optimizer skips predicate transfer and returns the original plan.

Comment thread src/optimizer/robust_optimizer.cpp Outdated
map<ColKey, ColKey> uf_parent;
auto all_nodes = BuildPhysicalPlanDAG(plan.get(), uf_parent);

if (ExistCycle(all_nodes)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we do this in an earlier stage, while computing edges or DAG creation perhaps?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The cycle dectection mechanism now relies on the DAG creation, so moving it to the end of DAG creation won't impact the control flow much. However, putting it at computing edges might be possible and might need a different cycle detection mechanism, should I try that?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yes we want to bail out as early as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants