Rewrite using functions #38

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
9 participants
Contributor

darobin commented Oct 25, 2012

This pull request implements the ability of using functions for rewriting in addition to the rewriting microlanguage that is currently available. The motivation for doing so comes from a discussion on the users' list in which it turned out that I wasn't the only one who found the microlanguage limited (the latter still works just the same though).

It uses the same "rewrites" ddoc key so that you can't have both. I think that this is a feature: specifying both and having to figure out which takes precedence would be excessive complication IMHO. And it's of course possible to have the function implement the microlanguage if for some reason one wishes to use both simultaneously.

I've provided some docs in the comments — if the patch is accepted and those need to be mirrored (and expanded) somewhere else, simply tell me where is the preferred location. I've also written some tests; I guess that there could be more but it seems clear that it works and doesn't break existing rewrites.

The only change that it makes to the current HTTP interface is that previously if you added a ddoc with a string as the value for "rewrites" you'd get a 400. Now, if that strings is a valid function is just works, and if it's not you get a 500. I doubt that this will break anything.

Some caveats:

• This is essentially my Hello World to Erlang. It's not out of the question that I've written something that looks terrifyingly stupid or woefully more convoluted than it needs to be.

• I've tried to follow the coding conventions that I noticed, apologies if I messed it up here or there.

• The rewrite functions are meant to be cacheable at least for the lifetime of a given ddoc but I haven't added such caching. I don't know how much it would affect performance (though I would expect some). I also don't know whether it should vary on userCtx or not. Either way, I think that it's an optimisation that can be added later.

Contributor

benoitc commented Oct 25, 2012

Initial version of the rewriter was working using functions. It was never accepted for performances reasons. Not sure what to do with this one.

Contributor

dch commented Nov 1, 2012

@davisp anything to add?

Member

davisp commented Nov 1, 2012

Performance would be terrible. Unless that changes I'd consider such an approach to be dead in the water. (Although, I can of course be convinced otherwise with numbers).

Also, major changes like this need to go through JIRA.

@rnewson rnewson pushed a commit to rnewson/couchdb that referenced this pull request Mar 10, 2013

@kocolosk kocolosk Merge pull request #38 from cloudant/13461-inconsistent-reduce-hang
BugzID: 13461
be98bdb

@rnewson rnewson pushed a commit to rnewson/couchdb that referenced this pull request Mar 10, 2013

@kocolosk kocolosk Merge pull request #38 from cloudant/17185-reduce-log-spam
Replace cache miss log with metrics

BugzID: 17185
2e9b66b
Member

nslater commented May 21, 2013

What's the status of this PR?

Contributor

benoitc commented May 21, 2013

like @davisp said the performance qould be terrible until at least we improve all the engine imo.

Contributor

darobin commented May 22, 2013

My understanding was that no one wanted this as done here, though the use case comes up on the mailing list regularly. The primary point is that it's not possible to have nice URLs, sufficiently unique IDs, and REST at the same time without more powerful rewriting than what we currently have.

Member

nslater commented May 22, 2013

@darobin I'd suggest you bring this up in a DISCUSS thread on dev@, and let's see if we can form some consensus around the best way forward. That sort of discussion is not best done in a pull request.

In the meantime, can we close out this PR?

Owner

Humbedooh commented Jan 5, 2014

pingaroo? :)

Owner

Humbedooh commented Jan 24, 2014

Test message for the couchdb dev list, please ignore

Member

garrensmith commented Jan 24, 2014

This is a response test message. Please ignore.

On 24 Jan 2014, at 3:13 PM, Daniel Gruno notifications@github.com wrote:

Test message for the couchdb dev list, please ignore


Reply to this email directly or view it on GitHub.

asfbot commented Jan 24, 2014

test@apache.org replies:

This is a response on response test message. Please ignore.

,,,^..^,,,

Contributor

darobin commented Sep 20, 2016

Maybe this can be closed now that the functionality seems to have been independently added? http://docs.couchdb.org/en/master/api/ddoc/rewrites.html#api-ddoc-rewrite

janl closed this Mar 8, 2017

Contributor

darobin commented Mar 8, 2017

@janl Man, nipped in the bud so close to its fifth year ;-)

@nickva nickva pushed a commit to cloudant/couchdb that referenced this pull request Apr 21, 2017

@mikewallace1979 mikewallace1979 Return forbidden error if encountered on any shard
This commit fixes an issue which caused HTTP 500 errors to be
returned when an authorized user attempted to access a database
that they did not have permission to access, when cassim is
disabled.

Instead of returning an internal server error once all shards
have failed to open we add a receive clause so that we throw
a forbidden error if one is encountered on any shard. This is the
same approach we already take for unauthorized errors.

Closes COUCHDB-2948

This closes #38
31be532
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment