(dsl): Support `multiMatch` query #305

milicns · 2023-08-17T09:38:39Z

Part of #91

Fields parameter is made optional so implementation of this query is a bit different than others.

And should type parameter be included and made optional? https://www.elastic.co/guide/en/elasticsearch/reference/7.17/query-dsl-multi-match-query.html#multi-match-types

milicns · 2023-08-17T09:41:56Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   */
+  final def multiMatch(value: String): MultiMatchQuery[Any] =
+    MultiMatch(fields = None, value = value)
+


Because fields are optional and type-safe method is now in HasFields trait, i suppose we don't need multiMatch type-safe method?

If you change fields inside of query, this will then be: fields=Chunk.empty

dbulaja98 · 2023-08-17T13:17:36Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

@@ -584,6 +585,25 @@ private[elasticsearch] final case class MatchPhrase[S](field: String, value: Str
    )
 }

+sealed trait MultiMatchQuery[S] extends ElasticQuery[S] with HasFields[MultiMatchQuery[S]]
+
+private[elasticsearch] final case class MultiMatch[S](fields: Option[Chunk[String]], value: String)


I would also add minimum_should_match and boost parameters, as they are already implemented, and can be applied to this query.

Also, @arnoldlacko @drmarjanovic, what do you think, do we need Types of multi_match query as additional parameter?

I think it could make sense to allow something like .matchingType(MultiMatchType.Phrase).
If it's more complex, it could also be a separate PR.

dbulaja98 · 2023-08-17T13:19:44Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+  def fields[S1: Schema](fields: Field[S1, String]*): MultiMatchQuery[S] =
+    self.copy(fields = Some(Chunk.fromIterable(fields.map(_.toString))))


I'm thinking maybe we should just have fields without option, as it is list, it can always be empty, and that way we will cover need for option.

So here, we can have method which only adds new field to the list of fields.

Also, some sort of restrictions should be discussed, as (I think) we can not have both type-safe and raw fields. @arnoldlacko what do you think?

I think given that we use _.toString here, the resulting query is the same regardless of being constructed using Field or raw strings, so it does not need to be an issue if they are mixed.
What I wonder is why we have a S1 type here with no defined relation to S, and why it does not affect the type of the resulting query. We might want to aim for something like:

fields[S1 <: S: Schema](fields: Field[S1, String]*): MultiMatchQuery[S1]

@mvelimir any thoughts?

We'll definitely need S1 to be a subtype of S, to narrow the type which will always start from Any based on the constructor method. Also, I think mixing both type-safe fields and plain strings should be allowed, since strings in this case can be used as wildcards, and having type-safe fields will still be useful in inferring the type of the document. Also, the type-safe method should definitely contain at least one field as to not clash with the strings method when the argument list is empty. With all this in mind, I think the method signature should be:

def fields[S1 <: S: Schema](field: Field[S1, String], fields: Field[S1, String]*): MultiMatchQuery[S1]

dbulaja98 · 2023-08-17T13:21:31Z

docs/overview/queries/elastic_query_multi_match.md

+title: "Multi Match Query"
+---
+
+The `MultiMatch` query builds on the `match` query to allow multi-field queries:


This seems to be unfinished?

dbulaja98 · 2023-08-17T13:23:09Z

docs/overview/queries/elastic_query_multi_match.md

+val query: MultiMatchQuery = multiMatch(value = "test")
+```
+
+You can create a `MultiMatch` query with fields that will be searched using the `multiMatch` method this way:


Let's use existing syntax:
If you want to change fields, you can use the fields method:

dbulaja98 · 2023-08-17T13:26:12Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   */
+  final def multiMatch(value: String): MultiMatchQuery[Any] =
+    MultiMatch(fields = None, value = value)
+


If you change fields inside of query, this will then be: fields=Chunk.empty

dbulaja98 · 2023-08-17T13:30:15Z

modules/library/src/main/scala/zio/elasticsearch/query/options/HasFields.scala

+  def fields(fields: String*): Q
+
+  /**
+   * Sets the `fields` parameter for this [[zio.elasticsearch.query.ElasticQuery]]. The `fields` parameter is the


Suggested change

* Sets the `fields` parameter for this [[zio.elasticsearch.query.ElasticQuery]]. The `fields` parameter is the

* Sets the type-safe `fields` parameter for this [[zio.elasticsearch.query.ElasticQuery]]. The `fields` parameter is the

drmarjanovic · 2023-08-22T19:09:09Z

modules/library/src/main/scala/zio/elasticsearch/query/MultiMatchType.scala

+
+private[elasticsearch] object MultiMatchType extends Enumeration {
+  type MultiMatchType = String
+  val BestFields   = "best_fields"


Please sort this:

BestFields
BoolPrefix
CrossFields
MostFields
Phrase
PhrasePrefix

drmarjanovic · 2023-08-22T19:10:26Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+private[elasticsearch] final case class MultiMatch[S](
+  fields: Chunk[String],
+  value: String,
+  matchingType: Option[MultiMatchType],


Let's put boost before matchingType.

dbulaja98 · 2023-08-23T20:24:44Z

docs/overview/queries/elastic_query_multi_match.md

+title: "Multi Match Query"
+---
+
+The `MultiMatch` query builds on the `match` query to allow multi-field queries:


dbulaja98 · 2023-08-23T20:41:45Z

docs/overview/queries/elastic_query_multi_match.md

+val query: MultiMatchQuery = multiMatch(value = "test").fields("stringField1", "stringField2")
+```
+
+If you want to change the [type-safe](https://lambdaworks.github.io/zio-elasticsearch/overview/overview_zio_prelude_schema) `fields` that will be searched, you can use the `fields` method this way:


Suggested change

If you want to change the [type-safe](https://lambdaworks.github.io/zio-elasticsearch/overview/overview_zio_prelude_schema) `fields` that will be searched, you can use the `fields` method this way:

If you want to change the `fields` that will be searched, you can use the `fields` method with the [type-safe](https://lambdaworks.github.io/zio-elasticsearch/overview/overview_zio_prelude_schema) parameters this way:

dbulaja98 · 2023-08-23T20:43:29Z

docs/overview/queries/elastic_query_multi_match.md

+val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2)
+```
+
+If you want to change the `type` of `MultiMatch` query, you can use the `matchingType` method:


Suggested change

If you want to change the `type` of `MultiMatch` query, you can use the `matchingType` method:

If you want to change the `type`, you can use the `matchingType` method:

dbulaja98 · 2023-08-23T20:46:02Z

docs/overview/queries/elastic_query_multi_match.md

+```scala
+import zio.elasticsearch.query.MultiMatchType
+
+val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)


Suggested change

val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)

val queryWithType: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)

dbulaja98 · 2023-08-23T20:46:38Z

docs/overview/queries/elastic_query_multi_match.md

+
+If you want to change the `boost`, you can use the `boost` method:
+```scala
+val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)


Suggested change

val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)

val queryWithBoost: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)

dbulaja98 · 2023-08-23T21:11:54Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   * @param matchingType
+   *   the [[zio.elasticsearch.query.MultiMatchType]] value of 'type' parameter


Can you explain possible values for matchingType.
You can lookup for other places with similar @param.

dbulaja98 · 2023-08-23T21:12:11Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   * @param matchingType
+   *   the [[zio.elasticsearch.query.MultiMatchType]] value of 'type' parameter
+   * @return
+   *   a new instance of the [[zio.elasticsearch.query.ElasticQuery]] with the `type` parameter set.


Same as above (an instance..).

dbulaja98 · 2023-08-23T21:13:30Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+  def fields(field: String, fields: String*): MultiMatchQuery[S] =
+    self.copy(fields = Chunk.fromIterable(field +: fields))


Does it make sense to add fields to chunk instead of setting it? @drmarjanovic

dbulaja98 · 2023-08-23T21:17:21Z

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

+          val queryWithMinimumShouldMatch = multiMatch("this is a test").minimumShouldMatch(2)
+          val queryWithAllParams = multiMatch("this is a test")
+            .fields(TestDocument.stringField)
+            .matchingType(MultiMatchType.BestFields)


Can you omit MultiMatchType? (everywhere in file)

dbulaja98 · 2023-08-23T21:17:50Z

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

@@ -2505,13 +2608,120 @@ object ElasticQuerySpec extends ZIOSpecDefault {
              |    "stringField": {
              |       "query" : "test"
              |    }
-              |  }
+              |   }


Was this on purpose?

dbulaja98 · 2023-08-24T07:40:51Z

modules/library/src/main/scala/zio/elasticsearch/query/MultiMatchType.scala

+
+package zio.elasticsearch.query
+
+private[elasticsearch] object MultiMatchType extends Enumeration {


Please refer to the existing classes that represent enumerations.

dbulaja98 · 2023-08-28T09:49:49Z

docs/overview/queries/elastic_query_multi_match.md

+If you want to change the `type`, you can use the `matchingType` method:
 ```scala
-import zio.elasticsearch.query.MultiMatchType
+import zio.elasticsearch.query.MultiMatchType._

-val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)
+val queryWithType: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)
 ```

 If you want to change the `boost`, you can use the `boost` method:
 ```scala
-val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)
+val queryWithBoost: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)
 ```


Put boost first, then minimum_should_match, then type.

dbulaja98 · 2023-08-28T09:52:29Z

modules/library/src/main/scala/zio/elasticsearch/query/MultiMatchType.scala

+  case object CrossFields  extends MultiMatchType { def value: String = "cross_fields"  }
+  case object MostFields   extends MultiMatchType { def value: String = "most_fields"   }
+  case object Phrase       extends MultiMatchType { def value: String = "phrase_fields" }
+  case object PhrasePrefix extends MultiMatchType { def value: String = "phrase_prefix" }
 }


Put empty line at the end.

If you mean at the end of file, sbt prepare removes it

dbulaja98 · 2023-08-28T09:55:20Z

modules/library/src/main/scala/zio/elasticsearch/query/MultiMatchType.scala

+sealed trait MultiMatchType {
+  def value: String
+  override def toString: String = value
+}


Maybe this is overkill, also, I think value should not be visible to users.
My advice is this: put only override def toString: String = value to each case object.

dbulaja98 · 2023-08-28T11:23:00Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

   */
  def fields(field: String, fields: String*): MultiMatchQuery[S]

  /**
   * Sets the type-safe `fields` parameter for this [[zio.elasticsearch.query.ElasticQuery]]. The `fields` parameter is
-   * the type-safe array of fields that will be searched.
+   * the array of type-safe fields that will be searched.


Omit the.

dbulaja98 · 2023-08-28T11:34:50Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   *   the [[zio.elasticsearch.query.MultiMatchType]] value of 'type' parameter, possible values are:
+   *   - [[zio.elasticsearch.query.MultiMatchType.BestFields]]
+   *   - [[zio.elasticsearch.query.MultiMatchType.BoolPrefix]]
+   *   - [[zio.elasticsearch.query.MultiMatchType.CrossFields]]
+   *   - [[zio.elasticsearch.query.MultiMatchType.MostFields]]
+   *   - [[zio.elasticsearch.query.MultiMatchType.Phrase]]
+   *   - [[zio.elasticsearch.query.MultiMatchType.PhrasePrefix]]


Please, provide short explanation for each value. You can see scaladoc for scoreMode def.

dbulaja98 · 2023-08-28T11:36:03Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+        .map(
+          "minimum_should_match" -> _.toJson
+        )


Can this be in one line? Or better formatted?

dbulaja98 · 2023-08-29T08:32:55Z

modules/library/src/main/scala/zio/elasticsearch/query/MultiMatchType.scala

-  case object MostFields   extends MultiMatchType { def value: String = "most_fields"   }
-  case object Phrase       extends MultiMatchType { def value: String = "phrase_fields" }
-  case object PhrasePrefix extends MultiMatchType { def value: String = "phrase_prefix" }
+  case object BestFields   extends MultiMatchType { override def toString: String = "best_fields"   }


Can you please put defs to new line inside of objects?

milicns commented Aug 17, 2023

View reviewed changes

dbulaja98 requested changes Aug 17, 2023

View reviewed changes

Vasilije Milic added 2 commits August 22, 2023 09:47

Make fields parameter optional

e402f7e

Fix code remarks

ecfe1ce

milicns force-pushed the task-multi-match-query-api branch from 56b9e26 to ecfe1ce Compare August 22, 2023 12:19

milicns requested a review from dbulaja98 August 22, 2023 12:22

drmarjanovic requested changes Aug 22, 2023

View reviewed changes

Fix order

2753621

milicns requested a review from drmarjanovic August 23, 2023 08:33

dbulaja98 requested changes Aug 23, 2023

View reviewed changes

dbulaja98 requested changes Aug 24, 2023

View reviewed changes

Fix code remarks

ece7054

milicns requested a review from dbulaja98 August 24, 2023 08:40

dbulaja98 requested changes Aug 28, 2023

View reviewed changes

Fix code remarks

263e9d9

milicns requested a review from dbulaja98 August 28, 2023 15:18

Fix doc

b8f7ded

dbulaja98 requested changes Aug 29, 2023

View reviewed changes

Space defs to new lines

f78ea5d

milicns requested a review from dbulaja98 August 29, 2023 08:53

dbulaja98 approved these changes Aug 29, 2023

View reviewed changes

dbulaja98 merged commit 512740f into lambdaworks:main Aug 29, 2023
13 checks passed

		def fields[S1: Schema](fields: Field[S1, String]*): MultiMatchQuery[S] =
		self.copy(fields = Some(Chunk.fromIterable(fields.map(_.toString))))

	* Sets the `fields` parameter for this [[zio.elasticsearch.query.ElasticQuery]]. The `fields` parameter is the
	* Sets the type-safe `fields` parameter for this [[zio.elasticsearch.query.ElasticQuery]]. The `fields` parameter is the

	If you want to change the [type-safe](https://lambdaworks.github.io/zio-elasticsearch/overview/overview_zio_prelude_schema) `fields` that will be searched, you can use the `fields` method this way:
	If you want to change the `fields` that will be searched, you can use the `fields` method with the [type-safe](https://lambdaworks.github.io/zio-elasticsearch/overview/overview_zio_prelude_schema) parameters this way:

	If you want to change the `type` of `MultiMatch` query, you can use the `matchingType` method:
	If you want to change the `type`, you can use the `matchingType` method:

	val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)
	val queryWithType: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).matchingType(MultiMatchType.MostFields)

	val query: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)
	val queryWithBoost: MultiMatchQuery = multiMatch(value = "test").fields(Document.stringField1, Document.stringField2).boost(2.2)

		* @param matchingType
		* the [[zio.elasticsearch.query.MultiMatchType]] value of 'type' parameter

		def fields(field: String, fields: String*): MultiMatchQuery[S] =
		self.copy(fields = Chunk.fromIterable(field +: fields))


		package zio.elasticsearch.query

		private[elasticsearch] object MultiMatchType extends Enumeration {

(dsl): Support multiMatch query #305

(dsl): Support multiMatch query #305

Conversation

milicns commented Aug 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnoldlacko Aug 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbulaja98 Aug 24, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

(dsl): Support `multiMatch` query #305

(dsl): Support `multiMatch` query #305

arnoldlacko Aug 21, 2023 •

edited

dbulaja98 Aug 24, 2023 •

edited