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.

Sign up for GitHub

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

Allow setting default dialect for search module #3452

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Allow setting default dialect for search module #3452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89e6948
Allow setting default search dialect
sazzad16 Jun 1, 2023
792a9c1
Merge remote-tracking branch 'redis/master' into rs-aggr-dialect
sazzad16 Jun 1, 2023
3c4764a
undo the comment as the plan was not implemented
sazzad16 Jun 1, 2023
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
2 changes: 2 additions & 0 deletions docs/jedis5-breaking.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@

- `addCommandEncodedArguments` and `addCommandBinaryArguments` methods have been removed from `FieldName` class.

- `getArgs` method is removed from `AggregationBuilder` class.

- `limit` and `getArgs` methods have been removed from `Group` class.

- `Reducer` abstract class is refactored:
Expand Down
33 changes: 21 additions & 12 deletions src/main/java/redis/clients/jedis/CommandObjects.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static redis.clients.jedis.Protocol.Keyword.*;

import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import org.json.JSONArray;
import org.json.JSONObject;
Expand Down Expand Up @@ -40,6 +41,7 @@ protected void setProtocol(RedisProtocol proto) {
}

private volatile JsonObjectMapper jsonObjectMapper;
private final AtomicInteger searchDialect = new AtomicInteger(0);

private JedisBroadcastAndRoundRobinConfig broadcastAndRoundRobinConfig = null;

Expand Down Expand Up @@ -3174,32 +3176,34 @@ public final CommandObject<SearchResult> ftSearch(String indexName, String query

public final CommandObject<SearchResult> ftSearch(String indexName, String query, FTSearchParams params) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName)
.add(query).addParams(params), new SearchResultBuilder(!params.getNoContent(), params.getWithScores(), true));
.add(query).addParams(params.dialectOptional(searchDialect.get())), new SearchResultBuilder(!params.getNoContent(), params.getWithScores(), true));
}

public final CommandObject<SearchResult> ftSearch(String indexName, Query query) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName).addParams(query),
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName)
.addParams(query.dialectOptional(searchDialect.get())),
new SearchResultBuilder(!query.getNoContent(), query.getWithScores(), true));
}

public final CommandObject<SearchResult> ftSearch(byte[] indexName, Query query) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName).addParams(query),
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName)
.addParams(query.dialectOptional(searchDialect.get())),
new SearchResultBuilder(!query.getNoContent(), query.getWithScores(), false));
}

public final CommandObject<String> ftExplain(String indexName, Query query) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.EXPLAIN), indexName)
.addParams(query), BuilderFactory.STRING);
.addParams(query.dialectOptional(searchDialect.get())), BuilderFactory.STRING);
}

public final CommandObject<List<String>> ftExplainCLI(String indexName, Query query) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.EXPLAINCLI), indexName)
.addParams(query), BuilderFactory.STRING_LIST);
.addParams(query.dialectOptional(searchDialect.get())), BuilderFactory.STRING_LIST);
}

public final CommandObject<AggregationResult> ftAggregate(String indexName, AggregationBuilder aggr) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.AGGREGATE), indexName)
.addObjects(aggr.getArgs()), !aggr.isWithCursor() ? SearchBuilderFactory.SEARCH_AGGREGATION_RESULT
.addParams(aggr.dialectOptional(searchDialect.get())), !aggr.isWithCursor() ? SearchBuilderFactory.SEARCH_AGGREGATION_RESULT
: SearchBuilderFactory.SEARCH_AGGREGATION_RESULT_WITH_CURSOR);
}

Expand All @@ -3218,7 +3222,7 @@ public final CommandObject<Map.Entry<AggregationResult, Map<String, Object>>> ft
String indexName, FTProfileParams profileParams, AggregationBuilder aggr) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.PROFILE), indexName)
.add(SearchKeyword.AGGREGATE).addParams(profileParams).add(SearchKeyword.QUERY)
.addObjects(aggr.getArgs()), new SearchProfileResponseBuilder<>(!aggr.isWithCursor()
.addParams(aggr.dialectOptional(searchDialect.get())), new SearchProfileResponseBuilder<>(!aggr.isWithCursor()
? SearchBuilderFactory.SEARCH_AGGREGATION_RESULT
: SearchBuilderFactory.SEARCH_AGGREGATION_RESULT_WITH_CURSOR));
}
Expand All @@ -3227,16 +3231,16 @@ public final CommandObject<Map.Entry<SearchResult, Map<String, Object>>> ftProfi
String indexName, FTProfileParams profileParams, Query query) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.PROFILE), indexName)
.add(SearchKeyword.SEARCH).addParams(profileParams).add(SearchKeyword.QUERY)
.addParams(query), new SearchProfileResponseBuilder<>(new SearchResultBuilder(
!query.getNoContent(), query.getWithScores(), true)));
.addParams(query.dialectOptional(searchDialect.get())), new SearchProfileResponseBuilder<>(
new SearchResultBuilder(!query.getNoContent(), query.getWithScores(), true)));
}

