-
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
Add Highlight In SQL #96
Changes from 20 commits
1097d9f
1a453e1
0d5c87b
26d0b7e
3f10b8b
543d0d7
f47ffe7
5fdb939
5c8db0a
ac9f080
b526132
807c475
74b6492
ad7affc
a192a21
092d054
9bdbb86
9b463cf
2648e0c
7290e90
2feb2c4
cd6f911
3db8788
3ee3491
e63b7b9
11d834b
373353b
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 |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package org.opensearch.sql.analysis; | ||
|
||
import lombok.RequiredArgsConstructor; | ||
import org.opensearch.sql.ast.AbstractNodeVisitor; | ||
import org.opensearch.sql.ast.expression.Alias; | ||
import org.opensearch.sql.ast.expression.HighlightFunction; | ||
import org.opensearch.sql.ast.expression.UnresolvedExpression; | ||
import org.opensearch.sql.expression.Expression; | ||
import org.opensearch.sql.planner.logical.LogicalHighlight; | ||
import org.opensearch.sql.planner.logical.LogicalPlan; | ||
|
||
@RequiredArgsConstructor | ||
public class HighlightAnalyzer extends AbstractNodeVisitor<LogicalPlan, AnalysisContext> { | ||
private final ExpressionAnalyzer expressionAnalyzer; | ||
private final LogicalPlan child; | ||
|
||
public LogicalPlan analyze(UnresolvedExpression projectItem, AnalysisContext context) { | ||
LogicalPlan highlight = projectItem.accept(this, context); | ||
return (highlight == null) ? child : highlight; | ||
} | ||
|
||
@Override | ||
public LogicalPlan visitAlias(Alias node, AnalysisContext context) { | ||
if (!(node.getDelegated() instanceof HighlightFunction)) { | ||
return null; | ||
} | ||
|
||
HighlightFunction unresolved = (HighlightFunction) node.getDelegated(); | ||
Expression field = expressionAnalyzer.analyze(unresolved.getHighlightField(), context); | ||
return new LogicalHighlight(child, field); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package org.opensearch.sql.analysis; | ||
|
||
import com.google.common.collect.ImmutableMap; | ||
import java.util.List; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.Getter; | ||
import lombok.ToString; | ||
import org.opensearch.sql.common.utils.StringUtils; | ||
import org.opensearch.sql.data.model.ExprTupleValue; | ||
import org.opensearch.sql.data.model.ExprValue; | ||
import org.opensearch.sql.data.model.ExprValueUtils; | ||
import org.opensearch.sql.data.type.ExprCoreType; | ||
import org.opensearch.sql.data.type.ExprType; | ||
import org.opensearch.sql.expression.DSL; | ||
import org.opensearch.sql.expression.Expression; | ||
import org.opensearch.sql.expression.FunctionExpression; | ||
import org.opensearch.sql.expression.env.Environment; | ||
import org.opensearch.sql.expression.function.BuiltinFunctionName; | ||
|
||
@EqualsAndHashCode(callSuper = false) | ||
@Getter | ||
@ToString | ||
public class HighlightExpression extends FunctionExpression { | ||
private final Expression highlightField; | ||
|
||
/** | ||
* HighlightExpression Constructor. | ||
* @param highlightField : Highlight field for expression. | ||
*/ | ||
public HighlightExpression(Expression highlightField) { | ||
super(BuiltinFunctionName.HIGHLIGHT.getName(), List.of(highlightField)); | ||
this.highlightField = highlightField; | ||
} | ||
|
||
/** | ||
* Return String or Map value matching highlight field. | ||
* @param valueEnv : Dataset to parse value from. | ||
* @return : String or Map value of highlight fields. | ||
*/ | ||
@Override | ||
public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) { | ||
String refName = "_highlight"; | ||
if (!getHighlightField().toString().contains("*")) { | ||
refName += "." + StringUtils.unquoteText(getHighlightField().toString()); | ||
} | ||
ExprValue retVal = valueEnv.resolve(DSL.ref(refName, ExprCoreType.STRING)); | ||
ImmutableMap.Builder<String, ExprValue> builder = new ImmutableMap.Builder<>(); | ||
|
||
if (retVal.isMissing() || retVal.type() != ExprCoreType.STRUCT) { | ||
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. When this condition is true? 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. If there was no highlight value parsed from the return, or only a single highlight field is found. |
||
return retVal; | ||
} | ||
|
||
var hlBuilder = ImmutableMap.<String, ExprValue>builder(); | ||
hlBuilder.putAll(retVal.tupleValue()); | ||
|
||
for (var entry : retVal.tupleValue().entrySet()) { | ||
String entryKey = "highlight(" + getHighlightField() + ")." + entry.getKey(); | ||
builder.put(entryKey, ExprValueUtils.stringValue(entry.getValue().toString())); | ||
} | ||
|
||
return ExprTupleValue.fromExprValueMap(builder.build()); | ||
} | ||
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. Is it worth returning always a map? 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. Line 55 returns only one field in the instance there aren't multiple highlight fields returned. |
||
|
||
/** | ||
* Get type for HighlightExpression. | ||
* @return : String type. | ||
*/ | ||
@Override | ||
public ExprType type() { | ||
return ExprCoreType.STRING; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/* | ||
* 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; | ||
|
||
@AllArgsConstructor | ||
@EqualsAndHashCode(callSuper = false) | ||
@Getter | ||
@ToString | ||
public class HighlightFunction extends UnresolvedExpression { | ||
acarbonetto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private final UnresolvedExpression highlightField; | ||
|
||
@Override | ||
public <T, C> T accept(AbstractNodeVisitor<T, C> nodeVisitor, C context) { | ||
return nodeVisitor.visitHighlight(this, context); | ||
} | ||
|
||
@Override | ||
public List<UnresolvedExpression> getChild() { | ||
return List.of(highlightField); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package org.opensearch.sql.planner.logical; | ||
|
||
import java.util.Collections; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.Getter; | ||
import lombok.ToString; | ||
import org.opensearch.sql.expression.Expression; | ||
|
||
@EqualsAndHashCode(callSuper = true) | ||
@Getter | ||
@ToString | ||
public class LogicalHighlight extends LogicalPlan { | ||
private final Expression highlightField; | ||
|
||
public LogicalHighlight(LogicalPlan childPlan, Expression field) { | ||
super(Collections.singletonList(childPlan)); | ||
highlightField = field; | ||
} | ||
|
||
@Override | ||
public <R, C> R accept(LogicalPlanNodeVisitor<R, C> visitor, C context) { | ||
return visitor.visitHighlight(this, context); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package org.opensearch.sql.planner.physical; | ||
|
||
import com.google.common.collect.ImmutableMap; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.Getter; | ||
import lombok.NonNull; | ||
import org.opensearch.sql.data.model.ExprValue; | ||
import org.opensearch.sql.expression.Expression; | ||
|
||
@EqualsAndHashCode(callSuper = false) | ||
@Getter | ||
public class HighlightOperator extends PhysicalPlan { | ||
@NonNull | ||
private final PhysicalPlan input; | ||
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 do you store a parent class instance there? 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. In line with Peng's comment in the POC, this class ended up not doing anything and can be removed. 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. Seems to be standard considering other operators also store the parent class instance like this. |
||
private final Expression highlightField; | ||
|
||
public HighlightOperator(PhysicalPlan input, Expression highlightField) { | ||
this.input = input; | ||
this.highlightField = highlightField; | ||
} | ||
|
||
@Override | ||
public <R, C> R accept(PhysicalPlanNodeVisitor<R, C> visitor, C context) { | ||
return visitor.visitHighlight(this, context); | ||
} | ||
|
||
@Override | ||
public List<PhysicalPlan> getChild() { | ||
return Collections.singletonList(input); | ||
} | ||
|
||
@Override | ||
public boolean hasNext() { | ||
return input.hasNext(); | ||
} | ||
|
||
@Override | ||
public ExprValue next() { | ||
ExprValue inputValue = input.next(); | ||
ImmutableMap.Builder<String, ExprValue> mapBuilder = new ImmutableMap.Builder<>(); | ||
inputValue.tupleValue().forEach(mapBuilder::put); | ||
|
||
return inputValue; | ||
} | ||
} |
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.
Class needs a javadoc