Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporary work-around to bypass function arguments being treated as expressions with columns. #5382

Merged
merged 1 commit into from
May 14, 2020

Conversation

mayankshriv
Copy link
Contributor

Only aggregation functions have the knowledge on how to interpret their arguments.
However, the assumption within ServerQueryRequest is that all expression arguments will contain
columns to be collected. A clean fix would be to create aggregation functions upfront and let
them interpret the arguments. However, until that happens, working-around by assuming that first
argument contains all the columns (to be used by DataSchemaPruner), which is true for all functions.

A side effect would be that DataSchemaPruner won't work for DistinctCountThetaSketchAggregatinoFunction.
But since this pruner is only for schema mis-matches, and not for performance, there should not be
an impact.

Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

…xpressions with columns.

Only aggregation functions have the knowledge on how to interpret their arguments.
However, the assumption within ServerQueryRequest is that all expression arguments will contain
columns to be collected. A clean fix would be to create aggregation functions upfront and let
them interpret the arguments. However, until that happens, working-around by assuming that first
argument contains all the columns (to be used by DataSchemaPruner), which is true for all functions.

A side effect would be that DataSchemaPruner won't work for DistinctCountThetaSketchAggregatinoFunction.
But since this pruner is only for schema mis-matches, and not for performance, there should not be
an impact.
@mayankshriv mayankshriv merged commit 753dd34 into apache:master May 14, 2020
@mayankshriv mayankshriv deleted the theta-sketch branch May 14, 2020 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants