Add missing refactoring of Scroll to PIT API calls for Joins and Pagination query

[manasvinibs]

Description

• Adds new point in time API class for join queries request while doing table scan in legacy code - https://github.com/opensearch-project/sql/pull/2981/files#diff-445722c06193dfbd4b0c26d95df7961120b7769ef008a3582014c6d7ba494da1
• Adds missing check for Pagination flag in Table scan operator - https://github.com/opensearch-project/sql/pull/2981/files#diff-039759a62de98418d87120c314e887f5fab1f5758be64cd09c8c5c4c6f884c0d
• Refactors new sql engine code to add PIT support while building OpenSearchQueryRequest - https://github.com/opensearch-project/sql/pull/2981/files#diff-96f998c99595de19404d0dd5c38b71b5fb7c52160cf34a847c2785dbb05f1d03
• Adds PIT flag check in places where scroll request was used default - https://github.com/opensearch-project/sql/pull/2981/files#diff-e4612a7ad8f1be8d78c421b592dfbf6a65dbcf11f00b858b8c41b8055b18dbb5
  ** https://github.com/opensearch-project/sql/pull/2981/files#diff-c1e49e7f69bbd6e7969071effcb3972827ed02ed86f552c03a710f548820fe3a

To-Do:

• Currently task :opensearch:jacocoTestCoverageVerification is failing. Will add more tests to increase coverage

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

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.

[dai-chen]

I forgot whether this class is the main IT for pagination? Is the new PIT logic enabled to test now?

[manasvinibs]

This class is only responsible for legacy/v1 engine pagination queries. For v2 engine queries, there is a different base class. Currently, integ-test cluster does not set the feature flag settings and by default the cluster settings will have PIT feature flag enabled/set to true. Hence there are around 80+ tests testing new PIT changes.

[dai-chen]

Not urgent but I think it would be helpful to get the IT run with PIT and Scroll both. In case we need to rollback to Scroll if anything wrong.

[manasvinibs]

Actually we are not disabling/removing Scroll implementation. There is a cluster level settings called SQL_PAGINATION_API_SEARCH_AFTER which is set to true to use PIT or false to default to Scroll API. So current IT tests run based on the setting of the above key and it covers both Scroll and PIT.

[dai-chen]

Do you mean the same IT set run 1 pass for PIT and another pass for Scroll?

[manasvinibs]

oh you meant running 2 passes of same set of tests for both PIT and Scroll? Ok I think that can be done by modifying the integ-test cluster settings as required. But currently we do not explicitly set the API settings in the test-cluster, so by default PIT changes are covered as part of IT tests and only 1 pass is run. I'll add this as part of the changes here - #2945

[dai-chen]

Add a new PIT request class instead? I think all the conditional check added make this class hard to maintain.

[manasvinibs]

I've captured this improvement as part of this issue -#2945

[dai-chen]

This method seems a copy of the original buildRequestWithScroll? I think the only difference is at 119 and 134 which decides to create a Scroll request or PIT request (if you added a new class)

[manasvinibs]

I've an issue created related to the improvements in branching strategy we have today for PIT and Scroll and its related implementation. This and the above comment can be addressed as part of this task: #2945
Let me know if you think this is a blocking comment. If not I would like to get this addressed as part of the other task to reduce duplicate effort.

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.x
# Create a new branch
git switch --create backport/backport-2981-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ce17d0a385d2d231f405fb8fcd8c27cf046cf2ac
# Push it to GitHub
git push --set-upstream origin backport/backport-2981-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-2981-to-2.x.

[opensearch-trigger-bot]

The backport to 2.18 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.18 2.18
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.18
# Create a new branch
git switch --create backport/backport-2981-to-2.18
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ce17d0a385d2d231f405fb8fcd8c27cf046cf2ac
# Push it to GitHub
git push --set-upstream origin backport/backport-2981-to-2.18
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.18

Then, create a pull request where the base branch is 2.18 and the compare/head branch is backport/backport-2981-to-2.18.