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: Oracle DB Support #24

Closed

Conversation

stefnedelchevbrady
Copy link
Contributor

@stefnedelchevbrady stefnedelchevbrady commented Sep 12, 2022

  • Added godror adapter
  • Bumped k6 to v0.40.0
  • Ran go mod tidy to consolidate Go packages and include godror packages to go.sum

@CLAassistant
Copy link

CLAassistant commented Sep 12, 2022

CLA assistant check
All committers have signed the CLA.

@imiric imiric self-requested a review September 13, 2022 09:01
@@ -56,7 +57,7 @@ func contains(array []string, element string) bool {
// Open establishes a connection to the specified database type using
// the provided connection string.
func (*SQL) Open(database string, connectionString string) (*dbsql.DB, error) {
supportedDatabases := []string{"mysql", "postgres", "sqlite3", "sqlserver"}
supportedDatabases := []string{"mysql", "postgres", "sqlite3", "sqlserver", "godror"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
supportedDatabases := []string{"mysql", "postgres", "sqlite3", "sqlserver", "godror"}
supportedDatabases := []string{"mysql", "postgres", "sqlite3", "sqlserver", "oracle"}

@imiric
Copy link
Contributor

imiric commented Sep 13, 2022

Hi @stefnedelchev, thanks for the PR!

Unfortunately we can't accept it, as we don't want to expand support for more SQL implementations internally. See #17 for details.

Eventually, we'd like to split apart each currently supported dialect into standalone sub-extensions, so that users who need support for only e.g. PostgreSQL can build a binary that only has these drivers built-in, instead of the current 4. Adding more drivers to this list further complicates this.

Besides, it's fairly trivial for users to build a k6 binary with an extension from their own or any other xk6-sql fork.

Additionally, supporting Oracle DB specifically is a bit challenging, since it requires building with CGO_ENABLED=1 (same as SQLite, so not a big problem), and installing Oracle Client libraries. Oracle DB is also not packaged by most Linux distributions, and there are no official OCI images on standard registries, so Oracle's registry should be used. This should all be documented in the README.

If you want to keep supporting your fork, we'd be happy to list it in our README, so that users who need Oracle support can use it. Let us know if you're OK with this. And please add the above documentation, if so.

@imiric imiric closed this Sep 13, 2022
@stefnedelchevbrady
Copy link
Contributor Author

@imiric Fair enough, I understand. Sure, I can keep my fork and update its documentation. I'll let you know when I introduce these changes so the fork could be referenced.

@stefnedelchevbrady stefnedelchevbrady deleted the feature/oracle-db branch September 13, 2022 10:08
@stefnedelchevbrady
Copy link
Contributor Author

@imiric I updated the readme of my fork, changed the Open method so it accepts "oracle" as database argument instead of "godror" and fixed the pipeline. I also published a new release with a binary build for Windows (also tested if it works).
Please, let me know if I'm missing something https://github.com/stefnedelchev/xk6-sql-with-oracle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants