Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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 @@ -5,9 +5,8 @@

package org.opensearch.sql.legacy.executor;

import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME;
import static org.opensearch.search.sort.SortOrder.ASC;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import java.io.IOException;
import org.apache.logging.log4j.LogManager;
Expand All @@ -16,7 +15,6 @@
import org.opensearch.action.search.SearchResponse;
import org.opensearch.search.SearchHits;
import org.opensearch.search.builder.PointInTimeBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.sql.legacy.domain.Select;
import org.opensearch.sql.legacy.exception.SqlParseException;
import org.opensearch.sql.legacy.pit.PointInTimeHandler;
Expand Down Expand Up @@ -66,8 +64,7 @@ public SearchResponse getResponseWithHits(
// Set sort field for search_after
boolean ordered = select.isOrderdSelect();
if (!ordered) {
request.addSort(DOC_FIELD_NAME, ASC);
request.addSort(METADATA_FIELD_ID, SortOrder.ASC);
request.addSort(SORT_FIELD_SHARD_DOC, ASC);
}
// Set PIT
request.setPointInTime(new PointInTimeBuilder(pit.getPitId()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package org.opensearch.sql.legacy.executor.join;

import static org.opensearch.core.xcontent.ToXContent.EMPTY_PARAMS;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import com.google.common.collect.ImmutableMap;
import java.io.IOException;
Expand All @@ -24,7 +24,6 @@
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.search.SearchHit;
import org.opensearch.search.SearchHits;
import org.opensearch.search.sort.FieldSortBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.sql.legacy.domain.Select;
import org.opensearch.sql.legacy.query.join.BackOffRetryStrategy;
Expand All @@ -39,8 +38,7 @@ public static SearchResponse scrollOneTimeWithHits(
requestBuilder.setScroll(new TimeValue(60000)).setSize(resultSize);
boolean ordered = originalSelect.isOrderdSelect();
if (!ordered) {
scrollRequest.addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
scrollRequest.addSort(METADATA_FIELD_ID, SortOrder.ASC);
scrollRequest.addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);
}
SearchResponse responseWithHits = scrollRequest.get();
// on ordered select - not using SCAN , elastic returns hits on first scroll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

package org.opensearch.sql.legacy.query;

import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import com.alibaba.druid.sql.ast.SQLExpr;
import com.alibaba.druid.sql.ast.expr.SQLBinaryOpExpr;
Expand Down Expand Up @@ -107,8 +107,7 @@ public void checkAndSetScroll() {
// set sort field for search_after
boolean ordered = select.isOrderdSelect();
if (!ordered) {
request.addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
request.addSort(METADATA_FIELD_ID, SortOrder.ASC);
request.addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);
}
// Request also requires PointInTime, but we should create pit while execution.
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@

package org.opensearch.sql.legacy.query.planner.physical.node.pointInTime;

import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.search.SearchHit;
import org.opensearch.search.builder.PointInTimeBuilder;
import org.opensearch.search.sort.FieldSortBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.sql.legacy.pit.PointInTimeHandlerImpl;
import org.opensearch.sql.legacy.query.join.TableInJoinRequestBuilder;
Expand Down Expand Up @@ -64,8 +63,7 @@ protected void loadFirstBatch() {
searchResponse =
request
.getRequestBuilder()
.addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC)
.addSort(METADATA_FIELD_ID, SortOrder.ASC)
.addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC)
.setSize(pageSize)
.setTimeout(TimeValue.timeValueSeconds(timeout))
.setPointInTime(new PointInTimeBuilder(pitId))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@

package org.opensearch.sql.legacy.query.planner.physical.node.scroll;

import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import org.opensearch.action.search.ClearScrollResponse;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.search.sort.FieldSortBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.sql.legacy.query.join.TableInJoinRequestBuilder;
import org.opensearch.sql.legacy.query.planner.physical.node.Paginate;
Expand Down Expand Up @@ -41,8 +40,7 @@ protected void loadFirstBatch() {
searchResponse =
request
.getRequestBuilder()
.addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC)
.addSort(METADATA_FIELD_ID, SortOrder.ASC)
.addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC)
.setSize(pageSize)
.setScroll(TimeValue.timeValueSeconds(timeout))
.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.*;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import java.util.*;
import org.junit.After;
Expand All @@ -17,7 +18,6 @@
import org.opensearch.action.search.SearchRequestBuilder;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.script.Script;
import org.opensearch.search.sort.FieldSortBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.sql.common.setting.Settings;
import org.opensearch.sql.legacy.domain.Field;
Expand Down Expand Up @@ -142,7 +142,7 @@ public void testIfScrollShouldBeOpenWithDifferentFormats() {
queryAction.setFormat(Format.JDBC);
queryAction.checkAndSetScroll();
Mockito.verify(mockRequestBuilder).setSize(settingFetchSize);
Mockito.verify(mockRequestBuilder).addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
Mockito.verify(mockRequestBuilder).addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);
Mockito.verify(mockRequestBuilder, never()).setScroll(timeValue);
}

Expand All @@ -162,7 +162,7 @@ public void testIfScrollShouldBeOpen() {
mockLocalClusterStateAndInitializeMetrics(timeValue);
queryAction.checkAndSetScroll();
Mockito.verify(mockRequestBuilder).setSize(settingFetchSize);
Mockito.verify(mockRequestBuilder).addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
Mockito.verify(mockRequestBuilder).addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);
Mockito.verify(mockRequestBuilder, never()).setScroll(timeValue);
}

Expand Down Expand Up @@ -190,7 +190,7 @@ public void testIfScrollShouldBeOpenWithDifferentFetchSize() {
doReturn(mockRequestBuilder).when(mockRequestBuilder).setSize(userFetchSize);
queryAction.checkAndSetScroll();
Mockito.verify(mockRequestBuilder).setSize(20);
Mockito.verify(mockRequestBuilder).addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
Mockito.verify(mockRequestBuilder).addSort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);
Mockito.verify(mockRequestBuilder, never()).setScroll(timeValue);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
package org.opensearch.sql.opensearch.request;

import static org.opensearch.core.xcontent.DeprecationHandler.IGNORE_DEPRECATIONS;
import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME;
import static org.opensearch.search.sort.SortOrder.ASC;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import java.io.IOException;
import java.util.Collections;
Expand Down Expand Up @@ -205,10 +204,7 @@ public OpenSearchResponse searchWithPIT(Function<SearchRequest, SearchResponse>
}
// Set sort field for search_after
if (this.sourceBuilder.sorts() == null) {
this.sourceBuilder.sort(DOC_FIELD_NAME, ASC);
// Workaround to preserve sort location more exactly,
// see https://github.com/opensearch-project/sql/pull/3061
this.sourceBuilder.sort(METADATA_FIELD_ID, ASC);
this.sourceBuilder.sort(SORT_FIELD_SHARD_DOC, ASC);
}
SearchRequest searchRequest =
new SearchRequest().indices(indexName.getIndexNames()).source(this.sourceBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import static java.util.stream.Collectors.toList;
import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
import static org.opensearch.index.query.QueryBuilders.nestedQuery;
import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME;
import static org.opensearch.search.sort.SortOrder.ASC;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -182,7 +182,7 @@ public void pushDownFilter(QueryBuilder query) {
}

if (sourceBuilder.sorts() == null) {
sourceBuilder.sort(DOC_FIELD_NAME, ASC); // Make sure consistent order
sourceBuilder.sort(SORT_FIELD_SHARD_DOC, ASC); // Make sure consistent order
}
}

Expand Down Expand Up @@ -313,7 +313,7 @@ public void pushDownCollapse(String field) {
private boolean isSortByDocOnly() {
List<SortBuilder<?>> sorts = sourceBuilder.sorts();
if (sorts != null) {
return sorts.equals(List.of(SortBuilders.fieldSort(DOC_FIELD_NAME)));
return sorts.equals(List.of(SortBuilders.fieldSort(SORT_FIELD_SHARD_DOC)));
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ public class OpenSearchIndex extends AbstractOpenSearchTable {

public static final String METADATA_FIELD_ROUTING = "_routing";

// _SHARD_DOC introduced since 3.3.0, and it is not a new metadate field.
// So we cannot read it directly, it can be used in sorting only.
public static final String SORT_FIELD_SHARD_DOC = "_shard_doc";

public static final java.util.Map<String, ExprType> METADATAFIELD_TYPE_MAP =
new LinkedHashMap<>() {
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.*;
import static org.opensearch.index.query.QueryBuilders.*;
import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME;
import static org.opensearch.search.sort.SortOrder.ASC;
import static org.opensearch.sql.data.type.ExprCoreType.INTEGER;
import static org.opensearch.sql.data.type.ExprCoreType.STRING;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.METADATA_FIELD_ID;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -225,7 +224,7 @@ void test_push_down_query() {
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.query(query)
.sort(DOC_FIELD_NAME, ASC),
.sort(SORT_FIELD_SHARD_DOC, ASC),
searchRequest.source());
return mock();
};
Expand Down Expand Up @@ -296,7 +295,7 @@ void test_push_down_query_and_sort() {
void test_push_down_query_not_null() {
SearchSourceBuilder sourceBuilder = requestBuilder.getSourceBuilder();
sourceBuilder.query(QueryBuilders.termQuery("name", "John"));
sourceBuilder.sort(DOC_FIELD_NAME, SortOrder.ASC);
sourceBuilder.sort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);

QueryBuilder query = QueryBuilders.termQuery("intA", 1);
requestBuilder.pushDownFilter(query);
Expand All @@ -310,7 +309,7 @@ void test_push_down_query_not_null() {
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.query(expectedQuery)
.sort(DOC_FIELD_NAME, SortOrder.ASC);
.sort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);

assertSearchSourceBuilder(expectedSourceBuilder, requestBuilder);
}
Expand All @@ -332,7 +331,7 @@ void test_push_down_query_with_bool_filter() {
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.query(initialBoolQuery)
.sort(DOC_FIELD_NAME, SortOrder.ASC);
.sort(SORT_FIELD_SHARD_DOC, SortOrder.ASC);

assertSearchSourceBuilder(expectedSourceBuilder, requestBuilder);
}
Expand Down Expand Up @@ -412,8 +411,7 @@ void test_push_down_project() {
.from(DEFAULT_OFFSET)
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.sort(DOC_FIELD_NAME, ASC)
.sort(METADATA_FIELD_ID, ASC)
.sort(SORT_FIELD_SHARD_DOC, ASC)
.pointInTimeBuilder(new PointInTimeBuilder("samplePITId"))
.fetchSource(new String[] {"intA"}, new String[0]),
requestBuilder);
Expand All @@ -425,8 +423,7 @@ void test_push_down_project() {
.from(DEFAULT_OFFSET)
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.sort(DOC_FIELD_NAME, ASC)
.sort(METADATA_FIELD_ID, ASC)
.sort(SORT_FIELD_SHARD_DOC, ASC)
.pointInTimeBuilder(new PointInTimeBuilder("samplePITId"))
.fetchSource("intA", null),
exprValueFactory,
Expand Down Expand Up @@ -540,8 +537,7 @@ void test_push_down_project_with_alias_type() {
.from(DEFAULT_OFFSET)
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.sort(DOC_FIELD_NAME, ASC)
.sort(METADATA_FIELD_ID, ASC)
.sort(SORT_FIELD_SHARD_DOC)
.pointInTimeBuilder(new PointInTimeBuilder("samplePITId"))
.fetchSource(new String[] {"intA"}, new String[0]),
requestBuilder);
Expand All @@ -553,8 +549,7 @@ void test_push_down_project_with_alias_type() {
.from(DEFAULT_OFFSET)
.size(MAX_RESULT_WINDOW)
.timeout(DEFAULT_QUERY_TIMEOUT)
.sort(DOC_FIELD_NAME, ASC)
.sort(METADATA_FIELD_ID, ASC)
.sort(SORT_FIELD_SHARD_DOC, ASC)
.pointInTimeBuilder(new PointInTimeBuilder("samplePITId"))
.fetchSource("intA", null),
exprValueFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.*;
import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME;
import static org.opensearch.search.sort.SortOrder.ASC;
import static org.opensearch.sql.data.type.ExprCoreType.STRING;
import static org.opensearch.sql.opensearch.storage.OpenSearchIndex.SORT_FIELD_SHARD_DOC;

import java.io.*;
import java.util.Arrays;
Expand Down Expand Up @@ -397,7 +397,7 @@ PushDownAssertion shouldQueryHighlight(QueryBuilder query, HighlightBuilder high
.query(query)
.size(MAX_RESULT_WINDOW)
.highlighter(highlight)
.sort(DOC_FIELD_NAME, ASC);
.sort(SORT_FIELD_SHARD_DOC, ASC);
OpenSearchRequest request =
new OpenSearchQueryRequest(
EMPLOYEES_INDEX, sourceBuilder, factory, List.of(), CURSOR_KEEP_ALIVE, null);
Expand All @@ -417,7 +417,7 @@ PushDownAssertion shouldQuery(QueryBuilder expected) {
.query(expected)
.size(MAX_RESULT_WINDOW)
.timeout(CURSOR_KEEP_ALIVE)
.sort(DOC_FIELD_NAME, ASC);
.sort(SORT_FIELD_SHARD_DOC, ASC);
OpenSearchRequest request =
new OpenSearchQueryRequest(
EMPLOYEES_INDEX, builder, factory, List.of(), CURSOR_KEEP_ALIVE, null);
Expand Down
Loading