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

DATACOUCH-28 - Remove explicit setting of setIncludeDocs on View queries. #8

Closed
wants to merge 1 commit into from

Conversation

dharrigan
Copy link
Contributor

Instead of always setting includeDocs to true for all View queries, it is now
the assumption that the programmer will enable this if they desire documents
to be returned on their query. The reasoning is to allow for an optimisation
when performing queries so that documents are not returned unless asked for.
This increases performance and reduces payload size in the communication from
Couchbase to the client.

-=david=-

Instead of always setting includeDocs to true for all View queries, it is now
the assumption that the programmer will enable this if they desire documents
to be returned on their query. The reasoning is to allow for an optimisation
when performing queries so that documents are not returned unless asked for.
This increases performance and reduces payload size in the communication from
Couchbase to the client.

-=david=-
@daschl
Copy link
Contributor

daschl commented Oct 4, 2013

@dharrigan I looked at this once more in the codebase and I know remember why I did it originally. Let's talk it through so we got an the same line..

in the findByView, I get a list of marshalled objects back, but when setIncludeDocs is not set to true always the method won't return useful data (because I have nothing to marshal from).

If you look down further, there is the queryView method which lets you access the query result directly (while still getting the exception mapping).

That's also why I force reduce to false because if you have a reduced view I can't get the object ids from the result.

@dharrigan
Copy link
Contributor Author

Hi Michael,

Yes, I follow. It makes sense. As you say, unless includeDocs is set to true (always) in findByView, then come unmarshalling, nothing useful is produced.

I assume I should close my pull request?

-=david=-

@daschl
Copy link
Contributor

daschl commented Oct 4, 2013

If it makes sense for you now yes. Sorry I haven't realized this earlier but I couldn't identify it clearly from the last one :) .. but at least you now know how it works internally 👍

For your use case, can you work with the provided methods or do we need to implement something else to make you more productive?

@dharrigan
Copy link
Contributor Author

Hi,

Totally fine. I was only doing this out of a wish to contribute to the project - not for myself personally :-) I saw the JIRA and thought okay, seems reasonable, I'll do a patch and see what comes from that :-)

Just an interested party :-)

-=david=-

@dharrigan dharrigan closed this Oct 4, 2013
@daschl
Copy link
Contributor

daschl commented Oct 4, 2013

cool, keep em' coming 👍

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