elastic / elasticsearch-net

This strongly-typed, client library enables working with Elasticsearch. It is the official client maintained and supported by Elastic.
https://www.elastic.co/guide/en/elasticsearch/client/net-api/current/index.html
Apache License 2.0
3.56k stars 1.15k forks source link

Cannot use RawJsonQuery #8195

Open gpetrou opened 3 months ago

gpetrou commented 3 months ago

Elastic.Clients.Elasticsearch version: 8.13.10

Elasticsearch version: 8.13.2

.NET runtime version: 8.0

Operating system version:

Description of the problem including expected versus actual behavior: Try to execute the following code:

Elastic.Clients.Elasticsearch.QueryDsl.RawJsonQuery rawQuery = new("{\"term\": { \"fieldname\": \"value\"  }}");

SearchRequest<JsonObject> elasticSearchRequest = new("IndexName")
{
    Query = rawQuery
};

Elastic.Clients.Elasticsearch.SearchResponse<JsonObject> expected = await _elasticsearchClient.SearchAsync<JsonObject>(elasticSearchRequest);

It results in:

{
  "query": {
    "raw_json": {"term": { "fieldname": "value"  }}
  }
}

Therefore there is a parsing_exception.

Steps to reproduce: 1. 2. 3.

Expected behavior A clear and concise description of what you expected to happen.

Provide ConnectionSettings (if relevant):

Provide DebugInformation (if relevant):

gpetrou commented 3 months ago

@flobernd looks like the issue is in one of your generated files, so I cannot just change that. Can you please fix it? I assume that for raw json, we shouldn't use writer.WriteStartObject();, writer.WritePropertyName(value.VariantName);, etc...

flobernd commented 3 months ago

Hi @gpetrou,

the RawJsonQuery is a bit of a hack. The most easiest fix for me would be to separate the variant-name (e.g. "term") from the rest of the query payload:

var q = new RawJsonQuery("term", "{ \"fieldname\": \"value\"  }");

Would that work for you?


Special handling in the generator is possible, but I'd like to avoid that.

Btw.: In general you should prefer not using the RawJsonQuery, if the functionality is otherwise supported in the client. This construct was only added as a last-resort fallback when the client was still lacking a lot of features. Is there a specific reason for using RawJsonQuery in your case? I'm curious if I can "properly" improve things to simplify your use-case.

gpetrou commented 3 months ago

There are still cases such as https://github.com/elastic/elasticsearch-net/issues/8169#issuecomment-2100975754 where this seems to be needed. I would expect this to work similarly to Nest.RawQuery.

flobernd commented 3 months ago

Yes, this case would work.

tabareh commented 3 months ago

Hi @gpetrou,

the RawJsonQuery is a bit of a hack. The most easiest fix for me would be to separate the variant-name (e.g. "term") from the rest of the query payload:

var q = new RawJsonQuery("term", "{ \"fieldname\": \"value\"  }");

Would that work for you?

Special handling in the generator is possible, but I'd like to avoid that.

Btw.: In general you should prefer not using the RawJsonQuery, if the functionality is otherwise supported in the client. This construct was only added as a last-resort fallback when the client was still lacking a lot of features. Is there a specific reason for using RawJsonQuery in your case? I'm curious if I can "properly" improve things to simplify your use-case.

There is well no RawJsonQuery constructor that accepts two params at least for version 8.13.12 that I use

flobernd commented 3 months ago

@tabareh Yes, this will be available after I released a fix.

andrewrauber commented 1 month ago

@flobernd do we have an update on a fix for this? Using RawJsonQuery seems to be our only option given that GeoShapeQuery doesn't exist yet. Our last resort would be to convert everything to Nest, which is not ideal.

flobernd commented 1 month ago

@andrewrauber If it's only about the GeoShapeQuery, I can offer to include it in the next release. The Shape property would be of type object which means you have to create your own shape classes, but I guess that would help?

andrewrauber commented 1 month ago

@flobernd Yes, that works for me, assuming the next release is in the coming week or so?

flobernd commented 1 month ago

@andrewrauber GeoShapeQuery and ShapeQuery is now available with the release of 8.14.7.

andrewrauber commented 1 month ago

Amazing! Thank you so much.

marcosrobles-qo commented 1 month ago

I also get an error trying to use RawJsonQuery

# Response: {"error":{"root_cause":[{"type":"parsing_exception","reason":"unknown query [raw_json]","line":1,"col":22}],"type":"parsing_exception","reason":"unknown query [raw_json]","line":1,"col":22,"caused_by":{"type":"named_object_not_found_exception","reason":"[1:22] unknown field [raw_json]"}},"status":400}

raw_json is being generated automatically :

image

flobernd commented 1 month ago

Yes @marcosrobles-qo, the RawJsonQuery is currently not working as intended and will most likely get removed completely. Is there a specific reason for you to use the RawJsonQuery over the concrete query variant?

marcosrobles-qo commented 1 month ago

I was trying to create an endpoint to make queries with any query dsl json. Is there any alternative to perform that kind of requirement ? or I should limit the scope and create endpoints for each specific kind of query dsl I use?

flobernd commented 1 month ago

I should limit the scope and create endpoints for each specific kind of query dsl I use?

This would be the preferred way. The RawJsonQuery was never meant to be an actual feature, but a last resort fallback for missing functionality. Most functionality is now available in the regular API which means that the main driver of supporting RawJsonQuery is gone.

tabareh commented 4 weeks ago

The raw jsson is much simpler and therefore much more maintainable than DSL in nest. There had been no one that I met who was happy with nest syntax. Therefore we, to get a maintainable code, write our queries in the syntax of payload. As this feature is missing, we need to migrate all our code to painless which is most maintainable syntax after raw json