-
Notifications
You must be signed in to change notification settings - Fork 18
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
Support boost in Range and Bool query #88
Support boost in Range and Bool query #88
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job overall!
@@ -16,8 +16,17 @@ | |||
|
|||
package zio.elasticsearch | |||
|
|||
import zio.elasticsearch.ElasticQuery.{ElasticPrimitive, MatchAllQuery, TermQuery, WildcardQuery} | |||
import zio.elasticsearch.ElasticQueryType.{MatchAll, Term, Wildcard} | |||
import zio.elasticsearch.ElasticQuery.{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can import zio.elasticsearch.ElasticQuery._
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes done !
@@ -181,11 +182,12 @@ object ElasticQuery { | |||
override def toJson: Option[(String, Json)] = None | |||
} | |||
|
|||
private[elasticsearch] final case class RangeQuery[A, LB <: LowerBound, UB <: UpperBound] private ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we are working on some code improvements - take a look here.
Therefore, after that is applied we should be able to keep private
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes done !
@@ -62,6 +62,11 @@ object QueryDSLSpec extends ZIOSpecDefault { | |||
|
|||
assert(query)(equalTo(MatchQuery(field = "name.keyword", value = "Name"))) | |||
}, | |||
test("successfully create Bool Query with boost") { | |||
val query = boolQuery().boost(1.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After new changes are applied, we are going to use boolQuery.
instead of boolQuery().
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes done !
import zio.elasticsearch.ElasticQuery.{ | ||
BoolQuery, | ||
ElasticPrimitive, | ||
LowerBound, | ||
MatchAllQuery, | ||
RangeQuery, | ||
TermQuery, | ||
UpperBound, | ||
WildcardQuery | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you import zio.elasticsearch.ElasticQuery._
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes done !
@@ -181,11 +182,12 @@ object ElasticQuery { | |||
override def toJson: Option[(String, Json)] = None | |||
} | |||
|
|||
private[elasticsearch] final case class RangeQuery[A, LB <: LowerBound, UB <: UpperBound] private ( | |||
private[elasticsearch] final case class RangeQuery[A, LB <: LowerBound, UB <: UpperBound]( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you omit private
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed private because of boost default argument would raise a warning: private default argument in object RangeQuery is never used during
during compilation on scala 2.12.17.
I saw that you removed all default arguments on case class, I'll make the changes !
c531a22
to
bed428e
Compare
5f38b7a
to
b551983
Compare
b551983
to
0ca53f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! 🤍
This PR addresses #66.