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/12/13 09:52:17 UTC
[GitHub] [flink] snuyanzin opened a new pull request, #21500: [FLINK-27995][table] Upgrade Janino version
snuyanzin opened a new pull request, #21500:
URL: https://github.com/apache/flink/pull/21500
## What is the purpose of the change
This is a draft version of Janino's upgrade
In https://github.com/apache/flink/pull/21203 Calcite was updated to 1.28.0
However Janino was still 3.0.11
The reason is that there is still of issues on Janino 3.1.x for the fine working cases with Janino 3.0.11
https://github.com/janino-compiler/janino/issues/185
https://github.com/janino-compiler/janino/issues/187
https://github.com/janino-compiler/janino/issues/188
Currently it seems that now only one is not fixed (looks like it is fixed partially in master)
There is a workaround for that at `org.apache.flink.table.planner.codegen.CodeGenUtils#genToExternalConverterAllWithLegacy` (hope it will be fixed soon and workaround will be removed)
## Verifying this change
It looks like it should be covered by tests in flink-table-planner
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes )
- 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? ( no)
- If yes, how is the feature documented? (not applicable)
--
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] snuyanzin merged pull request #21500: [FLINK-27995][table] Upgrade Janino version to 3.1.9
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #21500:
URL: https://github.com/apache/flink/pull/21500
--
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 #21500: [FLINK-27995][table] Upgrade Janino version
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #21500:
URL: https://github.com/apache/flink/pull/21500#issuecomment-1348089154
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "cec7edbce8b10f2f543e70a14cdfeca322682a21",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "cec7edbce8b10f2f543e70a14cdfeca322682a21",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* cec7edbce8b10f2f543e70a14cdfeca322682a21 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] fsk119 commented on pull request #21500: [FLINK-27995][table] Upgrade Janino version
Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 commented on PR #21500:
URL: https://github.com/apache/flink/pull/21500#issuecomment-1427291971
@snuyanzin I am not familiar with janino... I think you may need to find others to review this.
--
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] snuyanzin commented on pull request #21500: [FLINK-27995][table] Upgrade Janino version to 3.1.9
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #21500:
URL: https://github.com/apache/flink/pull/21500#issuecomment-1445435871
Rebased to resolve ci issue
```
Reading package lists...
E: Unsupported file ./libssl1.0.0_1.0.2n-1ubuntu5.10_amd64.deb given on commandline
##[error]Bash exited with code '100'.
```
https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=46553&view=logs&j=bea52777-eaf8-5663-8482-18fbc3630e81&t=11b0df07-3e5e-58da-eb81-03003e470195&l=1836
--
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] snuyanzin commented on pull request #21500: [FLINK-27995][table] Upgrade Janino version
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #21500:
URL: https://github.com/apache/flink/pull/21500#issuecomment-1423914353
@fsk119 since you are a reporter of FLINK-27995
could you have a look here please?
--
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 #21500: [FLINK-27995][table] Upgrade Janino version to 3.1.9
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #21500:
URL: https://github.com/apache/flink/pull/21500#discussion_r1117229017
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGenUtils.scala:
##########
@@ -1020,16 +1021,19 @@ object CodeGenUtils {
}
// convert internal format to target type
- val externalResultTerm = if (isInternalClass(targetDataType)) {
- s"($targetTypeTerm) ${internalExpr.resultTerm}"
+ val (externalResultTerm, resultType) = if (isInternalClass(targetDataType)) {
Review Comment:
can we use more meaningful names here? `resultType` does not improve code readability. `externalResultTypeTerm` is the right name here.
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGenUtils.scala:
##########
@@ -922,7 +922,8 @@ object CodeGenUtils {
if (targetDataType.getConversionClass.isPrimitive) {
externalResultTerm
} else {
- s"${internalExpr.nullTerm} ? null : ($externalResultTerm)"
+ // Cast of null is required because of janino issue https://github.com/janino-compiler/janino/issues/188
+ s"${internalExpr.nullTerm} ? (${typeTerm(targetDataType.getConversionClass)}) null : ($externalResultTerm)"
Review Comment:
introduce a variable `externalResultTypeTerm = typeTerm(targetDataType.getConversionClass)`
--
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] lincoln-lil commented on pull request #21500: [FLINK-27995][table] Upgrade Janino version
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on PR #21500:
URL: https://github.com/apache/flink/pull/21500#issuecomment-1427949339
@twalthr do you have any concern on this pr?
--
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] snuyanzin commented on pull request #21500: [FLINK-27995][table] Upgrade Janino version
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #21500:
URL: https://github.com/apache/flink/pull/21500#issuecomment-1396962825
@flinkbot run azure
--
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