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

ODBC Connection String #96

Open
ht250763 opened this issue Aug 15, 2018 · 4 comments
Open

ODBC Connection String #96

ht250763 opened this issue Aug 15, 2018 · 4 comments

Comments

@ht250763
Copy link

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
Copy link
Contributor

tomasguisasola commented Aug 15, 2018 via email

@blumf
Copy link
Contributor

blumf commented Aug 15, 2018

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
Copy link
Contributor

tomasguisasola commented Aug 15, 2018 via email

@blumf
Copy link
Contributor

blumf commented Aug 16, 2018

  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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants