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

Add Support for oceanbase tpch #670

Merged
merged 2 commits into from
Feb 27, 2024
Merged

Conversation

yaqi-zhao
Copy link

As described in #563. This PR support Oceanbase TPROC-H by adding a compatible parameter.

image

Co-authored-by: XiTang xi.tang@intel.com

TPROC-H
#Check 1 Database Exist
#Check 2 Tables Exist
#Check 3 supplier count in schema is the same as dict setting
#Check 4 Tables are indexed
#Check 5 Tables are populated
#Consistency check

Co-authored-by: XiTang <xi.tang@intel.com>
@yaqi-zhao yaqi-zhao requested a review from a team as a code owner February 19, 2024 06:50
@sm-shaw sm-shaw added the enhancement New feature or request label Feb 19, 2024
@sm-shaw sm-shaw linked an issue Feb 19, 2024 that may be closed by this pull request
@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 19, 2024

Many thanks for the pull request.

I will test to make sure everything is OK with the existing MySQL before approving.

From a quick overview there is a few minor changes to make and I will look to add those to the PR, such as keeping the scale factor and virtual users to build schema at the bottom of the options and also keeping all of the mysql dict options starting with mysql_.

We are currently just about to release v4.10 which has already been tagged and drafted and therefore once merged this is likely to be included with v4.11.

Copy link
Contributor

@sm-shaw sm-shaw left a comment

Choose a reason for hiding this comment

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

I've tested and added an additional commit to make some changes, what I have changed is:

  1. Updated some of the variable names for consistency with mysql prefix.
  2. It looks the partitions option is only needed in the build script and not the driver i.e. the variable wasn't used in the driver. So I've removed the partitions option from the driver and left it in the build.
  3. I've moved the fields around for consistency across HammerDB.
  4. The changes to ConnectToMySQL always set the user to oceanbase if the local socket was not used when logging in. I've fixed this so if not using oceanbase and are using a port to connect it still works.
  5. The transaction counter was changed to work against oceanbase, however the GUI dialog did then not allow an oceanbase option for the transaction counter - I've fixed this by adding options to the transaction counter.
  6. Cloud Analytic queries errored as they didn't include Oceanbase variables in the EDITABLE OPTIONS - this has been fixed.

I have tested against regular MySQL only to make sure all the TPC-H options work when you don't select the oceanbase options, so am happy to approve on this basis.

Please check to make sure that everything continues to work on the oceanbase side after the updates.

@xtangxtang
Copy link
Contributor

I've tested and added an additional commit to make some changes, what I have changed is:

  1. Updated some of the variable names for consistency with mysql prefix.
  2. It looks the partitions option is only needed in the build script and not the driver i.e. the variable wasn't used in the driver. So I've removed the partitions option from the driver and left it in the build.
  3. I've moved the fields around for consistency across HammerDB.
  4. The changes to ConnectToMySQL always set the user to oceanbase if the local socket was not used when logging in. I've fixed this so if not using oceanbase and are using a port to connect it still works.
  5. The transaction counter was changed to work against oceanbase, however the GUI dialog did then not allow an oceanbase option for the transaction counter - I've fixed this by adding options to the transaction counter.
  6. Cloud Analytic queries errored as they didn't include Oceanbase variables in the EDITABLE OPTIONS - this has been fixed.

I have tested against regular MySQL only to make sure all the TPC-H options work when you don't select the oceanbase options, so am happy to approve on this basis.

Please check to make sure that everything continues to work on the oceanbase side after the updates.

Thanks Steve, we will check if the oceanbase is corret. Any process we will update here.

@yaqi-zhao yaqi-zhao closed this Feb 27, 2024
@yaqi-zhao yaqi-zhao reopened this Feb 27, 2024
@yaqi-zhao
Copy link
Author

yaqi-zhao commented Feb 27, 2024

I've tested and added an additional commit to make some changes, what I have changed is:

  1. Updated some of the variable names for consistency with mysql prefix.
  2. It looks the partitions option is only needed in the build script and not the driver i.e. the variable wasn't used in the driver. So I've removed the partitions option from the driver and left it in the build.
  3. I've moved the fields around for consistency across HammerDB.
  4. The changes to ConnectToMySQL always set the user to oceanbase if the local socket was not used when logging in. I've fixed this so if not using oceanbase and are using a port to connect it still works.
  5. The transaction counter was changed to work against oceanbase, however the GUI dialog did then not allow an oceanbase option for the transaction counter - I've fixed this by adding options to the transaction counter.
  6. Cloud Analytic queries errored as they didn't include Oceanbase variables in the EDITABLE OPTIONS - this has been fixed.

I have tested against regular MySQL only to make sure all the TPC-H options work when you don't select the oceanbase options, so am happy to approve on this basis.

Please check to make sure that everything continues to work on the oceanbase side after the updates.

Thanks Steve, we have tested the oceanbase with the newest updates, everything continues to work fine.

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 27, 2024

Excellent, once fully approved by all reviewers we will merge the pull request

@abondvt89 abondvt89 merged commit c78d9f4 into TPC-Council:master Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unmatched Background Error - can't read "myoptsfields": no such variable Hope to support oceanbase databases
5 participants