Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update MacOS Version for ODBC Driver #987

Merged
merged 2 commits into from
Oct 28, 2022

Conversation

forestmvey
Copy link
Collaborator

Description

Updating MacOS from 10.15 to 12 poses compilation issues for libraries aws-sdk-cpp and googletest. A better solution than building from source is to manage dependencies through a package manager like vcpkg. Libraries rapidjson and rabbit I could not eliminate building from source completely. Windows failed to compile with VS 2022, and rabbit is not included in vcpkg.

Compilation Error Fixes -

VS 2022:

  • Addition of <algorith> includes
error C2039: 'any_of': is not a member of 'std'
  • Changes from std::string to Aws::String
error C2440: 'initializing': cannot convert from 'A
ws::String' to 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>'

XCode 13.3:

  • Removal of basic_array(const basic_array& other) sql-odbc/libraries/rabbit/include/rabbit.hpp
src/../libraries/rabbit/include/rabbit.hpp:1153:3: error: definition of implicit copy assignment operator for 'basic_array<rabbit::details::value_ref_traits<rapidjson::UTF8<>>>' is deprecated because it has a user-provided copy constructor [-Werror,-Wdeprecated-copy-with-user-provided-copy]
  basic_array(const basic_array& other)
  • Removal of bindpara_msg_to_utf8 function as it is never used, contains unused variables, and has potential for exploit when executing malloc(strlen(buff)) with a non-null terminated buffer.

  • Multiple deletions for un-used variables made throughout.

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • 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.

forestmvey and others added 2 commits October 28, 2022 08:59
…s to be included through vcpkg. Fixed compilation errors with newer compilers on libraries required to build from source.

Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey forestmvey requested a review from a team as a code owner October 28, 2022 21:13
Copy link
Collaborator

@MaxKsyunz MaxKsyunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@penghuo penghuo merged commit 99cebe3 into opensearch-project:2.x Oct 28, 2022
@dai-chen dai-chen added the ODBC label Oct 28, 2022
@Yury-Fridlyand
Copy link
Collaborator

@penghuo, @dai-chen,
Do you want to backport this to fix CI on 2.4, 1.x and main?

@dai-chen
Copy link
Collaborator

dai-chen commented Nov 4, 2022

@penghuo, @dai-chen, Do you want to backport this to fix CI on 2.4, 1.x and main?

Thanks for reminding! Just confirmed 2.4 has this commit. Because we may backport all changes in 2.x to main together, probably we only need to backport to 1.x now?

@Yury-Fridlyand
Copy link
Collaborator

I detected ODBC CI failed on main recently: https://github.com/opensearch-project/sql/actions/runs/3396132013
I also don't see this fix on main: https://github.com/opensearch-project/sql/commits/main
Please, double check me and label this PR if it looks good for you. I will backport manually if bot fail.

@dai-chen
Copy link
Collaborator

dai-chen commented Nov 4, 2022

I detected ODBC CI failed on main recently: https://github.com/opensearch-project/sql/actions/runs/3396132013 I also don't see this fix on main: https://github.com/opensearch-project/sql/commits/main Please, double check me and label this PR if it looks good for you. I will backport manually if bot fail.

Yes, it is not in main. I meant we will need to merge all changes to main together soon. Not only this PR. So probably we just need to back port to 1.x for now?

@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.x failed:

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

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.x 1.x
# Navigate to the new working tree
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-987-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 99cebe349710c6e2f3fecef8ddd1d9be488029c0
# Push it to GitHub
git push --set-upstream origin backport/backport-987-to-1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.x

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

@Yury-Fridlyand
Copy link
Collaborator

Ok I'll backport manually.

Yury-Fridlyand pushed a commit to Bit-Quill/opensearch-project-sql that referenced this pull request Nov 4, 2022
…s to be included through vcpkg. Fixed compilation errors with newer compilers on libraries required to build from source. (opensearch-project#987)

opensearch-project#987

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit 99cebe3)
Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Yury-Fridlyand added a commit that referenced this pull request Nov 9, 2022
[Backport 1.x] Update MacOS Version for ODBC Driver #987
YANG-DB added a commit to YANG-DB/sql-odbc that referenced this pull request Dec 14, 2022
This cherry-picking if intended to sync the ODBC new repo to the existing 2.X state of the unified SQL repo

see :: Update MacOS Version for ODBC Driver #987

Origin PR:  opensearch-project/sql#987
Done by:   https://github.com/forestmvey

Signed-off-by: YANGDB <yang.db.dev@gmail.com>
YANG-DB added a commit to YANG-DB/sql-odbc that referenced this pull request Dec 14, 2022
This cherry-picking if intended to sync the ODBC new repo to the existing 2.X state of the unified SQL repo

see :: Update MacOS Version for ODBC Driver #987

Origin PR:  opensearch-project/sql#987
Done by:   https://github.com/forestmvey

Signed-off-by: YANGDB <yang.db.dev@gmail.com>
YANG-DB added a commit to opensearch-project/sql-odbc that referenced this pull request Dec 22, 2022
* As part of SQL repo split - ODBC was missing the 2.X branch
This cherry-picking if intended to sync the ODBC new repo to the existing 2.X state of the unified SQL repo

see :: Update MacOS Version for ODBC Driver #987

Origin PR:  opensearch-project/sql#987
Done by:   https://github.com/forestmvey

Signed-off-by: YANGDB <yang.db.dev@gmail.com>

* As part of SQL repo split - ODBC was missing the 2.X branch
This cherry-picking if intended to sync the ODBC new repo to the existing 2.X state of the unified SQL repo

see :: Update MacOS Version for ODBC Driver #987

Origin PR:  opensearch-project/sql#987
Done by:   https://github.com/forestmvey

Signed-off-by: YANGDB <yang.db.dev@gmail.com>

* Adding missing VisualLeakDetector dependencies to resolve windows ODBC compilation.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

Signed-off-by: YANGDB <yang.db.dev@gmail.com>
Signed-off-by: forestmvey <forestv@bitquilltech.com>
Co-authored-by: forestmvey <forestv@bitquilltech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants