You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/08 14:10:32 UTC
[GitHub] [flink] matriv opened a new pull request, #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
matriv opened a new pull request, #19409:
URL: https://github.com/apache/flink/pull/19409
## What is the purpose of the change
Implement `CAST` from `BINARY`/`VARBINARY`/`BYTES` to `RAW(<class>)`.
## Brief change log
- Implement `BinaryToRawCastRule`
- Add support for explicit cast in `LogicalTypeCasts`
- Make changes to `RexSimplify` and `RexLiteral` to enable the cast
## Verifying this change
This change added tests and can be verified as follows:
- Added tests in `CastRulesTest`
- Added tests in `LogicalTypeCastsTest`
- Added tests in `CastFunctionMiscITCase`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): **no**
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: **no**
- The serializers: **no**
- The runtime per-record code paths (performance sensitive): **no**
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: **no**
- The S3 file system connector: **no**
## Documentation
- Does this pull request introduce a new feature? **yes**
- If yes, how is the feature documented? **docs**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a diff in pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
matriv commented on code in PR #19409:
URL: https://github.com/apache/flink/pull/19409#discussion_r867806761
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2024,7 +2029,9 @@ private RexNode simplifySearch(RexCall call, RexUnknownAs unknownAs) {
private RexNode simplifyCast(RexCall e) {
RexNode operand = e.getOperands().get(0);
operand = simplify(operand, UNKNOWN);
- if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())) {
+ if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())
Review Comment:
I've spent quite some time here, and couldn't find a workaround.
I could modify the PR and only allow it for TableAPI for the moment, and try to fix this in Calcite first.
If it's accepted, have a follow up PR to enable this for SQL in flink.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a diff in pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
matriv commented on code in PR #19409:
URL: https://github.com/apache/flink/pull/19409#discussion_r846159965
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2024,7 +2029,9 @@ private RexNode simplifySearch(RexCall call, RexUnknownAs unknownAs) {
private RexNode simplifyCast(RexCall e) {
RexNode operand = e.getOperands().get(0);
operand = simplify(operand, UNKNOWN);
- if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())) {
+ if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())
+ || e.getType() instanceof RawRelDataType) {
+ // || e.getType().getSqlTypeName() == SqlTypeName.OTHER) {
Review Comment:
Maybe we need this instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] slinkydeveloper commented on a diff in pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
slinkydeveloper commented on code in PR #19409:
URL: https://github.com/apache/flink/pull/19409#discussion_r847035102
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2024,7 +2029,9 @@ private RexNode simplifySearch(RexCall call, RexUnknownAs unknownAs) {
private RexNode simplifyCast(RexCall e) {
RexNode operand = e.getOperands().get(0);
operand = simplify(operand, UNKNOWN);
- if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())) {
+ if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())
+ || e.getType() instanceof RawRelDataType) {
+ // || e.getType().getSqlTypeName() == SqlTypeName.OTHER) {
Review Comment:
Not sure, OTHER catches other types in our type system, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
matriv commented on PR #19409:
URL: https://github.com/apache/flink/pull/19409#issuecomment-1092910642
To allow this cast we need to make changes to `RexSimplify` and `RexLiteral`.
If you agree with these, I'll update the javadocs of those classes to reference the changes we need.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19409:
URL: https://github.com/apache/flink/pull/19409#issuecomment-1092913816
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "f041c44622d047c780c6ebc125471c39fe3d97d1",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "f041c44622d047c780c6ebc125471c39fe3d97d1",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* f041c44622d047c780c6ebc125471c39fe3d97d1 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a diff in pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
matriv commented on code in PR #19409:
URL: https://github.com/apache/flink/pull/19409#discussion_r847061521
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2024,7 +2029,9 @@ private RexNode simplifySearch(RexCall call, RexUnknownAs unknownAs) {
private RexNode simplifyCast(RexCall e) {
RexNode operand = e.getOperands().get(0);
operand = simplify(operand, UNKNOWN);
- if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())) {
+ if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())
+ || e.getType() instanceof RawRelDataType) {
+ // || e.getType().getSqlTypeName() == SqlTypeName.OTHER) {
Review Comment:
For us, currently, only `RAW` is associated with `OTHER`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on a diff in pull request #19409: [FLINK-24577][table-planner] Implement Cast from BINARY to RAW
Posted by GitBox <gi...@apache.org>.
twalthr commented on code in PR #19409:
URL: https://github.com/apache/flink/pull/19409#discussion_r867767879
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2024,7 +2029,9 @@ private RexNode simplifySearch(RexCall call, RexUnknownAs unknownAs) {
private RexNode simplifyCast(RexCall e) {
RexNode operand = e.getOperands().get(0);
operand = simplify(operand, UNKNOWN);
- if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())) {
+ if (sameTypeOrNarrowsNullability(e.getType(), operand.getType())
Review Comment:
The change in this line and in `RexLiteral` is a blocker for this PR in my opinion. We should fix these things in Calcite first and only port the changes to the Flink code base, once merged in Calcite.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org