node-oracledb icon indicating copy to clipboard operation
node-oracledb copied to clipboard

Make autoCommit, maxRows, outFormat, etc. connection attributes rather than global properties

Open mrapczynski opened this issue 8 years ago • 9 comments

This is enhancement request to make the various SQL execution properties such as autoCommit, maxRows, or outFormat attributes of a connection whether directly established or acquired from a pool, and not global constants on the oracledb object itself.

I maintain an internal library for our developers that wraps around node-oracledb to provide a variety of conveniences like multiple named Oracle connections from external config, promisification, etc. I have had to do a measurable amount of extra engineering to work around the fact that something like autoCommit is a global property rather than a connection specific one.

Off the top of my head, I think this per connection configurability could be something many developers might like to use and not just myself. If I recall right, the Java JDBC driver works in this way where these behaviors are per connection and not at the global driver level.

mrapczynski avatar Feb 24 '16 18:02 mrapczynski

@mrapczynski I'm going to need some convincing that the added UI complexity is worth the flexibility. Early on we decided to make the attributes settable in two places: at the oracledb level and via execute()'s options.

Anyone else want to chime in with an opinion?

cjbj avatar Feb 25 '16 10:02 cjbj

I am definitely :+1: on removing it from global.

The problem with having it globally is when you have to connect to multiple oracle db's or different schemas. You often want to have different settings depending on the usage scenario. So then, you're forced to send it with every execute.

Another similar way it impacts the way we use it at my company, is that some of our services are deployed together and not separately. If two services, both end up using oracle, then those global settings are shared between the two. So if one service does not expect it, you can get bad results. That's the biggest problem with it.

Keeping the option of setting it with every execute should be maintained if this considered since that's very useful in its own.

ldesplat avatar Feb 25 '16 17:02 ldesplat

I'm for connection level autocommit, as you need same value for all your operations when working with that connection in your specific scenario but other scenarios might not need same value. So in app level, that might not make sense so you might want to set it once at start of scenario in the connection. That is why I have the 'transaction' capability in my wrapper, so all operations in that flow are automatically autocommit=false regardless of what you provide in the execute or globally and you don't need to worry about it. https://github.com/sagiegurari/simple-oracledb#usage-transaction

maxrows and outformat are really logical to set per operation, definitely not globally, because you write your output handling code based on that and you might do mixing in same scenario.

sagiegurari avatar Feb 25 '16 18:02 sagiegurari

To list them out, are these the options you want at the connection level? autoCommit fetchAsString lobPrefetchSize maxRows outFormat prefetchRows

What about resultSet?

I could spend time quibbling that outFormat is really an app-wide setting, but @sagiegurari is already wanting faster development!

cjbj avatar Feb 25 '16 23:02 cjbj

I think all of them. Thinking about this further...would it make sense to enable setting these options at several levels: global, pool, connection, and statement? Statement (execute) inherits and/or overrides settings from the connection, the connection from the pool, and the pool from global.

jeffm13 avatar Feb 26 '16 00:02 jeffm13

A global/pool/connection/execute hierarchy is ultimately flexible but there is a cognitive burden and added complexity. I know the gut feel is that it would be "nice" but I'd like to see some detailed discussion of exactly how difficult it is to always set these at the execute level.

cjbj avatar Feb 26 '16 01:02 cjbj

outFormat, autoCommit, maxRows, fetchAsString are not app level settings. we are not alone and we use other libraries when we develop (lets say we are working with sails and use their adapter to work with oracle, but also at same time use oracle directly to do other things), so we can't count on global settings being in a specific state. Since these attributes impact the code, not just behavior, we really need to know at the beginning of the flow, so connection level instead of app level makes more sense. However, some attributes you might want to mix in same flow like outFormat/resultset so some should stay on execute level.

In simple-oracledb I check the output of the query to see how it is structured and based on that, different code is executed (for example if it is rows or resultset, if a row is array or object and so on...). That is ok for a library which does this in 1 place only once, but it is not good for application that works directly with oracledb and either has to set it in execute everywhere or check the output everywhere.

sagiegurari avatar Feb 26 '16 07:02 sagiegurari

Let's keep this in the todo list.

cjbj avatar Mar 01 '16 02:03 cjbj

Since var oracledb = require('oracledb'); returns a singleton, we should have the ability to specify such settings at pool/connection/execution level. For example, we need to talk to multiple Oracle DBs with different setting using the same driver.

raymondfeng avatar Jan 13 '17 23:01 raymondfeng