Skip to content

Conversation

@syrm
Copy link

@syrm syrm commented Jan 12, 2016

Hello,

This feature change the behavior of query to return an Iterator, that a more nice way to return the result and allow code like that :

foreach ($db->query(CouchbaseN1qlQuery::fromString('SELECT * FROM `beer-sample`')) as $document) {
        var_dump($document);
}

@JordyMoos
Copy link
Contributor

This would change backward compatibility.
You can make your own service wrapper around this library to have the result as an iterator right

@syrm
Copy link
Author

syrm commented Jan 12, 2016

Yes but that should be the default behavior.

@syrm
Copy link
Author

syrm commented Jan 12, 2016

I can change to this and add Countable interface yes

@syrm
Copy link
Author

syrm commented Jan 12, 2016

I added Countable interface

@SimonSimCity
Copy link

@OXman, I guess it's confusing if you add a countable interface and let that one return the full amount of items in that list. I do neither know if the full amount of this query or the amount of items returned is expected by the user. What do you guess?

I expect a collection to return the amount of items it has if I count it. Would yours? I guess when adding a limit, yours wouldn't return the amount of items returned.

@syrm
Copy link
Author

syrm commented Jan 13, 2016

Even if i wouldn't return the full amount of data, the C driver does it.
When the C driver would make iteration on the fly like a Mysql driver, CouchbaseResult will use it and no change will be needed.

And when you add a limit, you want the total number of items for your query (so with the limit).

@SimonSimCity
Copy link

@OXman yea, but that's not the case if we would take that code you wrote.

Please count the rows for the implementation of the Countable interface and return the full amount of rows in an extra parameter or method call.

You could decide to internally use the limit and query the next set if the user didn't add a limit - then your implementation would be correct, but that needs a lot more work.

@syrm
Copy link
Author

syrm commented Jan 13, 2016

Is it ok for you with this sample ?
https://gist.github.com/oxman/c0023e0bda77b812bdec

@SimonSimCity
Copy link

@OXman Looks good - but I have to admit, that I never tried out the n1ql feature. I was thinking about usual view-queries. Maybe the client returns other additional parameters, that are now inaccessible, when using n1ql ...

@syrm
Copy link
Author

syrm commented Jan 13, 2016

Well in view queries you can limit the result set with many options, like startkey, endkey, then the behavior is like N1QL with LIMIT.

@syrm
Copy link
Author

syrm commented Jan 13, 2016

@JordyMoos about backward compatibility, semver is designed for that.

@brett19 brett19 force-pushed the master branch 3 times, most recently from 696ef14 to 93d687a Compare April 4, 2016 18:58
@brett19 brett19 closed this Apr 11, 2016
@brett19
Copy link
Owner

brett19 commented Apr 11, 2016

We do not currently have plans in our roadmap to support streaming of results from Couchbase's querying API's. Additionally, because of the nature of streaming, implementing Countable would not be possible if we did.

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.

4 participants