-
Notifications
You must be signed in to change notification settings - Fork 0
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
#639: allow metadata fields and score opensearch function #228
Changes from 27 commits
d8b412d
5eaa344
ea25455
9f1dcca
5a70363
8c0aaf6
4d8229d
4630c98
9a10273
0a92969
3b5e900
348b8b9
f67d4f2
be7191a
1a0b3ef
05d9dd3
996a166
2c469c7
4c7a5d7
dec36fa
7aab6ee
fcb3470
6e52d13
0d3beab
1c05aba
f97dfea
3ba3c85
dd15853
543d232
9d59e9f
7a05276
e75d6b5
fecf615
47636c3
6d9853c
f93f7c4
1340f11
bdc3020
31f0617
55b93ac
0410521
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,26 +13,55 @@ | |
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Optional; | ||
import java.util.stream.StreamSupport; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.Getter; | ||
import org.opensearch.sql.ast.AbstractNodeVisitor; | ||
import org.opensearch.sql.data.type.ExprCoreType; | ||
|
||
@Getter | ||
@EqualsAndHashCode(callSuper = false) | ||
public class QualifiedName extends UnresolvedExpression { | ||
private final List<String> parts; | ||
|
||
private final Boolean isMetadataField; | ||
acarbonetto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
public QualifiedName(String name) { | ||
this(name, Boolean.FALSE); | ||
} | ||
|
||
public static final String METADATA_FIELD_ID = "_id"; | ||
public static final String METADATA_FIELD_INDEX = "_index"; | ||
public static final String METADATA_FIELD_SCORE = "_score"; | ||
public static final String METADATA_FIELD_MAXSCORE = "_maxscore"; | ||
public static final String METADATA_FIELD_SORT = "_sort"; | ||
public static final java.util.Map<String, ExprCoreType> METADATAFIELD_TYPE_MAP = new HashMap<>() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may not want to hardcode this in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't use OpenSearchIndex as it would create a dependency on opensearch in core. Not ideal. We kind of need to wait until we have a user-defined functions that could override core visitors, like QualifiedName. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I didn't mean you need to depend on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh. Interesting. I'll see if I can hook that up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see changes in:
|
||
{ | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
put(METADATA_FIELD_ID, ExprCoreType.STRING); | ||
put(METADATA_FIELD_INDEX, ExprCoreType.STRING); | ||
put(METADATA_FIELD_SCORE, ExprCoreType.FLOAT); | ||
put(METADATA_FIELD_MAXSCORE, ExprCoreType.FLOAT); | ||
put(METADATA_FIELD_SORT, ExprCoreType.LONG); | ||
} | ||
}; | ||
|
||
public QualifiedName(String name, Boolean isMetadataField) { | ||
this.parts = Collections.singletonList(name); | ||
this.isMetadataField = isMetadataField; | ||
} | ||
|
||
public QualifiedName(Iterable<String> parts) { | ||
this(parts, Boolean.FALSE); | ||
} | ||
|
||
/** | ||
* QualifiedName Constructor. | ||
*/ | ||
public QualifiedName(Iterable<String> parts) { | ||
public QualifiedName(Iterable<String> parts, Boolean isMetadataField) { | ||
this.isMetadataField = isMetadataField; | ||
List<String> partsList = StreamSupport.stream(parts.spliterator(), false).collect(toList()); | ||
if (partsList.isEmpty()) { | ||
throw new IllegalArgumentException("parts is empty"); | ||
|
@@ -110,4 +139,8 @@ public List<UnresolvedExpression> getChild() { | |
public <R, C> R accept(AbstractNodeVisitor<R, C> nodeVisitor, C context) { | ||
return nodeVisitor.visitQualifiedName(this, context); | ||
} | ||
|
||
public Boolean isMetadataField() { | ||
return this.isMetadataField; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package org.opensearch.sql.ast.expression; | ||
|
||
import java.util.List; | ||
import lombok.AllArgsConstructor; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.Getter; | ||
import lombok.ToString; | ||
import org.opensearch.sql.ast.AbstractNodeVisitor; | ||
|
||
/** | ||
* Expression node of Score function. | ||
* Score takes a relevance-search expression as an argument and returns it | ||
*/ | ||
@AllArgsConstructor | ||
@EqualsAndHashCode(callSuper = false) | ||
@Getter | ||
@ToString | ||
public class ScoreFunction extends UnresolvedExpression { | ||
private final UnresolvedExpression relevanceQuery; | ||
private final List<UnresolvedExpression> funcArgs; | ||
|
||
@Override | ||
public <T, C> T accept(AbstractNodeVisitor<T, C> nodeVisitor, C context) { | ||
return nodeVisitor.visitScoreFunction(this, context); | ||
} | ||
|
||
@Override | ||
public List<UnresolvedExpression> getChild() { | ||
List<UnresolvedExpression> resultingList = List.of(relevanceQuery); | ||
resultingList.addAll(funcArgs); | ||
return resultingList; | ||
} | ||
} |
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 think we need to add this to
opensearch
module as storage-specific function. Personally I think we should prioritize opensearch-project#811. It will become more and more difficult as we keep adding more OpenSearch logic tocore
engine.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.
Agreed. I've got it on our radar, and we can start to scope it out. As is, there's quite a bit of work to do to pull out the opensearch specific classes, but I think it's do-able.
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.
We've had storage-specific function support in opensearch-project#1354. Can we start this now instead of adding more and more special logic to
core
? Agreed there is quite lots of work to move all toopensearch
but maybe easy to do this for single PR?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.
Let me take a look. There's already a lot in this PR.
Would you mind if we move the score function out as a proof of concept for a set of OpenSearch storage engine functions?