Merge remote-tracking branch 'upstream/main' into add-cluster-manager-timeout

Signed-off-by: Tianli Feng <ftianli@amazon.com>

Author: Tianli Feng

plugins/repository-azure/build.gradle

@@ -67,9 +67,9 @@ dependencies {
   api "com.fasterxml.jackson.dataformat:jackson-dataformat-xml:${versions.jackson}"
   api "com.fasterxml.jackson.module:jackson-module-jaxb-annotations:${versions.jackson}"
   api 'org.codehaus.woodstox:stax2-api:4.2.1'
-  implementation 'com.fasterxml.woodstox:woodstox-core:6.1.1'
+  implementation 'com.fasterxml.woodstox:woodstox-core:6.2.8'
   runtimeOnly 'com.google.guava:guava:31.1-jre'
-  api 'org.apache.commons:commons-lang3:3.4'
+  api 'org.apache.commons:commons-lang3:3.12.0'
   testImplementation project(':test:fixtures:azure-fixture')
 }
 

plugins/repository-azure/licenses/commons-lang3-3.12.0.jar.sha1

@@ -0,0 +1 @@
+c6842c86792ff03b9f1d1fe2aab8dc23aa6c6f0e

plugins/repository-azure/licenses/commons-lang3-3.4.jar.sha1

@@ -0,0 +1 @@
+670748292899c53b1963730d9eb7f8ab71314e90

plugins/repository-azure/licenses/woodstox-core-6.1.1.jar.sha1

@@ -62,6 +62,7 @@
 
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.opensearch.index.query.QueryBuilders.queryStringQuery;
+import static org.opensearch.index.query.QueryBuilders.rangeQuery;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertNoFailures;
 import static org.hamcrest.Matchers.allOf;
@@ -500,4 +501,100 @@ public void testExplainTermsQueryWithLookup() throws Exception {
             .actionGet();
         assertThat(response.isValid(), is(true));
     }
+
+    // Issue: https://github.com/opensearch-project/OpenSearch/issues/2036
+    public void testValidateDateRangeInQueryString() throws IOException {
+        assertAcked(prepareCreate("test").setSettings(Settings.builder().put(indexSettings()).put("index.number_of_shards", 1)));
+
+        assertAcked(
+            client().admin()
+                .indices()
+                .preparePutMapping("test")
+                .setSource(
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject(MapperService.SINGLE_MAPPING_NAME)
+                        .startObject("properties")
+                        .startObject("name")
+                        .field("type", "keyword")
+                        .endObject()
+                        .startObject("timestamp")
+                        .field("type", "date")
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                )
+        );
+
+        client().prepareIndex("test").setId("1").setSource("name", "username", "timestamp", 200).get();
+        refresh();
+
+        ValidateQueryResponse response = client().admin()
+            .indices()
+            .prepareValidateQuery()
+            .setQuery(
+                QueryBuilders.boolQuery()
+                    .must(rangeQuery("timestamp").gte(0).lte(100))
+                    .must(queryStringQuery("username").allowLeadingWildcard(false))
+            )
+            .setRewrite(true)
+            .get();
+
+        assertNoFailures(response);
+        assertThat(response.isValid(), is(true));
+
+        // Use wildcard and date outside the range
+        response = client().admin()
+            .indices()
+            .prepareValidateQuery()
+            .setQuery(
+                QueryBuilders.boolQuery()
+                    .must(rangeQuery("timestamp").gte(0).lte(100))
+                    .must(queryStringQuery("*erna*").allowLeadingWildcard(false))
+            )
+            .setRewrite(true)
+            .get();
+
+        assertNoFailures(response);
+        assertThat(response.isValid(), is(false));
+
+        // Use wildcard and date inside the range
+        response = client().admin()
+            .indices()
+            .prepareValidateQuery()
+            .setQuery(
+                QueryBuilders.boolQuery()
+                    .must(rangeQuery("timestamp").gte(0).lte(1000))
+                    .must(queryStringQuery("*erna*").allowLeadingWildcard(false))
+            )
+            .setRewrite(true)
+            .get();
+
+        assertNoFailures(response);
+        assertThat(response.isValid(), is(false));
+
+        // Use wildcard and date inside the range (allow leading wildcard)
+        response = client().admin()
+            .indices()
+            .prepareValidateQuery()
+            .setQuery(QueryBuilders.boolQuery().must(rangeQuery("timestamp").gte(0).lte(1000)).must(queryStringQuery("*erna*")))
+            .setRewrite(true)
+            .get();
+
+        assertNoFailures(response);
+        assertThat(response.isValid(), is(true));
+
+        // Use invalid date range
+        response = client().admin()
+            .indices()
+            .prepareValidateQuery()
+            .setQuery(QueryBuilders.boolQuery().must(rangeQuery("timestamp").gte("aaa").lte(100)))
+            .setRewrite(true)
+            .get();
+
+        assertNoFailures(response);
+        assertThat(response.isValid(), is(false));
+
+    }
 }

