fix(schema-compiler) Resolve rollup join pre-aggregation param allocator issue #8279
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 8 Ignored Deployments
|
github-actions
bot
added
the
pr:community
| // By default, we'll use BaseQuery, but it's important to note that different databases (Oracle, PostgreSQL, MySQL, Druid, etc.) | ||
| // have unique parameter allocator symbols. Using the wrong allocator can break the query, especially when rollup joins involve | ||
| // different cubes that require different allocators. | ||
| return this.options.queryFactory.createQuery(cube, this.compilers, { ...this.subQueryOptions(options), paramAllocator: undefined }); |
There was a problem hiding this comment.
Unset the paramAllocator set by this.subQueryOptions to allow QueryClasses to assume their default paramAllocator class
pauldheinrichs
changed the title
|
@pauldheinrichs Thanks for contributing! Could you please set the allocator to undefined in the branch without queryFactory as well? It should be consistent. |
@paveltiunov For sure, i've updated the |
|
@paveltiunov sorry for the ping - curious if there's any other adjustments needed from my end here to push this forward! |
| newSubQuery(options) { | ||
| const QueryClass = this.constructor; | ||
| return new QueryClass(this.compilers, this.subQueryOptions(options)); | ||
| return new QueryClass(this.compilers, { ...this.subQueryOptions(options), paramAllocator: undefined }); |
There was a problem hiding this comment.
We can't unset param allocator here as it should reused in case of actual sub_query: true members.
There was a problem hiding this comment.
What would you recommend here then, reverting the last commit and pushing forward with just the initial implementation?
|
Closing as it was fixed in #8619 |
2 participants
Check List
Issue Reference this PR resolves
Resolves: #8268
Description of Changes Made (if issue reference is not provided)
Resolves:
Summary:
So this is a fun one, here's the situation. At first i thought this was entirely community druid problem but tracked it down to an underlying issue with rollup_joins.
If we rollup_join two datasources that contain a different paramAllocator symbol (IE:
$1and?) the default paramAllocator would inject the wrong parameters into the pre-aggregation sql code invalidating the cache on the API node and causing weird behaviour. This would only present itself when actually querying the data and thus causing issues.Smoke tested:
Thanks ❤️