iconara / cql-rb

Cassandra CQL 3 binary protocol driver for Ruby
106 stars 31 forks source link

Add support for paging in C* 2.0 #39

Closed iconara closed 10 years ago

codesnik commented 10 years ago

I would like to work on that, if you don't mind.

https://github.com/codesnik/cql-rb/tree/paging

iconara commented 10 years ago

I appreciate the offer, but the protocol_v2 branch is currently in so much flux that I think it will be hard to integrate.

iconara commented 10 years ago

I just finished support for batching (see #74) so paging is next on the list. So even though I appreciate you taking your time to work on this, I think that it will be quicker if I do it.

codesnik commented 10 years ago

Not trying to race you, but I'm on a train and had nothing to do. I rebased my branch on top of latest "v2". You can cherry-pick that commit, or ignore it, I'm fine with it. :)

scottshea commented 10 years ago

I can help test it once you get it done.

iconara commented 10 years ago

Paging just landed in #74. I think a v2.0 prerelease is near. If you can please test it, I expect there will be some issues, the line stats are +5,061 -1,178 :)

If you check out the protocol_v2 branch you can test paging like this:

result_page = client.execute('SELECT * FROM whatever', page_size: 100)
loop do
  result_page.each do |row|
    p row
  end
  break if result_page.last_page?
  result_page = result_page.next_page
end
iconara commented 10 years ago

One design decision that I made was to not have the result set transparently do the paging when you iterate it (in other words you have to load the next page yourself, the result doesn't do it for you).

The reason for this is twofold: first of all it might not be what you want; you might for example want to load pages until you've found something you're looking for and then stop, and this is harder to do if the library is trying to be too helpful. As it is it's simple to implement automatic loading of the next page, and to implement short circuiting.

Secondly, it can't be done in the async API, not without implementing something like streams (which I would like, so maybe in the future). I want to keep the async API (which is still experimental, I'm not sure if I should make it public for 2.0 or not) as close to the synchronous API as possible to minimize the maintenance burden of having both. This feature required quite a lot of juggling with synchronous wrappers to maintain a good synchronous API.

What do you think? Do you think the paging API is useful even if it doesn't load the next page automatically? Should a wrapper that does automatic loading be included?

Here's a wrapper that does the automatic loading, if you need it:

class AutoPagedResult
  include Enumerable

  def initialize(first_page)
    @first_page = first_page
  end

  def each
    return self unless block_given?
    page = @first_page
    loop do
      page.each do |row|
        yield row
      end
      if page.last_page?
        break
      end
      page = page.next_page
    end
  end
end

and use it like this:

result = AutoPagedResult.newclient.execute('SELECT * FROM something', page_size: 100))
result.each do |row|
  p row
end

or, if you're feeling adventurous try this one:

class AutoPagedResult
  include Enumerable

  def initialize(first_page)
    @first_page = first_page
  end

  def each
    return self unless block_given?
    page = @first_page
    loop do
      unless page.last_page?
        next_page = page.async.next_page
      end
      page.each do |row|
        yield row
      end
      if page.last_page?
        break
      end
      page = @first_page.class.new(next_page.value)
    end
  end
end

it loads the next page in the background while the current page is being processed, so it should be much more efficient. It uses a trick that assumes a few things about the current implementation, so it's not something that is guaranteed to work forever (but this whole API might change before 2.0 anyway).

scottshea commented 10 years ago

For my needs having the app not load the next page automatically is great. I will start testing it here in a moment or two

iconara commented 10 years ago

Have any of you guys had a chance to test out v2.0.0.pre0?

scottshea commented 10 years ago

Sorry Theo another project got in the way. I am trying this now.

On Sat, Mar 1, 2014 at 3:52 AM, Theo Hultberg notifications@github.comwrote:

Have any of you guys had a chance to test out v2.0.0.pre0?

Reply to this email directly or view it on GitHubhttps://github.com/iconara/cql-rb/issues/39#issuecomment-36421826 .