Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

TAJO-1400: Add TajoStatement::setMaxRows method support#421

Closed
hys9958 wants to merge 4 commits intoapache:masterfrom
hys9958:TAJO-1400
Closed

TAJO-1400: Add TajoStatement::setMaxRows method support#421
hys9958 wants to merge 4 commits intoapache:masterfrom
hys9958:TAJO-1400

Conversation

@hys9958
Copy link
Contributor

@hys9958 hys9958 commented Mar 13, 2015

I implemented the setMaxRows and getMaxRows method support.
Please review my patch.

thanks.

@blrunner
Copy link
Contributor

Thank you for your contribution.
I'll review the patch today. :)

@jihoonson
Copy link
Contributor

Would you add some unit tests?
TestTajoJDBC will be good to add tests.

@hys9958
Copy link
Contributor Author

hys9958 commented Mar 21, 2015

Ok, jihoonson,
I'll update the TestTajoJDBC for unit tests of this issue soon.

@jihoonson
Copy link
Contributor

+1
Thanks for your contribution!

@jinossy
Copy link
Member

jinossy commented Mar 30, 2015

@hys9958
How do you think that move the setMaxRows() to TajoStatement ?

@jihoonson
Copy link
Contributor

@jinossy thanks for the good suggestion.
It would be a better implementation.

@hys9958
Copy link
Contributor Author

hys9958 commented Mar 30, 2015

@jinossy Ok, It's better idea.
I'll re-patch the implementation of maxRows(). :)

@jihoonson
Copy link
Contributor

@hys9958 thanks. I'll wait for the next patch.

@hys9958 hys9958 closed this Mar 30, 2015
@hys9958
Copy link
Contributor Author

hys9958 commented Mar 31, 2015

Hi @jihoonson ,
Query return value is 'java.sql.ResultSet' interfase in TajoStatement. It need to many refactoring.
And i'll also another patch about getMoreResults. When the getMoreResults patch time, i update maxRows.

@hys9958 hys9958 reopened this Mar 31, 2015
@jihoonson
Copy link
Contributor

Ok. Here is my +1.
If there are no objections, I'll commit.

@asfgit asfgit closed this in 696d2aa Apr 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants