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/03/27 03:26:11 UTC

[GitHub] [incubator-kyuubi] turboFei opened a new pull request #2225: Max res

turboFei opened a new pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225


   <!--
   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.
   -->
   
   
   ### _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
   
   - [ ] [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] codecov-commenter edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079838569


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?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 [#2225](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2a8aa6) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/86cd685de784f6ade61ea78502aae561cdb48a7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86cd685) will **decrease** coverage by `0.00%`.
   > The diff coverage is `86.36%`.
   
   > :exclamation: Current head f2a8aa6 differs from pull request most recent head 3823b11. Consider uploading reports for the commit 3823b11 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2225      +/-   ##
   ============================================
   - Coverage     61.80%   61.79%   -0.01%     
     Complexity       69       69              
   ============================================
     Files           331      331              
     Lines         16016    16034      +18     
     Branches       2031     2032       +1     
   ============================================
   + Hits           9899     9909      +10     
   - Misses         5295     5300       +5     
   - Partials        822      825       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.08% <71.42%> (-2.04%)` | :arrow_down: |
   | [...ine/spark/operation/SparkSQLOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vU3BhcmtTUUxPcGVyYXRpb25NYW5hZ2VyLnNjYWxh) | `84.90% <90.00%> (+0.81%)` | :arrow_up: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.16% <100.00%> (+0.02%)` | :arrow_up: |
   | [...org/apache/kyuubi/ha/client/ServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9TZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `89.28% <0.00%> (-3.58%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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) | `39.72% <0.00%> (-1.37%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [86cd685...3823b11](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835924075



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala
##########
@@ -136,4 +136,21 @@ class KyuubiOperationPerUserSuite extends WithKyuubiServer with SparkQueryTests
 
     assert(r1 !== r2)
   }
+
+  test("test engine spark result max rows") {
+    withSessionConf()(Map.empty)(Map(KyuubiConf.OPERATION_RESULT_MAX_ROWS.key -> "1")) {
+      withJdbcStatement() { statement =>

Review comment:
       thanks




-- 
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] yaooqinn commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835856217



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkSQLOperationManager.scala
##########
@@ -64,7 +65,15 @@ class SparkSQLOperationManager private (name: String) extends OperationManager(n
             case NONE =>
               val incrementalCollect = spark.conf.getOption(OPERATION_INCREMENTAL_COLLECT.key)
                 .map(_.toBoolean).getOrElse(operationIncrementalCollectDefault)
-              new ExecuteStatement(session, statement, runAsync, queryTimeout, incrementalCollect)
+              val resultMaxRows = spark.conf.getOption(ENGINE_SPARK_MAX_ROWS.key)

Review comment:
       move to ExecuteStatement

##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       kyuubi.operation.max.result.rows?

##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")
+      .doc("Max rows of Spark query results. Rows that exceeds the limit would be ignored. " +
+        "By setting this value to -1 to disable the max rows limit.")

Review comment:
       0




-- 
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 edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079838569


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?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 [#2225](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51983c3) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/86cd685de784f6ade61ea78502aae561cdb48a7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86cd685) will **increase** coverage by `0.01%`.
   > The diff coverage is `83.33%`.
   
   > :exclamation: Current head 51983c3 differs from pull request most recent head 5d827d1. Consider uploading reports for the commit 5d827d1 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2225      +/-   ##
   ============================================
   + Coverage     61.80%   61.81%   +0.01%     
     Complexity       69       69              
   ============================================
     Files           331      331              
     Lines         16016    16026      +10     
     Branches       2031     2032       +1     
   ============================================
   + Hits           9899     9907       +8     
   - Misses         5295     5296       +1     
   - Partials        822      823       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.35% <71.42%> (-1.78%)` | :arrow_down: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.16% <100.00%> (+0.02%)` | :arrow_up: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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) | `39.72% <0.00%> (-1.37%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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==) | `96.07% <0.00%> (+1.96%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [86cd685...5d827d1](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835920079



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       resolved




-- 
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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835859713



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       How about fall back to kyuubi.operation.max.result.rows if kyuubi.session.engine.*.max.rows is not specified?




-- 
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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835859607



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       I saw that there is a parameter named `kyuubi.session.engine.flink.max.rows`.




-- 
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 edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079890681






-- 
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] ulysses-you commented on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079884217


   > Can ForcedMaxOutputRowsRule be replaced by this one
   
   It's different in physical side. `dataset.take` just like the incremental mode which run job partition by partition if previous partitions data has not satisfied the limit number. `dataset.limit.collect` will run all partitions to do local limit then do global limit in one partition. So in general `dataset.limit.collect` is more effective.
   
   And this PR is useful. Since the limitation is controlled by client side using Incremental mode, but we can control it at engine side using `dataset.take`.


-- 
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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835916802



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       we can deprecate kyuubi.session.engine.flink.max.rows in another 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: 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] Nick-0723 commented on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
Nick-0723 commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079911332


   Does the user know that we truncate the data?


-- 
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 edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079838569


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?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 [#2225](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51983c3) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/86cd685de784f6ade61ea78502aae561cdb48a7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86cd685) will **increase** coverage by `0.01%`.
   > The diff coverage is `83.33%`.
   
   > :exclamation: Current head 51983c3 differs from pull request most recent head bb2acd8. Consider uploading reports for the commit bb2acd8 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2225      +/-   ##
   ============================================
   + Coverage     61.80%   61.81%   +0.01%     
     Complexity       69       69              
   ============================================
     Files           331      331              
     Lines         16016    16026      +10     
     Branches       2031     2032       +1     
   ============================================
   + Hits           9899     9907       +8     
   - Misses         5295     5296       +1     
   - Partials        822      823       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.35% <71.42%> (-1.78%)` | :arrow_down: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.16% <100.00%> (+0.02%)` | :arrow_up: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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) | `39.72% <0.00%> (-1.37%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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==) | `96.07% <0.00%> (+1.96%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [86cd685...bb2acd8](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov-commenter edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079838569


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?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 [#2225](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51983c3) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/86cd685de784f6ade61ea78502aae561cdb48a7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86cd685) will **increase** coverage by `0.01%`.
   > The diff coverage is `83.33%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2225      +/-   ##
   ============================================
   + Coverage     61.80%   61.81%   +0.01%     
     Complexity       69       69              
   ============================================
     Files           331      331              
     Lines         16016    16026      +10     
     Branches       2031     2032       +1     
   ============================================
   + Hits           9899     9907       +8     
   - Misses         5295     5296       +1     
   - Partials        822      823       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.35% <71.42%> (-1.78%)` | :arrow_down: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.16% <100.00%> (+0.02%)` | :arrow_up: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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) | `39.72% <0.00%> (-1.37%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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==) | `96.07% <0.00%> (+1.96%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [86cd685...51983c3](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835916802



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       we can deprecate kyuubi.session.engine.flink.max.rows in another pr.
   
   because the behavior is specific for flink.

##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       we can deprecate kyuubi.session.engine.flink.max.rows in another pr.
   
   because the behavior is special for 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: 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 edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079890681


   IIRC, the difference you described above is for incremental mode.
   
   w/ incremental mode, `CollectLimitExec#executeToIterator` will be called, which calls `getByteArrayRdd` and triggers to compute all partitions of previous stages firstly.
   
   w/o incremental mode, `CollectLimitExec#executeCollect` will be called, which submit partitions increase exponentially utils satisfy the numbers.
   


-- 
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 #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079890681


   IIRC, the difference you described above is for increment mode.
   
   w/ incremental mode, `CollectLimitExec#executeToIterator` will be called, which calls `getByteArrayRdd` and triggers to compute all partitions of previous stages firstly.
   
   w/o incremental mode, `CollectLimitExec#executeCollect` will be called, which submit partitions increase exponentially utils satisfy the numbers.
   


-- 
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] turboFei commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835891894



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       it is to align with `kyuubi.session.engine.flink.max.rows`




-- 
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 #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079842431


   Can `ForcedMaxOutputRowsRule` be replaced by this one? cc @cfmcgrady @ulysses-you @wForget 


-- 
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 edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079888703


   > `dataset.take` just like the incremental mode which run job partition by partition if previous partitions data has not satisfied the limit number.  `dataset.limit.collect` will run all partitions to do local limit then do global limit in one partition.
   
   I would argue that `dataset.take` is same as `dataset.limit.collect`
   
   ```
   def take(n: Int): Array[T] = head(n)
   def head(n: Int): Array[T] = withAction("head", limit(n).queryExecution)(collectFromPlan)
   def collect(): Array[T] = withAction("collect", queryExecution)(collectFromPlan)
   ```


-- 
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 #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079888703


   > `dataset.take` just like the incremental mode which run job partition by partition if previous partitions data has not satisfied the limit number.
   
   I would argue that `dataset.take` is same as `dataset.limit.collect`
   
   ```
   def take(n: Int): Array[T] = head(n)
   def head(n: Int): Array[T] = withAction("head", limit(n).queryExecution)(collectFromPlan)
   def collect(): Array[T] = withAction("collect", queryExecution)(collectFromPlan)
   ```


-- 
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] cfmcgrady commented on a change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835923876



##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala
##########
@@ -136,4 +136,21 @@ class KyuubiOperationPerUserSuite extends WithKyuubiServer with SparkQueryTests
 
     assert(r1 !== r2)
   }
+
+  test("test engine spark result max rows") {
+    withSessionConf()(Map.empty)(Map(KyuubiConf.OPERATION_RESULT_MAX_ROWS.key -> "1")) {
+      withJdbcStatement() { statement =>

Review comment:
       nit: `withJdbcStatement("va")`




-- 
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] cfmcgrady commented on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079953777


   > yea, if so I agree we can use this feature to replace the rule in extension.
   
   +1


-- 
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] ulysses-you commented on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079895064


   yea, if so I agree we can use this feature to replace the rule in extension.


-- 
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 #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079838569


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?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 [#2225](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2a8aa6) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/86cd685de784f6ade61ea78502aae561cdb48a7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86cd685) will **decrease** coverage by `0.00%`.
   > The diff coverage is `86.36%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2225      +/-   ##
   ============================================
   - Coverage     61.80%   61.79%   -0.01%     
     Complexity       69       69              
   ============================================
     Files           331      331              
     Lines         16016    16034      +18     
     Branches       2031     2032       +1     
   ============================================
   + Hits           9899     9909      +10     
   - Misses         5295     5300       +5     
   - Partials        822      825       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.08% <71.42%> (-2.04%)` | :arrow_down: |
   | [...ine/spark/operation/SparkSQLOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vU3BhcmtTUUxPcGVyYXRpb25NYW5hZ2VyLnNjYWxh) | `84.90% <90.00%> (+0.81%)` | :arrow_up: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.16% <100.00%> (+0.02%)` | :arrow_up: |
   | [...org/apache/kyuubi/ha/client/ServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9TZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `89.28% <0.00%> (-3.58%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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) | `39.72% <0.00%> (-1.37%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [86cd685...f2a8aa6](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] turboFei commented on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079912416


   > Does the user know that we truncate the data?
   
   By default, it is not truncated.
   It is determined by the service provider.


-- 
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 change in pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
pan3793 commented on a change in pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#discussion_r835912909



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -626,6 +626,14 @@ object KyuubiConf {
       .intConf
       .createWithDefault(1000000)
 
+  val ENGINE_SPARK_MAX_ROWS: ConfigEntry[Int] =
+    buildConf("session.engine.spark.max.rows")

Review comment:
       `kyuubi.operation.max.result.rows` makes sense to me, and we can deprecate `kyuubi.session.engine.flink.max.rows`




-- 
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] ulysses-you commented on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1080103953


   thanks, 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] ulysses-you closed pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225


   


-- 
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 edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079838569


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?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 [#2225](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2a8aa6) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/86cd685de784f6ade61ea78502aae561cdb48a7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86cd685) will **decrease** coverage by `0.00%`.
   > The diff coverage is `86.36%`.
   
   > :exclamation: Current head f2a8aa6 differs from pull request most recent head 51983c3. Consider uploading reports for the commit 51983c3 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2225      +/-   ##
   ============================================
   - Coverage     61.80%   61.79%   -0.01%     
     Complexity       69       69              
   ============================================
     Files           331      331              
     Lines         16016    16034      +18     
     Branches       2031     2032       +1     
   ============================================
   + Hits           9899     9909      +10     
   - Misses         5295     5300       +5     
   - Partials        822      825       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `82.08% <71.42%> (-2.04%)` | :arrow_down: |
   | [...ine/spark/operation/SparkSQLOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vU3BhcmtTUUxPcGVyYXRpb25NYW5hZ2VyLnNjYWxh) | `84.90% <90.00%> (+0.81%)` | :arrow_up: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.16% <100.00%> (+0.02%)` | :arrow_up: |
   | [...org/apache/kyuubi/ha/client/ServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9TZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `89.28% <0.00%> (-3.58%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225/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) | `39.72% <0.00%> (-1.37%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [86cd685...51983c3](https://codecov.io/gh/apache/incubator-kyuubi/pull/2225?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] turboFei edited a comment on pull request #2225: Support to set result max rows for spark engine

Posted by GitBox <gi...@apache.org>.
turboFei edited a comment on pull request #2225:
URL: https://github.com/apache/incubator-kyuubi/pull/2225#issuecomment-1079912416


   > Does the user know that we truncate the data?
   
   By default, it is not truncated.
   It is determined by the service provider.
   
   For example, for BI tool, it might limit the max result rows.


-- 
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