WINDOWING - Fix 2 nodes with same digest causing mapping issue#16301
WINDOWING - Fix 2 nodes with same digest causing mapping issue#16301LakshSingla merged 5 commits intoapache:masterfrom
Conversation
| public void testWindow() | ||
| { | ||
| testBuilder() | ||
| .sql("SELECT\n" + |
There was a problem hiding this comment.
A test case in MSQ window functions would also be helpful.
There was a problem hiding this comment.
great idea :)
after the separation of test suppliers patch gets in that we'll be even easier to do!
| partialQuery.getWindowProject().getProjects().stream().map(RexNode::toString).collect(Collectors.toSet()).size() | ||
| < partialQuery.getWindowProject().getProjects().size() | ||
| ? Project.getPartialMapping( | ||
| partialQuery.getWindowProject().getRowType().getFieldCount(), | ||
| partialQuery.getWindowProject().getProjects() | ||
| ) | ||
| : partialQuery.getWindowProject().getMapping(), | ||
| "mapping for windowProject[%s]", partialQuery.getWindowProject() |
There was a problem hiding this comment.
discussed offline; but leave a note here:
I believe we want a real PartialMapping ; you could copy getPartialMapping over and change
INVERSE_FUNCTION to INVERSE_PARTIAL_FUNCTION
that way we will support partial mapping even in case columns are being discarded...
I also think that Project.getPartialMapping might need this change as its apidoc suggersts that a source might have 0, 1 or more targets.
There was a problem hiding this comment.
didn't had to change the mapping type. both INVERSE_FUNCTION & INVERSE_PARTIAL_FUNCTION are internally using PartialFunctionImpl
made use of source might have 0 targets by providing the field count as 1 + highest index in our window projection
| public void testWindow() | ||
| { | ||
| testBuilder() | ||
| .sql("SELECT\n" + |
There was a problem hiding this comment.
great idea :)
after the separation of test suppliers patch gets in that we'll be even easier to do!
github-actions
Bot
added
Area - Batch Ingestion
Area - MSQ
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| @MethodSource("data") | ||
| @ParameterizedTest(name = "{index}:with context {0}") | ||
| public void testSimpleWindowWithDuplicateSelectNode(String contextName, Map<String, Object> context) |
Check notice
Code scanning / CodeQL
Useless parameter
| // as defensive programming. | ||
| // We know windowProject is a mapping due to the isMapping() check in DruidRules. | ||
| // check anyway as defensive programming. | ||
| Preconditions.checkArgument(partialQuery.getWindowProject().isMapping()); |
There was a problem hiding this comment.
possibly extracting partialQuery.getWindowProject() into a local variable may make it more readable
6 participants
Description
The PR aims to fixing the mapping issue in window fxns where 2 nodes get the same reference.
example:
SELECT(rank() over (order by count(*) desc)),(rank() over (order by count(*) desc))FROM tableKey changed/added classes in this PR
Windowing.javaThis PR has: