Skip to content
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

Refactor OpenSearch Relevance Functions out of Core #2019

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
import org.opensearch.sql.expression.function.BuiltinFunctionName;
import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
import org.opensearch.sql.expression.function.FunctionName;
import org.opensearch.sql.expression.function.OpenSearchFunctions;
import org.opensearch.sql.expression.function.OpenSearchFunction;
import org.opensearch.sql.expression.parse.ParseExpression;
import org.opensearch.sql.expression.span.SpanExpression;
import org.opensearch.sql.expression.window.aggregation.AggregateWindowFunction;
Expand Down Expand Up @@ -273,9 +273,8 @@ public Expression visitScoreFunction(ScoreFunction node, AnalysisContext context
// create a new function expression with boost argument and resolve it
Function updatedRelevanceQueryUnresolvedExpr =
new Function(relevanceQueryUnresolvedExpr.getFuncName(), updatedFuncArgs);
OpenSearchFunctions.OpenSearchFunction relevanceQueryExpr =
(OpenSearchFunctions.OpenSearchFunction)
updatedRelevanceQueryUnresolvedExpr.accept(this, context);
OpenSearchFunction relevanceQueryExpr =
(OpenSearchFunction) updatedRelevanceQueryUnresolvedExpr.accept(this, context);
relevanceQueryExpr.setScoreTracked(true);
return relevanceQueryExpr;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import org.opensearch.sql.expression.conditional.cases.CaseClause;
import org.opensearch.sql.expression.conditional.cases.WhenClause;
import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
import org.opensearch.sql.expression.function.OpenSearchFunctions;
import org.opensearch.sql.expression.function.OpenSearchFunction;
import org.opensearch.sql.planner.logical.LogicalAggregation;
import org.opensearch.sql.planner.logical.LogicalPlan;
import org.opensearch.sql.planner.logical.LogicalPlanNodeVisitor;
Expand Down Expand Up @@ -75,9 +75,9 @@ public Expression visitFunction(FunctionExpression node, AnalysisContext context
(Expression)
repository.compile(context.getFunctionProperties(), node.getFunctionName(), args);
// Propagate scoreTracked for OpenSearch functions
if (optimizedFunctionExpression instanceof OpenSearchFunctions.OpenSearchFunction) {
((OpenSearchFunctions.OpenSearchFunction) optimizedFunctionExpression)
.setScoreTracked(((OpenSearchFunctions.OpenSearchFunction) node).isScoreTracked());
if (optimizedFunctionExpression instanceof OpenSearchFunction) {
((OpenSearchFunction) optimizedFunctionExpression)
.setScoreTracked(((OpenSearchFunction) node).isScoreTracked());
}
return optimizedFunctionExpression;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.opensearch.sql.datasource.model;

import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
import static org.opensearch.sql.data.type.ExprCoreType.STRING;

import com.google.common.collect.ImmutableMap;
import java.util.Map;
import java.util.Set;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.datasource.DataSourceService;
import org.opensearch.sql.planner.logical.LogicalPlan;
import org.opensearch.sql.planner.physical.PhysicalPlan;
import org.opensearch.sql.storage.StorageEngine;
import org.opensearch.sql.storage.Table;

public class EmptyDataSourceService {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a javadoc and a good description why it is here

private static DataSourceService emptyDataSourceService =
new DataSourceService() {
@Override
public DataSource getDataSource(String dataSourceName) {
return new DataSource(
DEFAULT_DATASOURCE_NAME, DataSourceType.OPENSEARCH, storageEngine());
}

@Override
public Set<DataSourceMetadata> getDataSourceMetadata(boolean isDefaultDataSourceRequired) {
return Set.of();
}

@Override
public DataSourceMetadata getDataSourceMetadata(String name) {
return null;
}

@Override
public void createDataSource(DataSourceMetadata metadata) {}

@Override
public void updateDataSource(DataSourceMetadata dataSourceMetadata) {}

@Override
public void deleteDataSource(String dataSourceName) {}

@Override
public Boolean dataSourceExists(String dataSourceName) {
return null;
}
};

private static StorageEngine storageEngine() {
Table table =
new Table() {
@Override
public boolean exists() {
return true;
}

@Override
public void create(Map<String, ExprType> schema) {
throw new UnsupportedOperationException("Create table is not supported");
}

@Override
public Map<String, ExprType> getFieldTypes() {
return null;
}

@Override
public PhysicalPlan implement(LogicalPlan plan) {
throw new UnsupportedOperationException();
}

public Map<String, ExprType> getReservedFieldTypes() {
return ImmutableMap.of("_test", STRING);
}
};
return (dataSourceSchemaName, tableName) -> table;
}

public static DataSourceService getEmptyDataSourceService() {
return emptyDataSourceService;
}
}
56 changes: 3 additions & 53 deletions core/src/main/java/org/opensearch/sql/expression/DSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package org.opensearch.sql.expression;

import static org.opensearch.sql.datasource.model.EmptyDataSourceService.getEmptyDataSourceService;

import java.util.Arrays;
import org.opensearch.sql.ast.expression.SpanUnit;
import org.opensearch.sql.data.model.ExprShortValue;
Expand Down Expand Up @@ -119,10 +121,6 @@ public static NamedArgumentExpression namedArgument(String argName, Expression v
return new NamedArgumentExpression(argName, value);
}

public static NamedArgumentExpression namedArgument(String name, String value) {
return namedArgument(name, literal(value));
}

public static GrokExpression grok(
Expression sourceField, Expression pattern, Expression identifier) {
return new GrokExpression(sourceField, pattern, identifier);
Expand Down Expand Up @@ -823,54 +821,6 @@ public static FunctionExpression typeof(Expression value) {
return compile(FunctionProperties.None, BuiltinFunctionName.TYPEOF, value);
}

public static FunctionExpression match(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.MATCH, args);
}

public static FunctionExpression match_phrase(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.MATCH_PHRASE, args);
}

public static FunctionExpression match_phrase_prefix(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.MATCH_PHRASE_PREFIX, args);
}

public static FunctionExpression multi_match(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.MULTI_MATCH, args);
}

public static FunctionExpression simple_query_string(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.SIMPLE_QUERY_STRING, args);
}

public static FunctionExpression query(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.QUERY, args);
}

public static FunctionExpression query_string(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.QUERY_STRING, args);
}

public static FunctionExpression match_bool_prefix(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.MATCH_BOOL_PREFIX, args);
}

public static FunctionExpression wildcard_query(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.WILDCARD_QUERY, args);
}

public static FunctionExpression score(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.SCORE, args);
}

public static FunctionExpression scorequery(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.SCOREQUERY, args);
}

public static FunctionExpression score_query(Expression... args) {
return compile(FunctionProperties.None, BuiltinFunctionName.SCORE_QUERY, args);
}

public static FunctionExpression now(FunctionProperties functionProperties, Expression... args) {
return compile(functionProperties, BuiltinFunctionName.NOW, args);
}
Expand Down Expand Up @@ -953,7 +903,7 @@ public static FunctionExpression utc_timestamp(
private static <T extends FunctionImplementation> T compile(
FunctionProperties functionProperties, BuiltinFunctionName bfn, Expression... args) {
return (T)
BuiltinFunctionRepository.getInstance()
BuiltinFunctionRepository.getInstance(getEmptyDataSourceService())
.compile(functionProperties, bfn.getName(), Arrays.asList(args));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.apache.commons.lang3.tuple.Pair;
import org.opensearch.sql.common.utils.StringUtils;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.datasource.DataSourceService;
import org.opensearch.sql.datasource.model.DataSourceMetadata;
import org.opensearch.sql.exception.ExpressionEvaluationException;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.aggregation.AggregatorFunction;
Expand All @@ -46,7 +49,7 @@ public class BuiltinFunctionRepository {
private final Map<FunctionName, FunctionResolver> functionResolverMap;

/** The singleton instance. */
private static BuiltinFunctionRepository instance;
private static final Map<Integer, BuiltinFunctionRepository> instance = new HashMap<>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extend the comment and probably rename to instances.


/**
* Construct a function repository with the given function registered. This is only used in test.
Expand All @@ -64,25 +67,45 @@ public class BuiltinFunctionRepository {
*
* @return singleton instance
*/
public static synchronized BuiltinFunctionRepository getInstance() {
if (instance == null) {
instance = new BuiltinFunctionRepository(new HashMap<>());
public static synchronized BuiltinFunctionRepository getInstance(
DataSourceService dataSourceService) {
Set<DataSourceMetadata> dataSourceMetadataSet = dataSourceService.getDataSourceMetadata(true);
Set<Integer> dataSourceServiceHashSet =
dataSourceMetadataSet.stream()
.map(metadata -> metadata.hashCode())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it could be optimized to

Suggested change
.map(metadata -> metadata.hashCode())
.map(Object::hashCode)

.collect(Collectors.toSet());

// Creates new Repository for every dataSourceService
if (!dataSourceServiceHashSet.stream().anyMatch(hash -> instance.containsKey(hash))) {
BuiltinFunctionRepository repository = new BuiltinFunctionRepository(new HashMap<>());

// Register all built-in functions
ArithmeticFunction.register(instance);
BinaryPredicateOperator.register(instance);
MathematicalFunction.register(instance);
UnaryPredicateOperator.register(instance);
AggregatorFunction.register(instance);
DateTimeFunction.register(instance);
IntervalClause.register(instance);
WindowFunctions.register(instance);
TextFunction.register(instance);
TypeCastOperator.register(instance);
SystemFunctions.register(instance);
OpenSearchFunctions.register(instance);
MitchellGale marked this conversation as resolved.
Show resolved Hide resolved
ArithmeticFunction.register(repository);
BinaryPredicateOperator.register(repository);
MathematicalFunction.register(repository);
UnaryPredicateOperator.register(repository);
AggregatorFunction.register(repository);
DateTimeFunction.register(repository);
IntervalClause.register(repository);
WindowFunctions.register(repository);
TextFunction.register(repository);
TypeCastOperator.register(repository);
SystemFunctions.register(repository);
// Temporary as part of https://github.com/opensearch-project/sql/issues/811
// TODO: remove this resolver when Analyzers are moved to opensearch module
repository.register(new NestedFunctionResolver());

for (DataSourceMetadata metadata : dataSourceMetadataSet) {
dataSourceService
.getDataSource(metadata.getName())
.getStorageEngine()
.getFunctions()
.forEach(function -> repository.register(function));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.forEach(function -> repository.register(function));
.forEach(repository::register);

instance.put(metadata.hashCode(), repository);
}
Comment on lines +104 to +105
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
instance.put(metadata.hashCode(), repository);
}
}
instance.put(metadata.hashCode(), repository);

return repository;
}
return instance;
return instance.get(dataSourceServiceHashSet.iterator().next());
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.expression.function;

import org.apache.commons.lang3.tuple.Pair;
import org.opensearch.sql.data.model.ExprValue;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.FunctionExpression;
import org.opensearch.sql.expression.env.Environment;

/**
* Nested Function Resolver returns a builder to resolve nested function expressions
*/
public class NestedFunctionResolver implements FunctionResolver {
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
@Override
public Pair<FunctionSignature, FunctionBuilder> resolve(FunctionSignature unresolvedSignature) {
return Pair.of(
unresolvedSignature,
(functionProperties, arguments) ->
new FunctionExpression(BuiltinFunctionName.NESTED.getName(), arguments) {
@Override
public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
return valueEnv.resolve(getArguments().get(0));
}

@Override
public ExprType type() {
return getArguments().get(0).type();
}
});
}

@Override
public FunctionName getFunctionName() {
return BuiltinFunctionName.NESTED.getName();
}
}
Loading
Loading