Fix @timestamp upgrade issue by adding a version check on skip_list param (3.3)

[asimmahmood1]

Description

Date field's skip_list param was introduced in 3.3.0, so any index created since 3.3.0 with @timestamp name will have skip_list set to true. This way older indexes with @timestamp or index sort date field will continue to remain false.

This way new indexes can take advantage of the performance enhancement in date histogram aggregation.

This is proper fix compared to #19661

Testing

Manually tested appending to big5 index created using 3.2.0.

Looking at how to add a bwc integ test. Existing ones do not have date field.

Note: this is pushing to 3.3 version to help expedite patch release. Once merged I'll open one for mainline.

Related Issues

Fixes #19660

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[sandeshkr419]

Thanks @asimmahmood1 for working on the fix quickly!

Also, let us open a new issue separately to track what bwc compatibility test we can add as a follow-up to prevent these bugs!

---

3.3 builds are failing for now dependent on #19668 - gradle should succeed after it.

[github-actions]

❌ Gradle check result for 2d33d05: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for 429682f: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.98%. Comparing base (06d465f) to head (429682f).
⚠️ Report is 9 commits behind head on 3.3.

Files with missing lines	Patch %	Lines
...a/org/opensearch/index/mapper/DateFieldMapper.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                3.3   #19671      +/-   ##
============================================
- Coverage     72.99%   72.98%   -0.01%     
+ Complexity    70482    70477       -5     
============================================
  Files          5717     5717              
  Lines        323055   323054       -1     
  Branches      46794    46791       -3     
============================================
- Hits         235798   235784      -14     
- Misses        68237    68273      +36     
+ Partials      19020    18997      -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sandeshkr419]

Thanks @asimmahmood1 for quickly fixing this.

@jainankitk Nice call out on keeping the feature (instead of reverting it) with version check. Good to merge?

[jainankitk]

LGTM! Let us test it thoroughly and see if automated bwc test can be added

[asimmahmood1]

Thanks. Yes, still looking to add/update bwc tests. Mark pointed me to https://github.com/opensearch-project/OpenSearch/blob/main/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java and https://github.com/opensearch-project/OpenSearch/blob/main/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/IndexingIT.java. These only have a string keyword, checking to see if I can date field in it.