-
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 38 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 |
---|---|---|
|
@@ -29,14 +29,30 @@ public class TypeEnvironment implements Environment<Symbol, ExprType> { | |
private final TypeEnvironment parent; | ||
private final SymbolTable symbolTable; | ||
|
||
@Getter | ||
private final SymbolTable reservedSymbolTable; | ||
|
||
/** | ||
* Constructor with empty symbol tables. | ||
* | ||
* @param parent parent environment | ||
*/ | ||
public TypeEnvironment(TypeEnvironment parent) { | ||
this.parent = parent; | ||
this.symbolTable = new SymbolTable(); | ||
this.reservedSymbolTable = new SymbolTable(); | ||
} | ||
|
||
/** | ||
* Constructor with empty reserved symbol table. | ||
* | ||
* @param parent parent environment | ||
* @param symbolTable type table | ||
*/ | ||
public TypeEnvironment(TypeEnvironment parent, SymbolTable symbolTable) { | ||
this.parent = parent; | ||
this.symbolTable = symbolTable; | ||
this.reservedSymbolTable = new SymbolTable(); | ||
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. Why is a separate symbol table needed? Why can't we add metafields to 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. To separate out 'reserved fields' (that are in all tables) from fields that are specific to the index. The code needs to know the difference between these two lists later on. |
||
} | ||
|
||
/** | ||
|
@@ -59,6 +75,7 @@ public ExprType resolve(Symbol symbol) { | |
|
||
/** | ||
* Resolve all fields in the current environment. | ||
* | ||
* @param namespace a namespace | ||
* @return all symbols in the namespace | ||
*/ | ||
|
@@ -102,7 +119,11 @@ public void remove(ReferenceExpression ref) { | |
* Clear all fields in the current environment. | ||
*/ | ||
public void clearAllFields() { | ||
lookupAllFields(FIELD_NAME).keySet().stream() | ||
.forEach(v -> remove(new Symbol(Namespace.FIELD_NAME, v))); | ||
lookupAllFields(FIELD_NAME).keySet().forEach( | ||
v -> remove(new Symbol(Namespace.FIELD_NAME, v))); | ||
} | ||
|
||
public void addReservedWord(Symbol symbol, ExprType type) { | ||
reservedSymbolTable.store(symbol, type); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* 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 Literal relevanceFieldWeight; | ||
|
||
@Override | ||
public <T, C> T accept(AbstractNodeVisitor<T, C> nodeVisitor, C context) { | ||
return nodeVisitor.visitScoreFunction(this, context); | ||
} | ||
|
||
@Override | ||
public List<UnresolvedExpression> getChild() { | ||
return List.of(relevanceQuery); | ||
} | ||
} |
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?