Skip to content
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.

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

🪲 selection pushdown error #1587

Closed
joocer opened this issue Apr 18, 2024 · 6 comments · Fixed by #1589
Closed

🪲 selection pushdown error #1587

joocer opened this issue Apr 18, 2024 · 6 comments · Fixed by #1589
Labels
Bug 🪲 Something isn't working High Priority 1️⃣

Comments

@joocer
Copy link
Contributor

joocer commented Apr 18, 2024

SELECT name, Mission_Status, Mission 
FROM $astronauts CROSS JOIN UNNEST (missions) AS mission_names 
INNER JOIN $missions ON Mission = mission_names 
WHERE Mission = 'Apollo 11'

returns this error: TypeError: object of type 'NoneType' has no len()

AFTER COST OPTIMIZATION
└─ EXIT
   └─ PROJECT (Mission_Status, Mission, name)
      └─ INNER JOIN ($missions.Mission = $unnest-8fde.mission_names)
         ├─ SCAN ($missions) [Mission, Mission_Status]
         └─ FILTER (Mission = 'Apollo 11')
            └─ CROSS JOIN
               ├─ SCAN ($astronauts) [name, missions]
               └─ UNNEST (missions AS $unnest-8fde)

We're FILTERing Mission before the SCAN to read it.

The SCAN and the UNNEST are in the wrong order too ... this may be a problem with the tree drawing and a different error with the filter

@joocer joocer added Bug 🪲 Something isn't working High Priority 1️⃣ labels Apr 18, 2024
@joocer
Copy link
Contributor Author

joocer commented Apr 18, 2024

if we change the filter to be mission_names the query works... we appear to know that Mission and mission_names is the same data, but not which one to use when we push the filter past the JOIN

@joocer
Copy link
Contributor Author

joocer commented Apr 18, 2024

we current drop all of the predicates before we CROSS JOIN - this means we don't fail to push the predicate and put it back where we picked it up from. We should only drop the predicate if we know all of the columns.

@joocer
Copy link
Contributor Author

joocer commented Apr 18, 2024

after fix, this is the optimized plan

AFTER COST OPTIMIZATION
└─ EXIT
   └─ PROJECT (Mission_Status, mission_names, name)
      └─ INNER JOIN ($missions.Mission = $unnest-93df.mission_names)
         ├─ CROSS JOIN
         │  ├─ SCAN ($astronauts) [name, missions]
         │  └─ UNNEST (missions AS $unnest-93df)
         └─ FILTER (Mission = 'Apollo 11')
            └─ SCAN ($missions) [Mission, Mission_Status]

We now correctly push the Mission filter to the $missions SCAN

@joocer
Copy link
Contributor Author

joocer commented Apr 18, 2024

a regression test now fails

SELECT name, mission 
FROM $astronauts CROSS JOIN UNNEST(missions) AS mission 
WHERE mission = 'Apollo 11'
LOGICAL PLAN
└─ EXIT
   └─ PROJECT (name, mission)
      └─ FILTER (mission = 'Apollo 11')
         └─ CROSS JOIN
            ├─ SCAN ($astronauts)
            └─ UNNEST (missions AS $unnest-38da)
AFTER COST OPTIMIZATION
└─ EXIT
   └─ PROJECT (mission, name)
      └─ CROSS JOIN
         ├─ SCAN ($astronauts) [name, missions]
         └─ FILTER (mission = 'Apollo 11')
            └─ UNNEST (missions AS $unnest-38da)

@joocer
Copy link
Contributor Author

joocer commented Apr 18, 2024

I've fixed the UNNEST display in the plan to show the column name not the virtual relation - haven't fixed the bug though

@joocer
Copy link
Contributor Author

joocer commented Apr 19, 2024

fixed, need to dump predicates that reference the unnest target too, pushing them past the unnest is impossible

joocer added a commit that referenced this issue Apr 19, 2024
@joocer joocer mentioned this issue Apr 19, 2024
4 tasks
joocer added a commit that referenced this issue Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Something isn't working High Priority 1️⃣
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant