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

docasupsert support #51

Closed
caoilte opened this issue Dec 15, 2013 · 16 comments
Closed

docasupsert support #51

caoilte opened this issue Dec 15, 2013 · 16 comments
Assignees

Comments

@caoilte
Copy link

caoilte commented Dec 15, 2013

It looks like elastic4s doesn't support docasupsert yet.

http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/docs-update.html

or at least the DSL doesn't mention it and there are no tests for it.

Looks like a nice DSL otherwise. Hopefully there'll be some other places where I can use it.

@ghost ghost assigned sksamuel Dec 15, 2013
@sksamuel
Copy link
Collaborator

We can add this fairly quickly.

@sksamuel
Copy link
Collaborator

Actually brain freeze. you can already do this.

client.sync.execute {
  update id 5 in "scifi/trek" script "ctx._source.birthplace = 'iowa'"
}

@caoilte
Copy link
Author

caoilte commented Dec 15, 2013

cool. But, Possibly I didn't phrase the question as exhaustively as I could. From the same page, is there any way to do "partial doc" based updates instead of "script" based ones?

I have large documents that I want to update.

@sksamuel
Copy link
Collaborator

Ah ok.

On 15 December 2013 14:24, caoilte notifications@github.com wrote:

cool. But, Possibly I didn't phrase the question as exhaustively as I
could. From the same page, is there any way to do "partial doc" based
updates instead of "script" based ones?

I have large documents that I want to update.


Reply to this email directly or view it on GitHubhttps://github.com//issues/51#issuecomment-30607019
.

@sksamuel
Copy link
Collaborator

So I'll add in doc, and doc_as_upsert and you should be golden.

On 15 December 2013 14:33, Stephen Samuel (Sam) sam@sksamuel.com wrote:

Ah ok.

On 15 December 2013 14:24, caoilte notifications@github.com wrote:

cool. But, Possibly I didn't phrase the question as exhaustively as I
could. From the same page, is there any way to do "partial doc" based
updates instead of "script" based ones?

I have large documents that I want to update.


Reply to this email directly or view it on GitHubhttps://github.com//issues/51#issuecomment-30607019
.

@sksamuel
Copy link
Collaborator

You can do something like this now (in master):

    client.sync.execute {
      update(14).in("scifi/starwars").doc(
        "character" -> "chewie"
      )
    }

Which will do a partial update based on the doc. You can also replace doc with docAsUpsert as well. The tests give you an idea.

@caoilte
Copy link
Author

caoilte commented Dec 15, 2013

Thanks,
I'm not very familiar with the DSL yet, so forgive me if this is a stupid question, but will I be able to use the "source" semantics as well?

Maybe something like

client.sync.execute {
      update(14).in("scifi/starwars").source(anyOldObject).docAsUpsert
    }

(BTW I find "source" a little confusing as the only time ES docs seem to use it is as "_source" which I think means specific meta-attributes).

@sksamuel
Copy link
Collaborator

No, what it means is that when you create the index you need to enable _source { enabled: true } which you can do easily in the mapping DSL. This is because when you want to do a partial update, it has to get the original document source out, merge it with your new fields, and send it off to be indexed. If it didn't have the original document, it would only have the info that's stored in the indexes, and that might not be enough, if for example some stop words were discarded.

@caoilte
Copy link
Author

caoilte commented Dec 15, 2013

Sorry, I think you're answering a different question. With the plain old java and REST apis I can send JSON documents, much as elastic4s supports using the "source" operator for index commands.

I was wondering if I could use a similar "source" operator for update/docasupsert commands.

Your answer does do a very good job of highlighting the confusion I'm experiencing because "_source" (elasticsearch) is overloaded by "source" (elastic4s) though! ;-)

@sksamuel
Copy link
Collaborator

Ah I see what you mean, yes my choice of source is a bad one. In fact I think that's a good suggestion to change for the final version 1 release. Any suggestions on a better name ?

sksamuel added a commit that referenced this issue Dec 15, 2013
@caoilte
Copy link
Author

caoilte commented Dec 15, 2013

The index API is documented under the "document APIs" section
http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/docs-index_.html

So, although there is no specific definition of the POST data, "document" and/or "doc" feel most natural to me.

BTW, since we're talking about renaming things could you change the extra operations you just added to the update DSL to be "fields" and "fieldsAsUpsert" so that you've consistency with how you express "field" operations on the index DSL. Also, If you did then rename "source" to "doc" in the index DSL that would mean you could make "doc" work the same way on the update DSL.

@sksamuel
Copy link
Collaborator

Good ideas. The only downside is that calling them "fields" and "fieldsAsUpsert" goes against what they're called in the rest and java API. I try to remain true to that as much as I can, only renaming where I think the original is truely confusing. I guess here we have a choice of consistency in elastic4s vs staying true to the elasticsearch api. Hmmm.

@caoilte
Copy link
Author

caoilte commented Dec 15, 2013

"fieldsAsUpsert" does smell. Completely segregating it from the content that you're updating would be more consistent with the Java API (where it is a separate flag on the request object).

eg

client.sync.execute {
      update(14).in("scifi/starwars").fields(
        "character" -> "chewie"
      ).docAsUpsert
    }

or (in my dreams...)

client.sync.execute {
      update(14).in("scifi/starwars").doc(anyOldObject).docAsUpsert
    }

@sksamuel
Copy link
Collaborator

I've added docAsUpsert already as a boolean, eg

client.sync.execute {
      update(14).in("scifi/starwars").doc(anyOldObject).docAsUpsert(true)
    }

which I could easily change to what you just posted (I guess the boolean parameter is redundant anyway).

There's no reason why doc couldn't be overloaded to do what source does now, and renaming source in the index DSL to doc.

@caoilte
Copy link
Author

caoilte commented Dec 15, 2013

What you've posted looks ideal..

I'll try and test it out once I've got our simpler queries ported.

@sksamuel
Copy link
Collaborator

Ok, also would you mind commenting on #44 as I'd like some feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants