Skip to content

Better error message when theta_sketch_intersect is used on scalar expression#13508

Merged
kfaraz merged 2 commits intoapache:masterfrom
abhishekagarwal87:theta_sketch_intersect_error
Dec 7, 2022
Merged

Better error message when theta_sketch_intersect is used on scalar expression#13508
kfaraz merged 2 commits intoapache:masterfrom
abhishekagarwal87:theta_sketch_intersect_error

Conversation

@abhishekagarwal87
Copy link
Copy Markdown
Contributor

This PR has:

  • been self-reviewed.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

)
{
plannerContext.setPlanningError("%s can only be used on aggregates. " +
"It cannot be used directly on a column or or on a scalar expression.", getFunctionName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo: double or

Since the error message is specific to theta sketch, maybe we can mention the exact type of arguments this function expects, rather than mentioning what it cannot accept.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the type check is already done. This tells the user that they are trying to use the function as an expression while its only available as a post aggregator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, okay. Thanks for the clarification.

Copy link
Copy Markdown
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

+1 after fixing typo

@kfaraz kfaraz merged commit b25cf21 into apache:master Dec 7, 2022
@clintropolis clintropolis added this to the 26.0 milestone Apr 10, 2023
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