Changed how empty set of projected columns is handled #1587
Conversation
…umns to a ProjectedTable
…aliases and columns in some contexts
|
Max, I need your eyes on the changes to the plans. They look reasonable at first glance but there's a lot of them and you're more familiar than I am. We are now pruning lots of columns we didn't before. |
enginetest/queries/query_plans.go
Outdated
| " │ ├─ static: [{[1, 1]}]\n" + | ||
| " │ └─ Table\n" + | ||
| " │ └─ name: one_pk\n" + | ||
| " │ └─ static: [{[1, 1]}]\n" + |
There was a problem hiding this comment.
this one is missing a columns field
enginetest/queries/query_plans.go
Outdated
| " ├─ static: [{[1, 1], [2, 2]}]\n" + | ||
| " └─ Table\n" + | ||
| " └─ name: two_pk\n" + | ||
| " └─ static: [{[1, 1], [2, 2]}]\n" + |
enginetest/queries/query_plans.go
Outdated
| " ├─ static: [{[1, 1], [2, 2]}]\n" + | ||
| " └─ Table\n" + | ||
| " └─ name: two_pk\n" + | ||
| " └─ static: [{[1, 1], [2, 2]}]\n" + |
enginetest/queries/query_plans.go
Outdated
| " │ │ │ │ │ └─ Table\n" + | ||
| " │ │ │ │ │ └─ name: WRZVO\n" + | ||
| " │ │ │ │ │ └─ IndexedTableAccess(WRZVO)\n" + | ||
| " │ │ │ │ │ └─ index: [WRZVO.TVNW2]\n" + |
enginetest/queries/query_plans.go
Outdated
| " │ │ └─ Table\n" + | ||
| " │ │ └─ name: TPXBU\n" + | ||
| " │ │ └─ IndexedTableAccess(TPXBU)\n" + | ||
| " │ │ └─ index: [TPXBU.id]\n" + |
There was a problem hiding this comment.
all of these missing cols
enginetest/queries/query_plans.go
Outdated
| " │ └─ Table\n" + | ||
| " │ └─ name: OUBDL\n" + | ||
| " │ └─ IndexedTableAccess(OUBDL)\n" + | ||
| " │ └─ index: [OUBDL.ZH72S]\n" + |
| " │ ├─ rn.WNUNU:89 IS NULL\n" + | ||
| " │ └─ rn.HHVLX:90 IS NULL\n" + | ||
| " │ ├─ rn.WNUNU:15 IS NULL\n" + | ||
| " │ └─ rn.HHVLX:16 IS NULL\n" + |
There was a problem hiding this comment.
damn, should be a lot faster
There was a problem hiding this comment.
this is query b3, as a reference for tesing correctness next week
| @@ -9998,10 +9056,9 @@ WHERE | |||
| " │ │ │ ├─ aac.id:67!null\n" + | |||
| " │ │ │ └─ SL3S5.M22QN:53!null\n" + | |||
| " │ │ └─ TableAlias(aac)\n" + | |||
| " │ │ └─ IndexedTableAccess\n" + | |||
| " │ │ └─ IndexedTableAccess(TPXBU)\n" + | |||
| @@ -10761,34 +9791,34 @@ WHERE | |||
| " ├─ outerVisibility: false\n" + | |||
| " ├─ cacheable: true\n" + | |||
| " └─ Project\n" + | |||
| " ├─ columns: [bs.T4IBQ:1!null as T4IBQ, pa.DZLIM:3 as ECUWU, pga.DZLIM:15 as GSTQA, pog.B5OUF:13, fc.OZTQF:45, F26ZW.YHYLK:51, nd.TW55N:20 as TW55N]\n" + | |||
| " ├─ columns: [bs.T4IBQ:1!null as T4IBQ, pa.DZLIM:3 as ECUWU, pga.DZLIM:12 as GSTQA, pog.B5OUF:10, fc.OZTQF:20, F26ZW.YHYLK:24, nd.TW55N:14 as TW55N]\n" + | |||
|
I'm not seeing any noticeable differences on the sample integration test comparisons. |
You mean that you're not seeing any difference in performance on the actual integration test queries? This would make sense if the casing of the original queries were correct and we messed that up when obfuscating. I think this passes other tests now, so give it a final thumbs or thumbs down when you have a minute. |
Started with tightening the semantics of the
ProjectedTableinterface as it relates to an empty projection (nil v. empty slice). Then made changes to printing ofResolvedTableandIndexedTableAccess. This revealed problems in the prune columns rule when all columns were being pruned. Fixed those, which had been masking other bugs, where we hadn't been pruning when we could have been. This was in turn caused by other bugs in the prune rule dealing with case sensitivity.We should now be able to prune more columns than before, and we can see an empty column projection in plans.