public final CommandObject<Map.Entry<SearchResult, Map<String, Object>>> ftProfileSearch(
String indexName, FTProfileParams profileParams, String query, FTSearchParams searchParams) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.PROFILE), indexName)
.add(SearchKeyword.SEARCH).addParams(profileParams).add(SearchKeyword.QUERY).add(query)
.addParams(searchParams), new SearchProfileResponseBuilder<>(new SearchResultBuilder(
!searchParams.getNoContent(), searchParams.getWithScores(), true)));
.addParams(searchParams.dialectOptional(searchDialect.get())), new SearchProfileResponseBuilder<>(
new SearchResultBuilder(!searchParams.getNoContent(), searchParams.getWithScores(), true)));
}

public final CommandObject<String> ftDropIndex(String indexName) {
Expand Down Expand Up @@ -3292,7 +3296,7 @@ public final CommandObject<Map<String, Map<String, Double>>> ftSpellCheck(String
public final CommandObject<Map<String, Map<String, Double>>> ftSpellCheck(String index, String query,
FTSpellCheckParams spellCheckParams) {
return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SPELLCHECK), index).add(query)
.addParams(spellCheckParams), SearchBuilderFactory.SEARCH_SPELLCHECK_RESPONSE);
.addParams(spellCheckParams.dialectOptional(searchDialect.get())), SearchBuilderFactory.SEARCH_SPELLCHECK_RESPONSE);
}

public final CommandObject<Map<String, Object>> ftInfo(String indexName) {
Expand Down Expand Up @@ -4168,6 +4172,11 @@ public void setJsonObjectMapper(JsonObjectMapper jsonObjectMapper) {
this.jsonObjectMapper = jsonObjectMapper;
}

public void setDefaultSearchDialect(int dialect) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue says This configuration option should be optional. If it is not specified, the client should fall back to the current behavior.

Raising this argument isn't that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Firstly, users would be using the one in UnifiedJedis. I guess we're concerned about that and so check my answer in test file.

if (dialect == 0) throw new IllegalArgumentException("DIALECT=0 cannot be set.");
this.searchDialect.set(dialect);
}

private class SearchProfileResponseBuilder<T> extends Builder<Map.Entry<T, Map<String, Object>>> {

private final Builder<T> replyBuilder;
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/redis/clients/jedis/UnifiedJedis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4786,4 +4786,8 @@ public Object executeCommand(CommandArguments args) {
public void setJsonObjectMapper(JsonObjectMapper jsonObjectMapper) {
this.commandObjects.setJsonObjectMapper(jsonObjectMapper);
}

public void setDefaultSearchDialect(int dialect) {
this.commandObjects.setDefaultSearchDialect(dialect);
}
}
12 changes: 12 additions & 0 deletions src/main/java/redis/clients/jedis/search/FTSearchParams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,18 @@ public FTSearchParams dialect(int dialect) {
return this;
}

/**
* This method will not replace the dialect if it has been already set.
* @param dialect dialect
* @return this
*/
public FTSearchParams dialectOptional(int dialect) {
if (dialect != 0 && this.dialect == null) {
this.dialect = dialect;
}
return this;
}

public boolean getNoContent() {
return noContent;
}
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/redis/clients/jedis/search/FTSpellCheckParams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,18 @@ public FTSpellCheckParams dialect(int dialect) {
return this;
}

/**
* This method will not replace the dialect if it has been already set.
* @param dialect dialect
* @return this
*/
public FTSpellCheckParams dialectOptional(int dialect) {
if (dialect != 0 && this.dialect == null) {
this.dialect = dialect;
}
return this;
}

@Override
public void addParams(CommandArguments args) {

Expand Down
16 changes: 14 additions & 2 deletions src/main/java/redis/clients/jedis/search/Query.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public HighlightTags(String open, String close) {
private boolean wantsSummarize = false;
private String _scorer = null;
private Map<String, Object> _params = null;
private int _dialect = 0;
private Integer _dialect;
private int _slop = -1;
private long _timeout = -1;
private boolean _inOrder = false;
Expand Down Expand Up @@ -299,7 +299,7 @@ public void addParams(CommandArguments args) {
}
}

if (_dialect != 0) {
if (_dialect != null) {
args.add(SearchKeyword.DIALECT.getRaw());
args.add(_dialect);
}
Expand Down Expand Up @@ -544,6 +544,18 @@ public Query dialect(int dialect) {
return this;
}

/**
* This method will not replace the dialect if it has been already set.
* @param dialect dialect
* @return this
*/
public Query dialectOptional(int dialect) {
if (dialect != 0 && this._dialect == null) {
this._dialect = dialect;
}
return this;
}

/**
* Set the slop to execute the query accordingly
*
Expand Down
Loading