search

SAP / go-ase

SAP ASE Database Client for Go
Apache License 2.0
25 stars 14 forks source link

go-ase and cgo-ase should support same set of connection properties #194

Closed hqin closed 3 years ago

hqin commented 3 years ago

Problem Description

go-ase and cgo-ase should support same set of connection properties, even if some properties have no effect in a driver. Both drivers are registered with same name "ase". It would make developers life easier for switching drivers without changing application code.

Proposed Feature

Would like "log-client-msgs" and "log-client-msgs" in "cgo-ase" driver to be available in "go-ase" driver as well. These two properties prove to be very useful in trouble shooting.

Additional context

Add any other context or screenshots about the feature request here.

ntnn commented 3 years ago

Thank you for the issue - the drivers have different properties since they operate very differently under the hood.

cgo-ase is a shim/wrapper around Client-Library - SAPs client library for TDS communications. As such it is not possible to expose the inner workings as this functionality is not exposed by Client-Library. cgo-ase also doesn't support e.g. Cursors and we aren't planning to add that functionality.

go-ase on the other hand is written by us and as such allows us to provide more configuration and customization.

We had a discussion on whether or not we want go-ase and cgo-ase to support or even accept the same properties - we came to the conclusion that we do not want our drivers to accept properties they do not use to prevent any confusion about the properties a driver supports.

Both drivers register as the same name as they both target ASE - not because they support the exact same set of properties. E.g. https://github.com/lib/pq and https://github.com/jbarham/gopgsqldriver both register as "postgres" and don't support the same properties.

Regarding the properties you mentioned: log-server-msgs equivalents are already exposed, one option is https://github.com/SAP/go-dblib/blob/main/tds/info.go#L30 which logs all packages (a superset of a message in Client-Library). Alternatively you can add an EEDHook https://github.com/SAP/go-dblib/blob/main/tds/eedHook.go#L12 to your connection or connector: https://github.com/SAP/go-ase/blob/main/conn.go#L45 https://github.com/SAP/go-ase/blob/main/connector.go#L35

EEDPackages are roughly what messages are in cgo-ase/Client-Library. I'd recommend using the DebugLogPackages for debugging and the EEDHooks to tie go-ase into your logging infrastructure.

If you use NewInfoWithEnv to initialize the Info struct you can also execute your application with ASE_DEBUG_LOG_PACKAGES=true ./application to enable DebugLogPackages.

log-client-msgs is only relevant to cgo-ase as it logs the messages from Client-Library, which is not used in go-ase.

The difference in naming stems from the fact that cgo-ase has two sources of messages (Client-Library and the EEDPackages sent by the remote endpoint), while go-ase and go-dblib/tds only use EEDPackages directly and errors for everything else.

I'll be closing this issue as the proposal for noop properties has been denied and the desired functionality is present.

hqin commented 3 years ago

@ntnn: Thanks for the clarification of your position on properties.

Could you add the explanation of "debug-log-packages" in README.md? It is a very useful property for trouble shooting.