sql icon indicating copy to clipboard operation
sql copied to clipboard

Support common format geo point

Open bugmakerrrrrr opened this issue 1 year ago • 2 comments

Description

Currently, we only support geo point in the format of an object with a latitude and longitude, this PR aims to expand our support to all common formats that are described in OpenSearch doc.

Issues Resolved

#1432

Check List

  • [x] New functionality includes testing.
    • [x] All tests pass, including unit test, integration test and doctest
  • [x] New functionality has been documented.
    • [x] New functionality has javadoc added
    • [x] New functionality has user manual doc added
  • [x] Commits are signed per the DCO using --signoff

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.

bugmakerrrrrr avatar Jul 03 '24 14:07 bugmakerrrrrr

@dai-chen you might be interested in this :) Could you please take a look when you get a chance?

bugmakerrrrrr avatar Jul 03 '24 14:07 bugmakerrrrrr

The build failures contains low code test coverage: Execution failed for task ':opensearch:jacocoTestCoverageVerification'. Please run ./gradlew build in local and check the jacoco report file.

LantaoJin avatar Jul 10 '24 00:07 LantaoJin

Seems the workflows were not triggered correctly. @bugmakerrrrrr Could u merge latest change from main and re-run test? (try to avoid force-push)

LantaoJin avatar Jul 16 '24 14:07 LantaoJin

Seems the workflows were not triggered correctly. @bugmakerrrrrr Could u merge latest change from main and re-run test? (try to avoid force-push)

After rebasing the main branch, it appears that I have to perform a force push.

bugmakerrrrrr avatar Jul 16 '24 16:07 bugmakerrrrrr

You can git merge latest upstream/main to your local dev branch then push to your origin/[dev branch].

LantaoJin avatar Jul 16 '24 16:07 LantaoJin

Unit tests passed, fundamentally LGTM, @bugmakerrrrrr could you add a new IT similar to integ-test/src/test/java/org/opensearch/sql/sql/DateTimeFormatsIT.java?

Now only one geo_point format case used in IT, ref https://github.com/opensearch-project/sql/blob/2c29a1a592f53be96bcab180f568a55f313a4c93/integ-test/src/test/resources/datatypes.json#L2, you can add more rows in this file or add a new json file.

LantaoJin avatar Jul 17 '24 07:07 LantaoJin

@LantaoJin @dai-chen can we merge this?

bugmakerrrrrr avatar Jul 30 '24 08:07 bugmakerrrrrr

@penghuo do you have a chance to review this?

LantaoJin avatar Jul 31 '24 03:07 LantaoJin

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-2801-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 82ef68e2b25c7c10740e74968bbe960c000c1cee
# Push it to GitHub
git push --set-upstream origin backport/backport-2801-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-2801-to-2.x.

@bugmakerrrrrr could u help resolve conflict and submit backport PR to 2.x. https://github.com/opensearch-project/sql/pull/2801#issuecomment-2263318211

penghuo avatar Aug 01 '24 15:08 penghuo

@bugmakerrrrrr could u help resolve conflict and submit backport PR to 2.x. #2801 (comment)

#2896

bugmakerrrrrr avatar Aug 05 '24 07:08 bugmakerrrrrr