plugins/repository-azure/licenses/woodstox-core-6.2.8.jar.sha1

@@ -131,7 +131,7 @@ protected void doExecute(Task task, ValidateQueryRequest request, ActionListener
         if (request.query() == null) {
             rewriteListener.onResponse(request.query());
         } else {
-            Rewriteable.rewriteAndFetch(request.query(), searchService.getRewriteContext(timeProvider), rewriteListener);
+            Rewriteable.rewriteAndFetch(request.query(), searchService.getValidationRewriteContext(timeProvider), rewriteListener);
         }
     }
 
@@ -225,7 +225,7 @@ protected ShardValidateQueryResponse shardOperation(ShardValidateQueryRequest re
             request.nowInMillis(),
             request.filteringAliases()
         );
-        SearchContext searchContext = searchService.createSearchContext(shardSearchLocalRequest, SearchService.NO_TIMEOUT);
+        SearchContext searchContext = searchService.createValidationContext(shardSearchLocalRequest, SearchService.NO_TIMEOUT);
         try {
             ParsedQuery parsedQuery = searchContext.getQueryShardContext().toQuery(request.query());
             searchContext.parsedQuery(parsedQuery);

server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java

@@ -630,6 +630,22 @@ public IndexSettings getIndexSettings() {
      * {@link IndexReader}-specific optimizations, such as rewriting containing range queries.
      */
     public QueryShardContext newQueryShardContext(int shardId, IndexSearcher searcher, LongSupplier nowInMillis, String clusterAlias) {
+        return newQueryShardContext(shardId, searcher, nowInMillis, clusterAlias, false);
+    }
+
+    /**
+     * Creates a new QueryShardContext.
+     *
+     * Passing a {@code null} {@link IndexSearcher} will return a valid context, however it won't be able to make
+     * {@link IndexReader}-specific optimizations, such as rewriting containing range queries.
+     */
+    public QueryShardContext newQueryShardContext(
+        int shardId,
+        IndexSearcher searcher,
+        LongSupplier nowInMillis,
+        String clusterAlias,
+        boolean validate
+    ) {
         final SearchIndexNameMatcher indexNameMatcher = new SearchIndexNameMatcher(
             index().getName(),
             clusterAlias,
@@ -653,7 +669,8 @@ public QueryShardContext newQueryShardContext(int shardId, IndexSearcher searche
             clusterAlias,
             indexNameMatcher,
             allowExpensiveQueries,
-            valuesSourceRegistry
+            valuesSourceRegistry,
+            validate
         );
     }
 

server/src/main/java/org/opensearch/action/admin/indices/validate/query/TransportValidateQueryAction.java

@@ -72,7 +72,6 @@
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.util.concurrent.ReleasableLock;
 import org.opensearch.index.VersionType;
-import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.mapper.Mapping;
 import org.opensearch.index.mapper.ParseContext.Document;
 import org.opensearch.index.mapper.ParsedDocument;
@@ -736,13 +735,8 @@ public enum SearcherScope {
      * Creates a new history snapshot from Lucene for reading operations whose seqno in the requesting seqno range (both inclusive).
      * This feature requires soft-deletes enabled. If soft-deletes are disabled, this method will throw an {@link IllegalStateException}.
      */
-    public abstract Translog.Snapshot newChangesSnapshot(
-        String source,
-        MapperService mapperService,
-        long fromSeqNo,
-        long toSeqNo,
-        boolean requiredFullRange
-    ) throws IOException;
+    public abstract Translog.Snapshot newChangesSnapshot(String source, long fromSeqNo, long toSeqNo, boolean requiredFullRange)
+        throws IOException;
 
     public abstract boolean hasCompleteOperationHistory(String reason, long startingSeqNo);
 

server/src/main/java/org/opensearch/index/IndexService.java

@@ -91,7 +91,6 @@
 import org.opensearch.index.VersionType;
 import org.opensearch.index.fieldvisitor.IdOnlyFieldVisitor;
 import org.opensearch.index.mapper.IdFieldMapper;
-import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.mapper.ParseContext;
 import org.opensearch.index.mapper.ParsedDocument;
 import org.opensearch.index.mapper.SeqNoFieldMapper;
@@ -2773,20 +2772,13 @@ long getNumDocUpdates() {
     }
 
     @Override
-    public Translog.Snapshot newChangesSnapshot(
-        String source,
-        MapperService mapperService,
-        long fromSeqNo,
-        long toSeqNo,
-        boolean requiredFullRange
-    ) throws IOException {
+    public Translog.Snapshot newChangesSnapshot(String source, long fromSeqNo, long toSeqNo, boolean requiredFullRange) throws IOException {
         ensureOpen();
         refreshIfNeeded(source, toSeqNo);
         Searcher searcher = acquireSearcher(source, SearcherScope.INTERNAL);
         try {
             LuceneChangesSnapshot snapshot = new LuceneChangesSnapshot(
                 searcher,
-                mapperService,
                 LuceneChangesSnapshot.DEFAULT_BATCH_SIZE,
                 fromSeqNo,
                 toSeqNo,

server/src/main/java/org/opensearch/index/engine/Engine.java

@@ -36,7 +36,6 @@
 import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.NumericDocValues;
-import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.DocValuesFieldExistsQuery;
@@ -51,11 +50,8 @@
 import org.opensearch.common.lucene.Lucene;
 import org.opensearch.core.internal.io.IOUtils;
 import org.opensearch.index.fieldvisitor.FieldsVisitor;
-import org.opensearch.index.mapper.IdFieldMapper;
-import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.mapper.SeqNoFieldMapper;
 import org.opensearch.index.mapper.SourceFieldMapper;
-import org.opensearch.index.mapper.Uid;
 import org.opensearch.index.translog.Translog;
 
 import java.io.Closeable;
@@ -77,7 +73,6 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
     private final boolean requiredFullRange;
 
     private final IndexSearcher indexSearcher;
-    private final MapperService mapperService;
     private int docIndex = 0;
     private final int totalHits;
     private ScoreDoc[] scoreDocs;
@@ -88,20 +83,13 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
      * Creates a new "translog" snapshot from Lucene for reading operations whose seq# in the specified range.
      *
      * @param engineSearcher    the internal engine searcher which will be taken over if the snapshot is opened successfully
-     * @param mapperService     the mapper service which will be mainly used to resolve the document's type and uid
      * @param searchBatchSize   the number of documents should be returned by each search
      * @param fromSeqNo         the min requesting seq# - inclusive
      * @param toSeqNo           the maximum requesting seq# - inclusive
      * @param requiredFullRange if true, the snapshot will strictly check for the existence of operations between fromSeqNo and toSeqNo
      */
-    LuceneChangesSnapshot(
-        Engine.Searcher engineSearcher,
-        MapperService mapperService,
-        int searchBatchSize,
-        long fromSeqNo,
-        long toSeqNo,
-        boolean requiredFullRange
-    ) throws IOException {
+    LuceneChangesSnapshot(Engine.Searcher engineSearcher, int searchBatchSize, long fromSeqNo, long toSeqNo, boolean requiredFullRange)
+        throws IOException {
         if (fromSeqNo < 0 || toSeqNo < 0 || fromSeqNo > toSeqNo) {
             throw new IllegalArgumentException("Invalid range; from_seqno [" + fromSeqNo + "], to_seqno [" + toSeqNo + "]");
         }
@@ -114,7 +102,6 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
                 IOUtils.close(engineSearcher);
             }
         };
-        this.mapperService = mapperService;
         final long requestingSize = (toSeqNo - fromSeqNo) == Long.MAX_VALUE ? Long.MAX_VALUE : (toSeqNo - fromSeqNo + 1L);
         this.searchBatchSize = requestingSize < searchBatchSize ? Math.toIntExact(requestingSize) : searchBatchSize;
         this.fromSeqNo = fromSeqNo;
@@ -278,19 +265,17 @@ private Translog.Operation readDocAsOp(int docIndex) throws IOException {
             : SourceFieldMapper.NAME;
         final FieldsVisitor fields = new FieldsVisitor(true, sourceField);
         leaf.reader().document(segmentDocID, fields);
-        fields.postProcess(mapperService);
 
         final Translog.Operation op;
         final boolean isTombstone = parallelArray.isTombStone[docIndex];
-        if (isTombstone && fields.uid() == null) {
+        if (isTombstone && fields.id() == null) {
             op = new Translog.NoOp(seqNo, primaryTerm, fields.source().utf8ToString());
             assert version == 1L : "Noop tombstone should have version 1L; actual version [" + version + "]";
             assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Noop but soft_deletes field is not set [" + op + "]";
         } else {
-            final String id = fields.uid().id();
-            final Term uid = new Term(IdFieldMapper.NAME, Uid.encodeId(id));
+            final String id = fields.id();
             if (isTombstone) {
-                op = new Translog.Delete(id, uid, seqNo, primaryTerm, version);
+                op = new Translog.Delete(id, seqNo, primaryTerm, version);
                 assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Delete op but soft_deletes field is not set [" + op + "]";
             } else {
                 final BytesReference source = fields.source();

server/src/main/java/org/opensearch/index/engine/InternalEngine.java

@@ -46,7 +46,6 @@
 import org.opensearch.common.lucene.index.OpenSearchDirectoryReader;
 import org.opensearch.common.util.concurrent.ReleasableLock;
 import org.opensearch.core.internal.io.IOUtils;
-import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.seqno.SeqNoStats;
 import org.opensearch.index.seqno.SequenceNumbers;
 import org.opensearch.index.store.Store;
@@ -326,13 +325,7 @@ public Closeable acquireHistoryRetentionLock() {
     }
 
     @Override
-    public Translog.Snapshot newChangesSnapshot(
-        String source,
-        MapperService mapperService,
-        long fromSeqNo,
-        long toSeqNo,
-        boolean requiredFullRange
-    ) {
+    public Translog.Snapshot newChangesSnapshot(String source, long fromSeqNo, long toSeqNo, boolean requiredFullRange) {
         return newEmptySnapshot();
     }
 

server/src/main/java/org/opensearch/index/engine/LuceneChangesSnapshot.java

@@ -36,7 +36,6 @@
 import org.apache.lucene.util.BytesRef;
 import org.opensearch.common.bytes.BytesArray;
 import org.opensearch.common.bytes.BytesReference;
-import org.opensearch.index.mapper.DocumentMapper;
 import org.opensearch.index.mapper.IdFieldMapper;
 import org.opensearch.index.mapper.IgnoredFieldMapper;
 import org.opensearch.index.mapper.MappedFieldType;
@@ -67,7 +66,7 @@ public class FieldsVisitor extends StoredFieldVisitor {
     private final String sourceFieldName;
     private final Set<String> requiredFields;
     protected BytesReference source;
-    protected String type, id;
+    protected String id;
     protected Map<String, List<Object>> fieldsValues;
 
     public FieldsVisitor(boolean loadSource) {
@@ -98,10 +97,6 @@ public Status needsField(FieldInfo fieldInfo) {
     }
 
     public void postProcess(MapperService mapperService) {
-        final DocumentMapper mapper = mapperService.documentMapper();
-        if (mapper != null) {
-            type = mapper.type();
-        }
         for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
             MappedFieldType fieldType = mapperService.fieldType(entry.getKey());
             if (fieldType == null) {
@@ -167,13 +162,8 @@ public BytesReference source() {
         return source;
     }
 
-    public Uid uid() {
-        if (id == null) {
-            return null;
-        } else if (type == null) {
-            throw new IllegalStateException("Call postProcess before getting the uid");
-        }
-        return new Uid(type, id);
+    public String id() {
+        return id;
     }
 
     public String routing() {
@@ -195,7 +185,6 @@ public Map<String, List<Object>> fields() {
     public void reset() {
         if (fieldsValues != null) fieldsValues.clear();
         source = null;
-        type = null;
         id = null;
 
         requiredFields.addAll(BASE_REQUIRED_FIELDS);