Skip to content

Allow to use the SourceSerializer for InlineScript Parameter(s) #4971

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

Closed
lanwin opened this issue Aug 20, 2020 · 11 comments
Closed

Allow to use the SourceSerializer for InlineScript Parameter(s) #4971

lanwin opened this issue Aug 20, 2020 · 11 comments

Comments

@lanwin
Copy link

lanwin commented Aug 20, 2020

Is your feature request related to a problem? Please describe.
We currently have the case that we want to put a complex object as parameter into an InlineScript. That object uses Json.NET features like JsonConverters and co. Currently it seems that the RequestResponseSerializer is used for that object and so that features did not work.

Describe the solution you'd like
It should be possible to make NEST use the SourceSerializer also for the params of a Script or at least for some params. I could imaging that its possible to assign a Serializer in the settings of the InlineScript object or to wrap the object in something like DocumentSource(T source) which forces NEST to use the SourceSerializer instead.

Describe alternatives you've considered
I see no way of doing it yet.

@lanwin lanwin added the Feature label Aug 20, 2020
@russcam
Copy link
Contributor

russcam commented Aug 24, 2020

This has come up previously in #3652.

I'm not totally convinced in using SourceSerializer for script parameters; Script parameters are tangentially related to source documents, but scripting can also be used in other places too, such as token filters and similarities. There's a bit of performance overhead in handing off the request and response stream to SourceSerializer. What are your thoughts, @Mpdreamz?

@Mpdreamz
Copy link
Member

I tend to agree @russcam , passing complex objects to scripts is not extremely common. At the same time the goal of SourceSerializer is a unified funnel for user defined types.

@lanwin can you expend on your usecase?

Would love to learn more before we potentially commit to a complex new feature.

@lanwin
Copy link
Author

lanwin commented Aug 26, 2020

Yes I can.

We want to do an index request (override the entire object), but want to keep 2 properties , from the indexed document. To do that we make batch request with an bulk update per document. In that update request we use an InlineScript where we read the index document, backup the the 2 properties, override the 2 properties in our new document and replace the source with the new document. The thing is, this new document is passed to the script via parameter. That document is pretty complex and big and it works quiet well for now. But now we felt onto the problem that we need add a feature witch requires JsonNet.

And I currently see no good way to do that on a different way. I could create an update request witch included only the properties we want to update. But that is a pretty complex object with multiple sub objects and arrays in sub objects. In C# that would be very error prone and a nightmare to maintain.

And to say that again. I dont see the requirement to always use the SourceSerializer for Script parameters. But It would help if there is a way to force it on specific parameters. It also would be OK if I could pass pre serialized JSON as parameter. (Witch will be directly written into the json stream instead encoded as string). Something like a Special Object that the RequestResponseSerializer could handle.

@prochnowc
Copy link

prochnowc commented Nov 4, 2020

@russcam @Mpdreamz
We have the same use case than @lanwin, we added custom JsonConverter's to the Newtonsoft.Json based SourceSerializer. The custom converter is used to format Guid's in a shorter format ("N"). When we update documents in Elastic we do this partially using a script. The script receives an anonymous object which contains only the properties which need to be updated. That way the update script is really simple, we can call "putAll" with the whole complex object passed via the script parameter.

Passing each modified property via a script parameter would really increase the size and the complexity of the update script and the C# client code.

Even when we would pass the Guid as separate script param it would not use our custom converter and we didnt find a way to add custom serialization for script params.

So even if you dont want to add SourceSerializer support for script params, there should be a way to customize the script parameter serialization.

@goldenc
Copy link

goldenc commented Feb 17, 2021

We have the same problem, not being able to control how the params are serialized. In this case we pass a Dictionary<string, object> but null values are stripped out which we do not want. It doesn't take into account the NullValueHandling that we pass into the JsonNetSerializer settings

@nathan-c
Copy link

Same problem here. We want to update a document to add to a field which is an array of objects (which contain a NodaTime Instant for which we add a converter to SourceSerializer). Workaround for now is to serialize the object using the SourceSerializer and then deserialize it using the RequestResponseSerializer to a dynamic. This is obviously not a great from a performance perspective but I can't see any other way at the moment.

@mikelneo
Copy link

We have exact same issue. We want to update nested part of object. We have some doc like:

{
 "field1": 123,
 "nested": { ... },
 "field2": "",
 ...
}

and we want to update "nested" part on a group of documents. We use UpdateByQuery and pass an object as a script parameter. We want SourceSerializer to be applied for serialization, because we have some custom settings in it.

@stevejgordon
Copy link
Contributor

Thanks for all of these examples. Given the number of occurrences, this feels like something we should support in some form or another. I've marked some time to investigate this and see how we take this forward.

@Eylidian
Copy link

Today we experienced the same issue expressed above by @goldenc with serializing null values in scripts.
Did you have any workaround in the meanwhile you're trying to address the issue?

@stevejgordon
Copy link
Contributor

@Eylidian For complex objects, the most viable option might be to drop down to the low-level client right now. For small changes, you might get away with a param, per field where the object is null which I believe will serialize though today.

As it happens I was looking at some options for this last week in the v8 client. Right now it's an experimental global configuration that, when enabled uses the source serializer for the params Dictionary. It's a global change though, affecting all script params.

I have also got a potential option based on @ianwin's which allows a "raw" JSON string to be passed that is serialised verbatim. This sadly requires features in the serializer only available on .NET 6.0 targets but I can see being useful.

@stevejgordon
Copy link
Contributor

This has been implemented in v8.0.0-alpha.1 via two mechanisms.

  • On the ElasticsearchClientSettings the Experimental.UseSourceSerializerForScriptParameters property may be enabled which uses the source serializer for params on the script.
  • On .NET 6.0+, it's also possible to create the param object as RawJsonString which uses the string value verbatim when serializing the parameter.

We won't be backporting these to 7.x so I'll close this issue.

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

No branches or pull requests

9 participants