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 native support for OracleDB #41

Merged
merged 3 commits into from
Apr 20, 2024

Conversation

PhilippSalvisberg
Copy link
Contributor

@PhilippSalvisberg PhilippSalvisberg commented Apr 20, 2024

Background

After you've implemented #37 I opened an enhancement request for the node-oracledb driver (see oracle/node-oracledb#1629). This was accepted and implemented in version 6.4.0 of the node-oracledb driver. Oracle also opened a PR for the type definition (see DefinitelyTyped/DefinitelyTyped#69327). This PR has been accepted and is now included in version 6.4.0 of @types/oracldb on npm.

So, OracleDB is now a first-class citizen as PostgreSQL and MySQL (when using sql-template-tag >=5.2.0, node-oracledb >=6.4.0 and @types/oracledb >=6.4.0).

Change

This PR updates the README regarding query.statement and query.values which are now automatically considered by the latest node-oracledb driver.

@blakeembrey
Copy link
Owner

blakeembrey commented Apr 20, 2024

That is awesome, thank you! Did you also want to update the package.json to align (description and keywords):

"description": "ES2015 tagged template string for preparing SQL statements, works with `pg` and `mysql`",
"keywords": [
"sql",
"template",
"string",
"tag",
"es2015",
"es6",
"pg",
"postgres",
"mysql"
],

Happy to merge either way, this is really cool to see.

Separately, I did see this comment:

  1. bulk insert or executeMany doesn't work as the expected positional binds, values differ in syntax from what the sql function returns..
  2. RETURNING Clause with out binds for INTO clause don't work as the sql function expects the binds to have input values given.

Do you happen to know what those refer to and whether I should look into fixing anything in this library?

@PhilippSalvisberg
Copy link
Contributor Author

(...) update the package.json to align (description and keywords)

Done. Also added sqlite.

Do you happen to know what those refer to and whether I should look into fixing anything in this library?

The comment refers to cases where simple positional parameters (:1, :2, etc.) cannot be used as binds. See examples in 7.3. DML RETURNING Bind Parameters and 8. Executing Batch Statements and Bulk Loading. In such cases the library cannot be used. IMO that's good enough.

@blakeembrey
Copy link
Owner

Perfect, makes sense. I can create a follow up discussion if you're interested in solving them but it doesn't seem urgent. At least the batch statement use-case seems simple to do across all dialects, I can tag you on an attempt to solve it when I have a chance.

@blakeembrey blakeembrey merged commit ee2db48 into blakeembrey:main Apr 20, 2024
2 checks passed
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.

2 participants