You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/08/14 10:17:27 UTC
[GitHub] [incubator-kyuubi] pan3793 opened a new pull request, #3230: Flink SQL engine supports run across versions
pan3793 opened a new pull request, #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
To make sure flink sql engine build against flink-1.15 can run on flink-1.14.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945462454
##########
.github/workflows/master.yml:
##########
@@ -111,12 +111,11 @@ jobs:
- '1.15'
flink-archive: [ "" ]
comment: [ "normal" ]
- # FIXME: Cross Flink versions verification is not supported yet
- # include:
- # - java: 8
- # flink: '1.15'
- # flink-archive: '-Dflink.archive.mirror=https://archive.apache.org/dist/flink/flink-1.14.5 -Dflink.archive.name=flink-1.14.5-bin-scala_2.12.tgz'
- # comment: 'verify-flink-1.14'
+ include:
+ - java: 8
+ flink: '1.15'
+ flink-archive: '-Dflink.archive.mirror=https://archive.apache.org/dist/flink/flink-1.14.5 -Dflink.archive.name=flink-1.14.5-bin-scala_2.12.tgz'
+ comment: 'verify-flink-1.14'
Review Comment:
changed to `verify-on-flink-1.14-binary`, 1.15 is acutally the default flink version, we add this verification to ensure the default released binary tgz works on all supported versions of flink binary.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214712451
Thanks all for review, merging to master
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] deadwind4 commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214679166
LGTM.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214538582
cc @link3280
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945449843
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkEngineUtils.scala:
##########
@@ -46,11 +46,20 @@ object FlinkEngineUtils extends Logging {
logger.info(s"The current Flink version is $flinkVersion")
case _ =>
throw new UnsupportedOperationException(
- s"The current Flink version is $flinkVersion, " +
- s"Only Flink 1.14.x and 1.15 are supported, not supported in other versions")
+ s"You are using unsupported Flink version $flinkVersion, " +
+ s"only Flink 1.14 and 1.15 are supported now.")
Review Comment:
good suggestion, updated
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] SteNicholas commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945436302
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/operation/ExecuteStatement.scala:
##########
@@ -116,17 +118,16 @@ class ExecuteStatement(
(1 to result.getPayload).foreach { page =>
if (rows.size < resultMaxRows) {
// FLINK-24461 retrieveResultPage method changes the return type from Row to RowData
- val result = executor.retrieveResultPage(resultId, page).asScala.toList
- result.headOption match {
- case None =>
- case Some(r) =>
- // for flink 1.14
- if (r.getClass == classOf[Row]) {
- rows ++= result.asInstanceOf[List[Row]]
- } else {
- // for flink 1.15+
- rows ++= result.map(r => convertToRow(r.asInstanceOf[RowData], dataTypes))
- }
+ val retrieveResultPage = DynMethods.builder("retrieveResultPage")
+ .impl(executor.getClass, classOf[String], classOf[Int])
+ .build(executor)
+ val _page = Integer.valueOf(page)
+ if (FlinkEngineUtils.isFlinkVersionEqualTo("1.14")) {
Review Comment:
It‘s better to use the `FlinkVersion` enum here.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] deadwind4 commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945453655
##########
.github/workflows/master.yml:
##########
@@ -111,12 +111,11 @@ jobs:
- '1.15'
flink-archive: [ "" ]
comment: [ "normal" ]
- # FIXME: Cross Flink versions verification is not supported yet
- # include:
- # - java: 8
- # flink: '1.15'
- # flink-archive: '-Dflink.archive.mirror=https://archive.apache.org/dist/flink/flink-1.14.5 -Dflink.archive.name=flink-1.14.5-bin-scala_2.12.tgz'
- # comment: 'verify-flink-1.14'
+ include:
+ - java: 8
+ flink: '1.15'
+ flink-archive: '-Dflink.archive.mirror=https://archive.apache.org/dist/flink/flink-1.14.5 -Dflink.archive.name=flink-1.14.5-bin-scala_2.12.tgz'
+ comment: 'verify-flink-1.14'
Review Comment:
```suggestion
comment: 'verify-engine-1.15-on-flink-1.14 '
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214358891
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3230](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9c01955) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/e847ab3566f8cbc840fed2216b22ee7c12655b39?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e847ab3) will **decrease** coverage by `0.82%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #3230 +/- ##
============================================
- Coverage 51.68% 50.86% -0.83%
Complexity 6 6
============================================
Files 464 468 +4
Lines 25651 26035 +384
Branches 3551 3601 +50
============================================
- Hits 13259 13242 -17
- Misses 11133 11525 +392
- Partials 1259 1268 +9
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../java/org/apache/kyuubi/reflection/DynClasses.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL3JlZmxlY3Rpb24vRHluQ2xhc3Nlcy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../org/apache/kyuubi/reflection/DynConstructors.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL3JlZmxlY3Rpb24vRHluQ29uc3RydWN0b3JzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...n/java/org/apache/kyuubi/reflection/DynFields.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL3JlZmxlY3Rpb24vRHluRmllbGRzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../java/org/apache/kyuubi/reflection/DynMethods.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL3JlZmxlY3Rpb24vRHluTWV0aG9kcy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../kyuubi/server/mysql/constant/MySQLErrorCode.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvY29uc3RhbnQvTXlTUUxFcnJvckNvZGUuc2NhbGE=) | `13.84% <0.00%> (-6.16%)` | :arrow_down: |
| [...ache/kyuubi/server/mysql/MySQLCommandHandler.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxDb21tYW5kSGFuZGxlci5zY2FsYQ==) | `76.13% <0.00%> (-3.41%)` | :arrow_down: |
| [...ache/kyuubi/server/mysql/MySQLGenericPackets.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxHZW5lcmljUGFja2V0cy5zY2FsYQ==) | `76.59% <0.00%> (-2.13%)` | :arrow_down: |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `78.00% <0.00%> (-2.00%)` | :arrow_down: |
| [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `80.82% <0.00%> (-1.37%)` | :arrow_down: |
| [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9Mb2dnaW5nLnNjYWxh) | `43.42% <0.00%> (-1.32%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/3230/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945269581
##########
.github/workflows/master.yml:
##########
@@ -128,11 +127,21 @@ jobs:
java-version: ${{ matrix.java }}
cache: 'maven'
check-latest: false
- - name: Build and test Flink with maven w/o linters
- run: |
- TEST_MODULES="externals/kyuubi-flink-sql-engine,integration-tests/kyuubi-flink-it"
+ - name: Build Flink with maven w/o linters
+ run: |
+ ENGINE_MODULE=externals/kyuubi-flink-sql-engine
+ IT_MODULE=integration-tests/kyuubi-flink-it
+ TEST_MODULES="${ENGINE_MODULE},${IT_MODULE}"
./build/mvn ${MVN_OPT} -pl ${TEST_MODULES} -Pflink-${{ matrix.flink }} ${{ matrix.flink-archive }} -am clean install -DskipTests
+ - name: Test Flink
+ if: matrix.flink-archive == ''
+ run: |
./build/mvn ${MVN_OPT} -pl ${TEST_MODULES} -Pflink-${{ matrix.flink }} ${{ matrix.flink-archive }} test
+ - name: Cross-version test Flink
+ if: matrix.flink-archive != ''
+ run: |
+ IT_FLINK=`echo "${{ matrix.flink-archive }}" | grep -E 'flink\-([0-9]+\.[0-9]+.[0-9]+)\-bin' -o | grep -E '[0-9]+\.[0-9]+' -o`
+ ./build/mvn ${MVN_OPT} -pl ${IT_MODULE} -Pflink-${IT_FLINK} ${{ matrix.flink-archive }} test
Review Comment:
cc @deadwind4 FYI
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214590348
Also cc @benjobs
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
link3280 commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214619199
@pan3793 The newly introduced reflection utils seem very promising! I would take a detailed look later today. 👍🏼
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] SteNicholas commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945436302
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/operation/ExecuteStatement.scala:
##########
@@ -116,17 +118,16 @@ class ExecuteStatement(
(1 to result.getPayload).foreach { page =>
if (rows.size < resultMaxRows) {
// FLINK-24461 retrieveResultPage method changes the return type from Row to RowData
- val result = executor.retrieveResultPage(resultId, page).asScala.toList
- result.headOption match {
- case None =>
- case Some(r) =>
- // for flink 1.14
- if (r.getClass == classOf[Row]) {
- rows ++= result.asInstanceOf[List[Row]]
- } else {
- // for flink 1.15+
- rows ++= result.map(r => convertToRow(r.asInstanceOf[RowData], dataTypes))
- }
+ val retrieveResultPage = DynMethods.builder("retrieveResultPage")
+ .impl(executor.getClass, classOf[String], classOf[Int])
+ .build(executor)
+ val _page = Integer.valueOf(page)
+ if (FlinkEngineUtils.isFlinkVersionEqualTo("1.14")) {
Review Comment:
It‘s better to use the `FlinkVersion` enum here.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945269546
##########
.github/workflows/master.yml:
##########
@@ -128,11 +127,21 @@ jobs:
java-version: ${{ matrix.java }}
cache: 'maven'
check-latest: false
- - name: Build and test Flink with maven w/o linters
- run: |
- TEST_MODULES="externals/kyuubi-flink-sql-engine,integration-tests/kyuubi-flink-it"
+ - name: Build Flink with maven w/o linters
+ run: |
+ ENGINE_MODULE=externals/kyuubi-flink-sql-engine
+ IT_MODULE=integration-tests/kyuubi-flink-it
+ TEST_MODULES="${ENGINE_MODULE},${IT_MODULE}"
./build/mvn ${MVN_OPT} -pl ${TEST_MODULES} -Pflink-${{ matrix.flink }} ${{ matrix.flink-archive }} -am clean install -DskipTests
+ - name: Test Flink
+ if: matrix.flink-archive == ''
+ run: |
./build/mvn ${MVN_OPT} -pl ${TEST_MODULES} -Pflink-${{ matrix.flink }} ${{ matrix.flink-archive }} test
+ - name: Cross-version test Flink
+ if: matrix.flink-archive != ''
+ run: |
+ IT_FLINK=`echo "${{ matrix.flink-archive }}" | grep -E 'flink\-([0-9]+\.[0-9]+.[0-9]+)\-bin' -o | grep -E '[0-9]+\.[0-9]+' -o`
+ ./build/mvn ${MVN_OPT} -pl ${IT_MODULE} -Pflink-${IT_FLINK} ${{ matrix.flink-archive }} test
Review Comment:
The cross-version verification composes of:
- flink-sql-engine compiled against flink-1.15
- flink-1.14 binary tgz
- flink-1.14 mini cluster
- flink-1.14 it test suites
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#issuecomment-1214590618
Also cc @wolfboys
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] SteNicholas commented on a diff in pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on code in PR #3230:
URL: https://github.com/apache/incubator-kyuubi/pull/3230#discussion_r945445919
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkEngineUtils.scala:
##########
@@ -46,11 +46,20 @@ object FlinkEngineUtils extends Logging {
logger.info(s"The current Flink version is $flinkVersion")
case _ =>
throw new UnsupportedOperationException(
- s"The current Flink version is $flinkVersion, " +
- s"Only Flink 1.14.x and 1.15 are supported, not supported in other versions")
+ s"You are using unsupported Flink version $flinkVersion, " +
+ s"only Flink 1.14 and 1.15 are supported now.")
Review Comment:
Please maintain a supported Flink version list here.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 closed pull request #3230: Flink SQL engine supports run across versions
Posted by GitBox <gi...@apache.org>.
pan3793 closed pull request #3230: Flink SQL engine supports run across versions
URL: https://github.com/apache/incubator-kyuubi/pull/3230
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org