-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-37778][1/N] introduce MODEL keyword for model as argument #26530
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
Conversation
e91f4c3 to
e45d7ff
Compare
fsk119
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. I just have one question.
| import java.util.List; | ||
|
|
||
| /** SqlExplicitModelCall is a SQL call that represents an explicit model. */ | ||
| public class SqlExplicitModelCall extends SqlBasicCall { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? We can use SqlOperator#createCall to build the SqlCall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm planning to use this to get and store the model based on functionQualifier later in SqlValidatorImpl. It won't be possible with just SqlBasicCall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I think we can discuss more implement details when validating sql node.
What is the purpose of the change
Parse
MODELkeyword in function argument and convert it toSqlExplicitModelCallwithSqlExplicitModelOperator.Brief change log
MODELkeyword in function argument.SqlExplicitModelCallsql node for model argumentSqlExplicitModelOperatorsql operatorVerifying this change
Added unit test for parser changes
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes)Documentation