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

Update API: Add support for scripted upserts. #7144

Closed
wants to merge 4 commits into from
Closed

Update API: Add support for scripted upserts. #7144

wants to merge 4 commits into from

Conversation

markharwood
Copy link
Contributor

In the case of inserts the UpdateHelper class will now allow the script used to apply updates to run on the upsert doc provided by clients. This allows the logic for managing the internal state of the data item to be managed by the script and is not reliant on clients performing the initialisation of data structures managed by the script.
Associated issue: #7143

In the case of inserts the UpdateHelper class will now allow the script used to apply updates to run on the upsert doc provided by clients. This allows the logic for managing the internal state of the data item to be managed by the script and is not reliant on clients performing the initialisation of data structures managed by the script.
//Allow the script to abort the create by setting "op" to "none"
String scriptOpChoice = (String) ctx.get("op");
if (scriptOpChoice != null) {
if ("none".equals(scriptOpChoice)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these if statements can be combined? Something like this:

if (!"create".equals(scriptOpChoice)) {
   if ("none".equals(scriptOpChoice)) {
      logger.warn("Used upsert operation [{}] for script [{}], doing nothing...", scriptOpChoice, request.script);
   }
   UpdateResponse update = new UpdateResponse(getResult.getIndex(), getResult.getType(), getResult.getId(),
   getResult.getVersion(), false);
   update.setGetResult(getResult);
   return new Result(update, Operation.NONE, upsertDoc, XContentType.JSON);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Martijn. Looks good but the log msg should only be output if not none i.e.

if (!"none".equals(scriptOpChoice)) {
    logger.warn("Used upsert operation [{}] for script [{}], doing nothing...", scriptOpChoice, request.script);
}

@martijnvg
Copy link
Member

@markharwood LGTM

[source,js]
--------------------------------------------------
curl -XPOST 'localhost:9200/sessions/session/dh3sgudg8gsrgl/_update' -d '{
"script_id" : "myWebSessionSummariser",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use underscore_case for consistency with the rest of the docs?

@markharwood
Copy link
Contributor Author

Will go ahead and push if no objections

@jpountz
Copy link
Contributor

jpountz commented Aug 5, 2014

LGTM

@markharwood
Copy link
Contributor Author

Committed in e6b459c

@markharwood markharwood closed this Aug 5, 2014
@markharwood markharwood removed the review label Aug 5, 2014
@clintongormley clintongormley changed the title Update API enhancement - add support for scripted upserts. Update API: Add support for scripted upserts. Sep 10, 2014
return this.scriptedUpsert;
}

public void scriptedUpsert(boolean scriptedUpsert) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return this;

@clintongormley clintongormley added the :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >feature release highlight v1.4.0.Beta1 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants