Skip to content

Conversation

@lsm1
Copy link
Contributor

@lsm1 lsm1 commented Nov 2, 2023

Why are the changes needed?

close #5582

JDBC Engine supports configurable default fetchSize

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 locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

NO

@lsm1 lsm1 changed the title [KYUUBI #5582] JDBC Engine supports configurable fetchSize [KYUUBI #5582] JDBC Engine supports configurable default fetchSize Nov 9, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b1953e8) 61.45% compared to head (dd58a6d) 62.11%.
Report is 7 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5614      +/-   ##
============================================
+ Coverage     61.45%   62.11%   +0.65%     
  Complexity       23       23              
============================================
  Files           603      604       +1     
  Lines         35664    36279     +615     
  Branches       4876     4941      +65     
============================================
+ Hits          21916    22533     +617     
+ Misses        11375    11374       -1     
+ Partials       2373     2372       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


var defaultFetchSize: Int = 0
def get(conf: KyuubiConf): JdbcDialect = {
defaultFetchSize = conf.get(ENGINE_JDBC_FETCH_SIZE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fetch size may be modified by the conf of different sessions. Can the fetch size be obtained in JdbcOperationManager?

val incrementalCollect = normalizedConf.get(OPERATION_INCREMENTAL_COLLECT.key).map(
_.toBoolean).getOrElse(
session.sessionManager.getConf.get(OPERATION_INCREMENTAL_COLLECT))

@lsm1 lsm1 force-pushed the branch-kyuubi-5582 branch from 0d6e858 to 9688064 Compare November 13, 2023 05:08
@lsm1 lsm1 force-pushed the branch-kyuubi-5582 branch from 9688064 to d0eb7c5 Compare November 13, 2023 06:14
Copy link
Contributor

@bowenliang123 bowenliang123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Minor comments for code readability.

@cxzl25 cxzl25 added this to the v1.9.0 milestone Nov 14, 2023
@cxzl25 cxzl25 closed this in e498bdb Nov 15, 2023
@cxzl25
Copy link
Contributor

cxzl25 commented Nov 15, 2023

Thanks, merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TASK][EASY] JDBC Engine supports configurable fetchSize

4 participants