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.

Sign up for GitHub

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

Jump to bottom

throw exception while non-parametric functions having parameters #48115

Merged
merged 7 commits into from
Apr 4, 2023
Merged

throw exception while non-parametric functions having parameters #48115

merged 7 commits into from
Apr 4, 2023

Conversation

save-my-heart
Copy link
Contributor

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Now non-parametric functions only validate 'arguments', any 'parameters' will be ignored, these may cause unexpected behavior in some cases.
See: #48089

@Avogar Avogar self-assigned this Mar 28, 2023
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 28, 2023
@ucasfl ucasfl added the can be tested Allows running workflows for external contributors label Mar 29, 2023
@Avogar Avogar merged commit 5f930ae into ClickHouse:master Apr 4, 2023
@save-my-heart save-my-heart deleted the throw_non_parametric_function branch April 4, 2023 10:58
novikd
novikd reviewed May 2, 2023
View reviewed changes
src/Interpreters/ActionsVisitor.cpp
@@ -1109,6 +1110,12 @@ void ActionsMatcher::visit(const ASTFunction & node, const ASTPtr & ast, Data &
}
}

/// Normal functions are not parametric for now.
Copy link
Member

Choose a reason for hiding this comment

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

This if is not correct because executable UDF can have parameters. See #38142, #37929 and #30957.

@novikd novikd mentioned this pull request May 2, 2023
Merged
@nickitat nickitat mentioned this pull request Jul 7, 2023
Merged
alexey-milovidov added a commit that referenced this pull request Jul 7, 2023
@alexey-milovidov
Revert "Merge pull request #48115 from save-my-heart/throw_non_parame…
a968748 
…tric_function"

This reverts commit 5f930ae, reversing
changes made to 3557232.
@alexey-milovidov alexey-milovidov mentioned this pull request Jul 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@novikd novikd novikd left review comments

@Avogar Avogar Avogar approved these changes

Assignees

@Avogar Avogar

Labels
can be tested Allows running workflows for external contributors pr-not-for-changelog This PR should not be mentioned in the changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@save-my-heart @novikd @Avogar @ucasfl @robot-ch-test-poll3