alexbrainman / odbc

odbc driver written in go
BSD 3-Clause "New" or "Revised" License
352 stars 140 forks source link

Panic on init #122

Open HJTP opened 6 years ago

HJTP commented 6 years ago

It appears I have run into the same error as https://github.com/alexbrainman/odbc/issues/99 I get the same panic, with the same strange characters. I do not know why I see these characters instead of a normal error, I do now know the characters at all.

panic: SQLSetEnvUIntPtrAttr: {㠳6} ㈵″㠳4
 goroutine 1 [running]:
github.com/alexbrainman/odbc.init.0()
        /home/USER/go/src/github.com/alexbrainman/odbc/driver.go:75 +0x75
github.com/alexbrainman/odbc.init()
        <autogenerated>:1 +0x6c
main.init()
        <autogenerated>:1 +0x4e

The error clearly happens in the initialisation phase, as the minimum failing example is tiny:

package main

import (
    _ "github.com/alexbrainman/odbc"
)

func main() {
    sql.Open("", "")
}

I have followed the wiki guide to install unixODBC (version 2.3.1-11.el7, x64), and unixODBC-devel (same version). I am trying to connect to a Postgres database, so I have also installed postgresql-odbc.

I am on centOS, kernel 3.10.0-693.21.1.el7.x86_64

The file odbc.ini looks like this:

[Postgrestestdb]
Description=PostgreSQL connection to test db
Driver=PostgreSQL
Database=mytestdb
Servername=localhost
Username=postgres
Password=redacted
Port=5432

I can succesfully connect to this by running isql -v Postgrestestdb.

And the odbcinst.ini file:

[PostgreSQL]
Description = ODBC for PostgreSQL
Driver = /usr/lib/psqlodbcw.so
Setup = /usr/lib/libodbcpsqlS.so
Driver64 = /usr/lib64/psqlodbcw.so
Setup64 = /usr/lib64/libodbcpsqlS.so
FileUsage = 1

As is the default from the unixODBC package.

Thanks in advance for your help.

alexbrainman commented 6 years ago

It appears I have run into the same error as #99

Looks similar.

I am trying to connect to a Postgres database

Why don't you use https://github.com/lib/pq instead? Then you won't need to dable with ODBC.

Alex

HJTP commented 6 years ago

Thanks for your reply.

Why don't you use https://github.com/lib/pq instead? Then you won't need to dable with ODBC.

Because Postgres is not the final station. I'm just trying to get ODBC to work, and then I'll try to connect a third-party database.

Do you have any suggestions to at least make the error message legible?

alexbrainman commented 6 years ago

Do you have any suggestions to at least make the error message legible?

I do not have any good suggestion. Sorry.

I suspect something is misconfigured on your system.

I created this package when I needed to access MS SQL Server from my Windows program. And that worked really well, and did not need any configuration. It did not need CGO. It did not need any installing - because any Windows computer has MS SQL Server ODBC driver installed.

And then others started using this package on non-Windows. And non-Windows needs CGO. And you need to install ODBC software. And you need to install ODBC driver. And some drivers comes with a source, and some without source. And all that required a lot of configuration / compiling to actually work together.

It, probably, is possible to make it all work, but you need to be prepared for complications and time wasting. I don't have much free time now days to help people to debug their system configuration. So you have to do it yourself. I am happy to help in whichever way I can, but in this situation, I do not even know where to start.

Alex

awaken commented 4 years ago

Hi Alex, I also incurred in this error. I understand your position, and I agree with your point of view.

However, when panic is called panic from the init function, the application has no opportunity to recover the execution, my app cannot handle this scenario (I need my app to stay running, as service, with remote logging of the error). I will understand that calling panic during init is not appropriate for several production scenarios, indeed it is also discouraged for design principle.

Therefore, I am just asking you to avoid to call panic from any init function. You might keep track of the arisen error (from init) into some (unexported) global variable, so that, any future call for creating a new database instance will fail, returning this error. Does it sound appropriate to you?

Thanks

alexbrainman commented 4 years ago

Therefore, I am just asking you to avoid to call panic from any init function. You might keep track of the arisen error (from init) into some (unexported) global variable, so that, any future call for creating a new database instance will fail, returning this error. Does it sound appropriate to you?

It does sounds good to me.

I just pushed this branch

https://github.com/alexbrainman/odbc/tree/do_not_panic

with your change. Does it look good to you?

Thank you.

Alex

awaken commented 4 years ago

Thank you very much! Much appreciated.

I would be great if you can change the initDriver function for returning a little more detailed error, in order to have the opportunity to identify where it has been originated, without ambiguity.

I am not asking for anything complex. It might be as simple as replacing (in the body of the initDriver func), the code lines like:

    return NewError("SQLSetEnvUIntPtrAttr", drv.h)

with these ones:

    return NewError("SQLSetEnvUIntPtrAttr(SQL_ATTR_ODBC_VERSION, SQL_OV_ODBC3)"), drv.h)   // this is line 42 of driver.go

    return NewError("SQLSetEnvUIntPtrAttr(SQL_ATTR_CONNECTION_POOLING, SQL_CP_ONE_PER_HENV)", drv.h)    // this is line 50 of driver.go

    return NewError("SQLSetEnvUIntPtrAttr(SQL_ATTR_CP_MATCH, SQL_CP_RELAXED_MATCH)", drv.h)    // this is line  57 of driver.go

Thanks a lot!

alexbrainman commented 4 years ago

I would be great if you can change the initDriver function for returning a little more detailed error,

https://github.com/alexbrainman/odbc/tree/do_not_panic

change is only about not panicking during init. If you want to change error messages, please, send separate PR.

So, should I submit do_not_panic branch? Does it fixes your problem of not panicking in init?

Thank you.

Alex

awaken commented 4 years ago

I tested it, and it works properly, I am able to keep my app and running. Thanks a lot! Yes, submit it please.

I will send you the PR. Thanks again.

alexbrainman commented 4 years ago

I tested it, and it works properly, I am able to keep my app and running. Thanks a lot! Yes, submit it please.

No worries. Submitted.

I will send you the PR.

Sounds good.

Alex

awaken commented 4 years ago

Here's the PR https://github.com/alexbrainman/odbc/pull/143!

It contains a fix for better compatibility, in accordance to the odbc specs. Now I can query Teradata Vantage from Go.

As you can see, it is pretty simple, I hope you will merge it to the master branch. Greetings

alexbrainman commented 4 years ago

Here's the PR #143!

It contains a fix for better compatibility, in accordance to the odbc specs. Now I can query Teradata Vantage from Go.

As you can see, it is pretty simple, I hope you will merge it to the master branch. Greetings

I will have a look at it sometime this week.

Thank you.

Alex