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

Feature/singlestore integration tests #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Rodriguespn
Copy link
Collaborator

@Rodriguespn Rodriguespn commented Aug 30, 2024

Integration Tests

Copied and renamed every mysql reference to singlestore on integration-tests directory

Key differences between MySQL vs SIngleStore IT Tests

  • Renamed all references from MySQL to SingleStore in the integration tests directory.
  • The tests are performed against the singlestore-dev docker image, to replicate a production environment
  • SingleStore's serial column type only assures uniqueness of the column values. The tests were modified to (set operation tests in particular) to expect SingleStore-specific behavior, by adding an ORDER BY instruction to every select instruction.
  • ORDER BY and LIMIT cannot be chained together.
  • Removed SingleStore proxy package and its IT tests. If it's necessary, this package can be replicated from the MySQLL proxy package later
  • The foreign key integration tests were removed since they are not supported by singlestore
  • Removed intersectAll and exceptAll tests since these operations are not supported by SIngleStore
  • Removed nested transaction tests since this feature is not supported by SingleStore
  • Removed isolationLevel config and tests as singlestore only supports one isolationLevel

Side notes

There are some SingleStore-specific operations, such as attach/detach, create/drop milestone and optimyzeTable for column stores tables, that are implemented on drizzle-orm/src/singlestore-core but not on drizzle-kit. There are also no integration tests for these operations. This is a known issue and should be fixed in the upcoming releases.

@Rodriguespn Rodriguespn added the enhancement New feature or request label Aug 30, 2024
@Rodriguespn Rodriguespn force-pushed the feature/singlestore-integration-tests branch from 9ea059b to e79e6e9 Compare August 30, 2024 09:40
@Rodriguespn Rodriguespn force-pushed the feature/singlestore-integration-tests branch from 5a8ad0e to fced943 Compare August 30, 2024 11:55
@Rodriguespn Rodriguespn marked this pull request as ready for review September 6, 2024 16:35
@Rodriguespn Rodriguespn force-pushed the feature/singlestore-integration-tests branch 3 times, most recently from b1b7672 to 1f47600 Compare September 6, 2024 17:12
@Rodriguespn Rodriguespn force-pushed the feature/singlestore-integration-tests branch 2 times, most recently from 14712f6 to a5f5f71 Compare September 6, 2024 17:17
@Rodriguespn
Copy link
Collaborator Author

@apeng-singlestore @mitchwadair @tiagoacastro @drodrigues4 is ready to be reviewed

@mitchwadair
Copy link
Collaborator

Cool, I'll take a look soon 👍

@apeng-singlestore
Copy link
Collaborator

Notable Changes

  • Renamed all references from MySQL to SingleStore in the integration tests directory.
  • Modified timestamp precision and other minor test adjustments
  • Updated the test environment to use the SingleStore dev Docker image.

Known Issues:

  • SingleStore does not order by by default, so some test expectations (set operation tests in particular) were modified to expect SingleStore-specific behavior
  • ORDER BY and LIMIT cannot be chained together.
  • INTERSECT ALL, EXCEPT ALL, and nested transaction operations are not supported and associated tests were removed
  • Only one isolationLevel is supported, so associated tests and config options were removed

@Rodriguespn
Copy link
Collaborator Author

Thank you @apeng-singlestore. I updated the PR's description with your notable changes/known issues

Copy link
Collaborator

@mitchwadair mitchwadair left a comment

Choose a reason for hiding this comment

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

Generally LGTM, just a few comments. Very exciting!

@mitchwadair
Copy link
Collaborator

I also updated the main PR description with @apeng-singlestore's notes

"name"
],
"isUnique": true,
"using": "btree"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think this should be here after removing using method

@Rodriguespn Rodriguespn force-pushed the feature/singlestore-integration-tests branch 2 times, most recently from e4c786a to 9f50061 Compare September 20, 2024 15:33
@Rodriguespn Rodriguespn force-pushed the feature/singlestore-integration-tests branch from 9f50061 to bab7f4a Compare September 20, 2024 15:33
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.

3 participants