Skip to content
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.

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

Athena can only pull 999 rows at a time #8

Closed
wants to merge 4 commits into from
Closed

Conversation

daroczig
Copy link
Collaborator

@daroczig daroczig commented Nov 6, 2017

No description provided.

@nfultz
Copy link
Owner

nfultz commented Nov 7, 2017

Yeck, RJDBC needs to get updated to use new DBI -

I think it makes more sense to implement dbFetch here than dbGetQuery, otherwise things might work inconsistently across the package.

In the dbFetch implementation, you should be able to calculate block size a la:
with block = if(n < 0) 999 else pmin(n, 999)

and then delegate the request back to RJDBC::fetch with the smaller block size.

See also [1] and [2]

[1] https://github.com/s-u/RJDBC/blob/master/java/JDBCResultPull.java#L69
[2] https://github.com/s-u/RJDBC/blob/master/R/class.R#L328

@nfultz
Copy link
Owner

nfultz commented Nov 7, 2017

Also is this 999 limitation documented anywhere?

@nfultz
Copy link
Owner

nfultz commented Nov 7, 2017

Ah s-u/RJDBC#44

@daroczig
Copy link
Collaborator Author

daroczig commented Nov 7, 2017

I'd like to get dbGetQuery working on Athena, and as fetch has a default 2K set for the blocksize -- that is called by dbGetQuery without passing block, I don't really see any other easy way for this. This PR required more commits than I originally hoped :) But seems to work fine now.

@daroczig
Copy link
Collaborator Author

daroczig commented Nov 7, 2017

@nfultz pls feel free to tweak this further, I don't really want to start hacking dbSendQuery and fetch instead

@daroczig
Copy link
Collaborator Author

daroczig commented Nov 7, 2017

Closing in favour of #9

@daroczig daroczig closed this Nov 7, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants