Skip to content

[KYUUBI #360] Correct handle getNextRowSet with FETCH_PRIOR FETCH_FIRST #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
pan3793 wants to merge 8 commits into from
Closed

[KYUUBI #360] Correct handle getNextRowSet with FETCH_PRIOR FETCH_FIRST #370

pan3793 wants to merge 8 commits into from

Conversation

@pan3793
Copy link
Member

@pan3793 pan3793 commented Feb 25, 2021 •
edited by pull-request-badge bot
Loading

pan3793 Closes #370 332 24 8 Feature Bug ❨?❩

Why are the changes needed?

close #360

Ref: apache/spark#30600

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

@pan3793 pan3793 changed the title [KYUUBI #360] [KYUUBI #360] Correct handle getNextRowSet with FETCH_PRIOR FETCH_FIRST Feb 25, 2021
@codecov
Copy link

codecov bot commented Feb 25, 2021 •
edited
Loading

Codecov Report

Merging #370 (ef364fe) into master (d94b1c4) will increase coverage by 0.09%.
The diff coverage is 92.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
+ Coverage   80.74%   80.84%   +0.09%     
==========================================
  Files          98       99       +1     
  Lines        3574     3612      +38     
  Branches      416      423       +7     
==========================================
+ Hits         2886     2920      +34     
- Misses        474      475       +1     
- Partials      214      217       +3
Impacted Files Coverage Δ
.../org/apache/kyuubi/operation/KyuubiOperation.scala 46.66% <ø> (ø)
...ache/kyuubi/operation/KyuubiOperationManager.scala 58.46% <ø> (ø)
.../org/apache/kyuubi/session/KyuubiSessionImpl.scala 72.22% <ø> (ø)
...kyuubi/engine/spark/operation/SparkOperation.scala 70.68% <62.50%> (-2.39%) ⬇️
...org/apache/kyuubi/engine/spark/FetchIterator.scala 96.77% <96.77%> (ø)
...uubi/engine/spark/operation/ExecuteStatement.scala 84.44% <100.00%> (ø)
...he/kyuubi/engine/spark/operation/GetCatalogs.scala 83.33% <100.00%> (ø)
...che/kyuubi/engine/spark/operation/GetColumns.scala 97.95% <100.00%> (ø)
...e/kyuubi/engine/spark/operation/GetFunctions.scala 93.75% <100.00%> (ø)
...che/kyuubi/engine/spark/operation/GetSchemas.scala 90.00% <100.00%> (ø)
... and 7 more

yaooqinn
yaooqinn reviewed Feb 25, 2021
View reviewed changes
...ls/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/FetchIterator.scala

package org.apache.kyuubi.engine.spark

private[engine] sealed trait FetchIterator[A] extends Iterator[A] {
Copy link
Member

@yaooqinn yaooqinn Feb 25, 2021

Choose a reason for hiding this comment

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

Let's honor Spark libraries and add some comment about it

Copy link
Member Author

@pan3793 pan3793 Feb 25, 2021

Choose a reason for hiding this comment

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

Updated

@yaooqinn yaooqinn added this to the v1.1.0 milestone Feb 25, 2021
@yaooqinn yaooqinn added Bug kind:feature Feature request labels Feb 25, 2021
@yaooqinn yaooqinn closed this in c659089 Feb 25, 2021
@yaooqinn
Copy link
Member

yaooqinn commented Feb 25, 2021

thanks, merged to master for v1.1.0

@pan3793 pan3793 deleted the KYUUBI-360 branch February 25, 2021 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaooqinn yaooqinn yaooqinn approved these changes

Assignees

@pan3793 pan3793

Labels

kind:feature Feature request

Projects

None yet

Milestone

v1.1.0

Development

Successfully merging this pull request may close these issues.

FetchOrientation.FETCH_FIRST is not handled properly

2 participants

@pan3793 @yaooqinn