-
Notifications
You must be signed in to change notification settings - Fork 13.9k
[FLINK-24547][Table SQL / Planner]A function named row_kind() can get… #17483
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
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 3bc3287 (Thu Oct 14 12:29:47 UTC 2021) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
Airblader
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.
There are several things missing from this PR:
- Expose the function in Table API (Java + Scala) + JavaDocs / ScalaDocs
- Expose the function in PyFlink + its docs
- Document this in the list of built-in functions.
- Add tests for SQL and Table API.
I only reviewed this preliminarily, I don't know whether this function will be accepted or not.
| |$resultTerm = org.apache.flink.table.data.binary.BinaryStringData.fromString( | ||
| |$DEFAULT_INPUT1_TERM.getRowKind().shortString()); | ||
| |""".stripMargin.trim | ||
| // the proctime has been materialized, so it's TIMESTAMP now, not PROCTIME_INDICATOR |
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.
This comment makes no sense. Copy paste?
| isCharacterString(operands(2).resultType) => | ||
| methodGen(BuiltInMethods.CONVERT_TZ) | ||
|
|
||
| case org.apache.flink.table.planner.functions.sql.FlinkSqlOperatorTable.ROW_KIND => |
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.
Please import ROW_KIND to shorten this.
| val resultTerm = ctx.addReusableLocalVariable(resultTypeTerm, "rowKind") | ||
| val resultCode = | ||
| s""" | ||
| |$resultTerm = org.apache.flink.table.data.binary.BinaryStringData.fromString( |
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.
There's a constant for this class name
|
@Airblader Thanks for your review, I made some changes |
|
This PR is being marked as stale since it has not had any activity in the last 180 days. If you are having difficulty finding a reviewer, please reach out to the [community](https://flink.apache.org/what-is-flink/community/). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 90 days, it will be automatically closed. |
|
This PR has been closed since it has not had any activity in 120 days. |
Add a function can get row_kind, the function return "+I","-U","+U","-D"
you can use this function as "select row_kind() as op from xxx".