lunarmodules / luasql

LuaSQL is a simple interface from Lua to a DBMS.
http://lunarmodules.github.io/luasql
535 stars 192 forks source link

ODBC Connection String #96

Open ht250763 opened 5 years ago

ht250763 commented 5 years ago

Hello, for an ODBC connection I use in Excel following string:

"DSN=DataSource;Driver={};DBQ=.;UID=mngr;PW=xxxx;DOMAIN=HS;RemoteHost=HS;CPU=N;LAN=N"

... and get a connection to the database.

In Lua I wrote: local driver = require"luasql.odbc" local env = driver.odbc()

How should be the connect statement?

This delivers "nil":

local con = env:connect("DSN=DataSource;Driver={};DBQ=.;UID=mngr;PW=xxxx;DOMAIN=HS;RemoteHost=HS;CPU=N;LAN=N")

Thanks!

tomasguisasola commented 5 years ago

Hi

I am sorry I cannot remember the details about ODBC connections. I used it more than 10 years ago... What I can tell you is that this first string passed to env:connect() will be passed to SQLConnect() ODBC driver function. It will likely receive another two strings (following that first one) with the username and the password.

I recommend you check the error message returned. You could change your last line of code to:

local con = assert (env:connect("DSN=DataSource;Driver={};DBQ=.;UID=mngr;PW=xxxx;DOMAIN=HS;RemoteHost=HS;CPU=N;LAN=N"))

This will throw an error when failed AND show the error message. Could you send this message to me?

Regards, Tomás

Em qua, 15 de ago de 2018 às 07:43, ht250763 notifications@github.com escreveu:

Hello, for an ODBC connection I use in Excel following string:

"DSN=DataSource;Driver={};DBQ=.;UID=mngr;PW=xxxx;DOMAIN=HS;RemoteHost=HS;CPU=N;LAN=N"

... and get a connection to the database.

In Lua I wrote: local driver = require"luasql.odbc" local env = driver.odbc()

How should be the connect statement?

This delivers "nil":

local con = env:connect("DSN=DataSource;Driver={};DBQ=.;UID=mngr;PW=xxxx;DOMAIN=HS;RemoteHost=HS;CPU=N;LAN=N")

Thanks!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/keplerproject/luasql/issues/96, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIA7VUaR89ekDd8BHhGR4uxWdk4S6nEks5uQ_s2gaJpZM4V96Fn .

blumf commented 5 years ago

It won't work. You need to use a different ODBC C API function to use direct DSN connections (SQLDriverConnect instead of SQLConnect)

I did try and get a patch into the driver years ago (see ODBC_DSN branch) to support this but hit resistance to the change.

tomasguisasola commented 5 years ago

Hi

I cannot remember the episode you mentioned. However, I am giving attention to the issue now and I have some questions:

  1. The current implementation may not work for your case, but it works in general, don't you agree? Thus I don't want to change that behavior. In particular, passing arguments to env:connect() must be standard to all LuaSQL drivers. We can add another connection method or accept other formats but we cannot change the standard signature.

  2. What do you think about adding another connection method? It could be very straight forward, isn't it? On top of it, one can override the original env:connect adding some inteligence to choose the proper connection function.

  3. Another option is adapting the current env:connect() to handle those two cases (or three, if we also consider the function SQLBrowseConnect). In this case, I would like to know if we can try one of these connection functions (say SQLConnect) then if it returns a certain error message, we try another one (say SQLDriverConnect) and so on... What do you think about?

Regards, Tomás

Em qua, 15 de ago de 2018 às 09:45, blumf notifications@github.com escreveu:

It won't work. You need to use a different ODBC C API function to use direct DSN connections (SQLDriverConnect instead of SQLConnect)

I did try and get a patch into the driver years ago (see ODBC_DSN branch) to support this but hit resistance to the change.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/keplerproject/luasql/issues/96#issuecomment-413184848, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIA7ZgXCt4AmCrEhDx3dJn9zYm5-0pbks5uRBYjgaJpZM4V96Fn .

blumf commented 5 years ago
  1. The current implementation may not work for your case, but it works in general, don't you agree?

It doesn't for many people, hence this ticket.

Also the patch is backwards compatible. Current connect calls still work.

  1. What do you think about adding another connection method?

That has several drawbacks:

For starters, imagine a typical application where the DB connection details are placed in some config file or other area away from the call to connect. Contrast...

cfg = get_db_config()
con = env:connect(cfg)

To a much messier

cfg = get_db_config()
if cfg.something then
  con = env:connect_this_way(cfg)
else if cfg.other_thing then
  con = env:connect_other_way(cfg.unpack, cfg.this, cfg.mess)
else
  -- etc. etc.
end

All DB driver specific too. If your app supports other DB engines, that gets worse.

But, even if you're happy with that mess. You also have the issue that many DB systems have far more complex connection criteria than just plain [db],[user],[paswd].

Currently, we just add on params to the connect call (or reuse standard param positions e.g. SQLite3's case. What was that you were saying about standard to all drivers?), so, you can end up with...

env:connect(db, user, pswd, nil, nil, nil, nil, "Thing I'm actually bothered about")

Not exactly self documenting code. Contrast with...

env:connect{
  db = "...",
  user = "...",
  password = "...",
  charset = "...",
}

You can tell exactly what's going on there (note the direct table call, this is the Lua way to do things, the language explicitly supports this)

And remember, the current call standard still works, this change is backwards compatible.

  1. Another option is adapting the current env:connect() to handle those two cases ... In this case, I would like to know if we can try one of these connection functions (say SQLConnect) then if it returns a certain error message, we try another one (say SQLDriverConnect) and so on... What do you think about?

That's sort of possible, but leads to issues: typos producing spurious errors, doubling up slow, I/O bound calls.

(or three, if we also consider the function SQLBrowseConnect).

You wouldn't use SQLBrowseConnect in that way, it needs interaction as successive calls return requests for more info until a connect succeeds. It would make sense to expose this in a separate method though as it's useful at development time.

tl;dr This change not only solves this particular ticket, but results in cleaner app code and greater flexibility in use, currently and into the future. All whilst remaining backwards compatible.