Skip to content

Stats function pushdown cleanup#6685

Closed
hashhar wants to merge 1 commit intotrinodb:masterfrom
hashhar:hashhar/cleanup-pushdowns
Closed

Stats function pushdown cleanup#6685
hashhar wants to merge 1 commit intotrinodb:masterfrom
hashhar:hashhar/cleanup-pushdowns

Conversation

@hashhar
Copy link
Copy Markdown
Member

@hashhar hashhar commented Jan 22, 2021

Cleans up SqlServer pushdown implementations to be more consistent.

Also documents why covar_samp and covar_pop are not implemented.

@cla-bot cla-bot bot added the cla-signed label Jan 22, 2021
@hashhar hashhar added the WIP label Jan 22, 2021
@hashhar hashhar force-pushed the hashhar/cleanup-pushdowns branch from b89ef89 to 236d00b Compare January 22, 2021 07:28
@hashhar hashhar removed the WIP label Jan 22, 2021
@hashhar hashhar marked this pull request as ready for review January 22, 2021 07:28
{
return basicAggregation()
.with(functionName().equalTo("stddev_pop"))
.with(functionName().matching(name -> name.equalsIgnoreCase("stddev_pop")))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i see .with(functionName().equalto 10 times in the codebase.

We should rather change the functionName().equalto impl, that "workaround" it in every place

Copy link
Copy Markdown
Member Author

@hashhar hashhar Jan 25, 2021

Choose a reason for hiding this comment

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

I'm creating a new PR for just the comments added to SqlServerClient about the missing pushdowns.

Will update this PR with suggested changes before the upcoming release.

{
Variable input = captures.get(INPUT);
JdbcColumnHandle columnHandle = (JdbcColumnHandle) context.getAssignment(input.getName());
verifyNotNull(columnHandle, "Unbound variable: %s", input);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

redundant, already checked by ontext.getAssignment

@hashhar
Copy link
Copy Markdown
Member Author

hashhar commented Feb 10, 2021

This seems to have been covered in #6736.

@hashhar hashhar closed this Feb 10, 2021
@hashhar hashhar deleted the hashhar/cleanup-pushdowns branch February 10, 2021 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants