Skip to content

throw exception while non-parametric functions having parameters#48115

Merged
Avogar merged 7 commits intoClickHouse:masterfrom
save-my-heart:throw_non_parametric_function
Apr 4, 2023
Merged

throw exception while non-parametric functions having parameters#48115
Avogar merged 7 commits intoClickHouse:masterfrom
save-my-heart:throw_non_parametric_function

Conversation

@save-my-heart
Copy link
Copy Markdown
Contributor

@save-my-heart save-my-heart commented Mar 28, 2023

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
}
}

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

@novikd novikd May 2, 2023

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
@den-crane den-crane mentioned this pull request Sep 5, 2024
Closed
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.

5 participants

@save-my-heart @novikd @Avogar @ucasfl @robot-ch-test-poll3