Skip to content

add missing duplicates of join keys to RS schema#3504

Closed
kasakrisz wants to merge 3 commits intoapache:masterfrom
kasakrisz:master-mapjoin
Closed

add missing duplicates of join keys to RS schema#3504
kasakrisz wants to merge 3 commits intoapache:masterfrom
kasakrisz:master-mapjoin

Conversation

@kasakrisz
Copy link
Contributor

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Copy link

@amansinha100 amansinha100 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Is there a JIRA id for this ?

return rsOp;
}

private void addJoinKeyToRowScema(

Choose a reason for hiding this comment

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

nit: spelling: RowSchema (missing 'h')

if (exprBack != null) {
if (ExprNodeDescUtils.isConstant(exprBack)) {
int kindex = reduceKeysBack.indexOf(exprBack);
addJoinKeyToRowScema(outputRR, index, i, colInfo, nm, nm2, kindex);

Choose a reason for hiding this comment

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

Previously we were checking if kindex >= 0 before adding to the output row schema. We should check here as well.

int kindex;
// joinKey may present multiple times, add the duplicates to the schema with different internal name
// join LU_CUSTOMER a16
// on (a15.CUSTOMER_ID = a16.CUSTOMER_ID and pa11.CUSTOMER_ID = a16.CUSTOMER_ID)

Choose a reason for hiding this comment

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

For clarity, could you pls add the internal name that would be produced for the 2 occurrences of a16.CUSTOMER_ID ?
Also, the duplicate occurrences could be in a WHERE clause instead of ON clause ..can we verify if that case also works.

@kasakrisz
Copy link
Contributor Author

Final patch: #3505

@kasakrisz kasakrisz closed this Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants