I added ODBC module ported from Q. It compiles under Linux for now.
There is a difference against Q: Records of resultsets and parameters
to sql_exec are represented as lists instead of tuples.
Most functions seem to work here, but `odbc_sources' and `odbc_drivers'
return always an empty list. Who knows why, `SQLDataSources' and
`SQLDrivers' return always `SQL_NO_DATA'. The same code from Q works fine.
The binary data is represented by bytestrings in the same way as in Q.
Pure is now lacking direct support for bytestrings. Should we create it
or is there a better way to represent binary data in Pure?
When time permits I'll continue with Windows compilation.
Enjoy.
Jiri
Wow, we're really getting somewhere. :) Thanks a lot!
I ran into various compilation issues, mostly header-related stuff and
64 bit quirks. I already fixed these in svn. Note that I changed the
default odbc lib to -lodbc which is what seems to be available on most
systems nowadays. I also added an ODBCLIB variable for that. If you
prefer, you can change the default value of ODBCLIB back to -liodbc, but
AFAICT unixODBC seems to be the way to go (iodbc seems to be largely
unmaintained and SUSE doesn't even ship it any more; whereas unixodbc is
still actively developed and is included in all major Linux distros).
There are still quite a few memory leaks to be fixed, due to the
different way in which (is_)tuplev/listv works in Pure. I'm looking at
these right now.
I also did some minor cosmetic surgery on odbc.pure (r764).
Specifically, ODBC::odbc_error is just ODBC::error now and the 'using
namespace' declaration in the second half of the module was superflous
(the current namespace is always searched anyway).
I'd really prefer it if the namespace was named 'odbc' rather than
'ODBC' so that it matches the module name. Any objections to that?
> There is a difference against Q: Records of result sets and parameters
> to sql_exec are represented as lists instead of tuples.
Yes, this makes sense. But then the singleton case in odbc_sql_exec
ought to be removed and only lists should be allowed as the third
parameter; the singleton case really makes sense only in conjunction
with tuples. I've already committed this in r763.
> Most functions seem to work here, but `odbc_sources' and `odbc_drivers'
> return always an empty list. Who knows why, `SQLDataSources' and
> `SQLDrivers' return always `SQL_NO_DATA'. The same code from Q works fine.
Did you configure your ODBC data sources? On Linux there are some config
files (.odbcrc or similar) which need to be edited. UnixODBC also offers
some GUI tools for that. Also, you need to have the ODBC drivers for
your RDBMS installed (as well as the RDBMS itself, of course).
> The binary data is represented by bytestrings in the same way as in Q.
This doesn't work. In Pure bytestrings are just raw C pointers without
any size information, so the size information needs to be passed
separately in some way, or we need a new kind of blob data type. I'm not
sure what the right thing is, I'll have to take a closer look at it.
> When time permits I'll continue with Windows compilation.
IIRC there should be some mingw headers and an import library for the
Win ODBC library. I'll have to look it up.
Cheers,
Albert
--
Dr. Albert Gr"af
Dept. of Music-Informatics, University of Mainz, Germany
Email: Dr.G...@t-online.de, a...@muwiinfa.geschichte.uni-mainz.de
WWW: http://www.musikinformatik.uni-mainz.de/ag
Inbetween I added README, COPYING and config.guess. README is mostly
updated.
Jiri
On Sat 07/02/09 4:34 PM , Jiri Spitz jiri....@gmail.com sent:
> I added ODBC module ported from Q. It compiles under Linux for now.
This one is a must for me. Thanks :)
> Enjoy.
That, I shall ;-)
e.r.
Now I discovered a weird bug maybe caused by memory allocation/disposal
disorder. To reproduce do:
> let db = odbc::connect ... ;
> odbc::tables db; // OK
> odbc::sql_exec db ... ; // OK
> odbc::sql_fetch db; // crash
It happens always both under Linux and Windows.
Jiri
Sorry, I didn't have the time to get around fixing the memleaks yet, as
I'm still working on pure-gen.
It isn't terribly hard to do if you want to fix those memleaks yourself.
You have to look for all calls to pure_listv/pure_is_listv as well as
pure_tuplev/pure_is_tuplev.
The expression vectors taken or returned by these aren' freed
automatically in Pure, you'll have to do that yourself. That's all. But
you have to be careful with pure_is_listv and pure_is_tuplev; if these
return zero elements then the corresponding expression vector will be
NULL, so better don't free it in that case. ;-)
> I let it be the same way as in Q - the bstr struct. Only the Pure stuff
> to deal with it is missing.
Well, those are simply pointers to a struct containing the size and
another pointer to the data, which can easily be handled using the ffi
struct functions if necessary. I'll have a look at this when time
permits. Fortunately, starting next week we have semester holidays. :)
>> IIRC there should be some mingw headers and an import library for the
>> Win ODBC library. I'll have to look it up.
>>
> It compiles without any problems :-) .
Great. :)
Just for the record, the bigint stuff needs a review as well.
Specifically, pure_mpz copies its argument, so the mpz_t argument needs
to be freed with mpz_clear after that. Similarly, the mpz_t result in
the call to pure_is_mpz in set_arg also needs to be cleared after use.
Also, the bigint marshalling uses an intermediate string representation
right now. This is wasteful. runtime.h has functions for directly
converting Pure bigints to 64 bit integers and vice versa, these should
be used instead (cf. odbc.c lines 132 and 1065).
Hmm, I don't see any obvious glitches in that routine. Maybe add some
printf's to the odbc_sql_fetch routine to see where exactly it bombs?
> The expression vectors taken or returned by these aren' freed
> automatically in Pure, you'll have to do that yourself. That's all. But
> you have to be careful with pure_is_listv and pure_is_tuplev; if these
> return zero elements then the corresponding expression vector will be
> NULL, so better don't free it in that case. ;-)
ISO C and Posix compliant free() will do nothing if passed NULL, so that
advice is pretty much obsolete.
--
Do I contradict myself? John Cowan
Very well then, I contradict myself. co...@ccil.org
I am large, I contain multitudes. http://www.ccil.org/~cowan
--Walt Whitman, Leaves of Grass
Yes, that's because Pure doesn't need all this bstr stuff any more since
it can handle raw C pointers just fine and interface directly to the C
routines that handle them. So the sooner we get rid of this legacy the
better. :)
> Well, those are simply pointers to a struct containing the size and
> another pointer to the data, which can easily be handled using the ffi
> struct functions if necessary. I'll have a look at this when time
> permits.
All right, I reviewed the uses of these in pure-odbc, and basically
they're employed for two purposes:
- odbc::getinfo: This is essentially a raw interface to SQLGetInfo(),
which returns a pointer to the requested information, which either
points to an integer or a string. I think that it would make things much
easier to just return a malloc'd pointer there. The caller presumably
knows what kind of information he wants to get, thus he can just invoke
get_int or cstring_dup from the prelude to extract the information. In
any case the len field doesn't provide any useful additional information
here.
- As query parameters in sql_exec and query results in sql_fetch. Here
the binary data will usually be handled through a third party library.
E.g., you might want to store, sounds, graphics and videos in a
database, that's what these blobs are for. Those external libraries
won't be able to do anything with a bstr struct either. Hence I think we
should represent the blobs as pairs size,data, on both input and output,
where data is just a malloc'd pointer to the data and size the size of
the blob (better allow a bigint for that, I'd guess that SQLLEN is
actually a 64 bit type nowadays, at least on 64 bit systems).
The necessary changes to odbc.c are trivial, I can do them if you want.
Ok?
I'm afraid we cannot avoid this (at least on 32 bit systems). Citing
from ODBC documentation:
"SQL_BIGINT:
The number of bytes required to hold the character representation of
this data if the character set is ANSI, and twice this number if the
character set is UNICODE, because this data type is returned as a
character string by default. The character representation consists of 20
characters: 19 for digits and a sign, if signed, or 20 digits, if
unsigned. Therefore, the length is 20."
Since the `iv' member of the ODBCParam struct is defined as long, we
cannot assign a 64 bit integer to it on a 32 bit system. Moreover, I did
not find a SQL C type for BIGINT.
Am I right?
Jiri
Jiri
I enclose the crash dump.
Jiri
Yes, I came pretty much to the same conclusion. Ok, then so be it.
Will do, as soon as I get an odbc client up and running.
Are you sure? No function should ever crash on a return statement like
that, unless the C stack is thrashed. Now that would be *really* bad.
> I suspect there is something wrong with `odbc_tables'. As long as I do not
> call it, `odbc_sql_fetch' works fine. As soon as I call `odbc_tables'
> then `odbc_sql_fetch' always crashes regardless how many other
> statements were called in between. It seems, `odbc_tables' leaves the
> environment somehow corrupted. I have no idea, how to debug this.
Well, it might be that odbc_tables() trashes the sql statement data
structure. Reinitializing that from scratch might help. Will look into
that when I find the time.
Albert
Yes, I am. And the dump witnesses for your malicious diagnosis - "Stack
smashing" :-( .
Jiri
Revision 851 does not suffer from this weird error under Windows
anymore. :-) I'll try it under Linux in the evening and let you know.
Jiri
Unfortunately, under Linux the problem persists :-(. Moreover, I found
other similar bug. The combination typeinfo + sql causes a segfault.
Jiri
Also, numelem in set_arg needs to be a size_t. I just fixed that (r862).
I'll take a look at the other issue some time next week. Can you please
upload a minimal example which reliably reproduces the segfault on Linux?
>
> I'll take a look at the other issue some time next week. Can you please
> upload a minimal example which reliably reproduces the segfault on Linux?
>
It is enclosed.
Jiri
Looks good to me. I did some minor surgery, though, you can find the
details in the svn logs.
Having thought about this some more, I think that we should actually
split odbc::getinfo into two routines, odbc::getinfo_int which returns
an int and odbc::getinfo_string which returns a string value. Thereby we
can completely get rid of the pointer returns in odbc::getinfo (which
always invite memleaks). Do you see any problems with that?
Or maybe there's some easy clue in the type_info constants which would
allow odbc::getinfo allow to figure out the return type on its own? I
don't see anything in the SQLGetInfo description, though. Another
example of an API that's broken by design.
I also think that we should turn the raw blob pointers in sql_fetch
results into cooked pointers which free themselves after use, so that
they don't leak memory either. That's easy to add and very convenient
since the user doesn't have to free those blobs manually. Ok?
NB: I finally got a MySQL database with unixODBC up and running so that
I can now test stuff myself. ;-) I did some more cosmetic surgery, most
notably the example is now in a separate examples subdir as required by
'make dist' and I also renamed it to menagerie.pure. I expect to look at
the tables/sql_fetch bomb later today or tomorrow.
> I also think that we should turn the raw blob pointers in sql_fetch
> results into cooked pointers which free themselves after use, so that
> they don't leak memory either. That's easy to add and very convenient
> since the user doesn't have to free those blobs manually. Ok?
>
Oh, yes.
Jiri
The problem is that the info_type enumerants are open-ended, so in
general we don't really have that type information. Note that odbc.pure
only defines the common enumerants, ODBC drivers are free to define
their own. I wouldn't want odbc::getinfo to preclude use of the
vendor-specific info_types.
As a compromise I made the result of odbc::getinfo a cooked pointer so
that it frees itself now. Blob results returned by odbc::sql_fetch are
now auto-disposing as well (r956).
NB: There's a bug in the pure_symbol() function of the Pure runtime (all
versions <= 0.18) which keeps the 'free' sentries defined in odbc.c from
working. This has been fixed in r955.
Cheers,
Albert
P.S.: Ryan and Toni, I've attached the necessary patch to runtime.cc for
your convenience. It would be nice if you'd consider applying that patch
to your pure-0.18 packages. The bug is not a real showstopper (doesn't
cause any segfaults), but will cause memleaks with pure-odbc which I
expect to be released soon.
> P.S.: Ryan and Toni, I've attached the necessary patch to
> runtime.cc for
> your convenience. It would be nice if you'd consider applying that
> patch
> to your pure-0.18 packages. The bug is not a real showstopper (doesn't
> cause any segfaults), but will cause memleaks with pure-odbc which I
> expect to be released soon.
Done in MacPorts:
http://trac.macports.org/changeset/47267
Fixed (r980). Jiri, can you please verify? (This is about the
typeinfo/table-related crashes. Pretty obvious in retrospect. Wonder why
that went unfixed for so long.)
I also fixed a bunch of other bugs and memleaks (see the svn log), added
lazy variations of sql/msql which produce streams instead of (eager)
lists (pretty useful for dealing with large result sets), and added some
remarks about how to interface to SQLite via ODBC (I think that's by far
the most convenient way to get a local database up and running quickly,
if you don't want to depend on Windows-only stuff).
Jiri, I'm through with my TODO list (except rechecking the Windows
port), so I'd say it's about time for a 0.1 release. Anything you still
want to fix or add?
> I also fixed a bunch of other bugs and memleaks (see the svn log), added
> lazy variations of sql/msql which produce streams instead of (eager)
> lists (pretty useful for dealing with large result sets), and added some
> remarks about how to interface to SQLite via ODBC (I think that's by far
> the most convenient way to get a local database up and running quickly,
> if you don't want to depend on Windows-only stuff).
>
I would rather say you rebuilt the interface from the scratch ;-) .
> Jiri, I'm through with my TODO list (except rechecking the Windows
> port), so I'd say it's about time for a 0.1 release. Anything you still
> want to fix or add?
>
In between I noticed we should change lists of tuples into lists of
lists for columns, primary_keys and foreign_keys as well because
one-element tuples can occur there.
Otherwise, I am happy with the functionality. As regards Windows, it
compiles and works without any problems.
Many thanks,
Jiri
Great. :)
> I would rather say you rebuilt the interface from the scratch ;-) .
Not really. I just got rid of a few unnecessary complications. ;-)
Seriously, you did most of the tedious work on that port. I really
appreciate that!
> In between I noticed we should change lists of tuples into lists of
> lists for columns, primary_keys and foreign_keys as well because
> one-element tuples can occur there.
columns and primary_keys are supposed to always return lists of strings,
foreign_keys always a list of string triples. I don't see why these
should be changed. Can you elaborate, please?
> Otherwise, I am happy with the functionality. As regards Windows, it
> compiles and works without any problems.
I don't recall whether any additional packages are needed to compile and
link odbc apps with mingw, did you have to install anything special for
that?
Correction: columns is supposed to return a list of quadrupels.
Yeah, I see what you mean now. The NULLABLE and DEFAULT members in the
columns result may be () if they are "empty". Hmm.
Ok, then we could just turn the tuples in columns and foreign_keys into
lists, that makes some sense. tables should be changed then, too. But
columns? It seems inconvenient to turn a silly list of strings into a
list of 1-lists just for the sake of 100% consistency. ;-)
s/columns/primary_keys/. I should think before hitting "send". :)
> I don't recall whether any additional packages are needed to compile and
> link odbc apps with mingw, did you have to install anything special for
> that?
>
Nothing else than for Pure itself (with GMP) is required and "CC=gcc
ODBCLIB=-lodbc32" options for make.
Jiri
Jiri
Right. I've implemented this now (r987). The special constant is named
odbc::SQLNULL. (Didn't want to shadow ::NULL.)
Let me know how this works for you. The SQL NULL representation is now
encapsulated in the pure_sqlnull/pure_is_sqlnull functions in odbc.c, so
we can easily change it back to () if we want.
In fact, the situation with odbc::columns isn't all that bad anyway. The
only field which can actually become SQLNULL right now is the last one
(the default value), one could also just return an empty string in that
case instead. The 'nullable' field is a string which is supposed to be
the empty string (rather than SQLNULL) if the nullable status of a
column isn't known (which isn't supposed to happen in ISO SQL databases
anyway).
Note that now that we have a dedicated SQLNULL constant, we could also
go back to a tuple representation for the data records if we want. (We'd
have to change the blob representation to something other than a pair,
but that isn't a big deal.)
However, I think that representing data records as lists has some
definitive advantages, such as having the entire arsenal of list
functions and comprehensions available to work on them. And it's also
nice for compatibility with Eddie's csv module which also represents
records as lists.
Opinions?
I changed that in the Makefile now, so that no special 'make' options
should be needed. Will test tomorrow.
> In fact, the situation with odbc::columns isn't all that bad anyway. The
> only field which can actually become SQLNULL right now is the last one
> (the default value), one could also just return an empty string in that
> case instead. The 'nullable' field is a string which is supposed to be
> the empty string (rather than SQLNULL) if the nullable status of a
> column isn't known (which isn't supposed to happen in ISO SQL databases
> anyway).
>
There seems to be something wrong with the 'nullable' field - it returns
*always* SQLNULL (or () in older revisions) independently of the 'not
null' flag (even for primary keys, where it is the default).
> Note that now that we have a dedicated SQLNULL constant, we could also
> go back to a tuple representation for the data records if we want. (We'd
> have to change the blob representation to something other than a pair,
> but that isn't a big deal.)
>
It would not be very practical because a tuple with just one member is
reduced to that member itself. So we would need to add special treatment
for result sets with one field only. I'd prefer a uniform way of
processing for all result sets.
> However, I think that representing data records as lists has some
> definitive advantages, such as having the entire arsenal of list
> functions and comprehensions available to work on them. And it's also
> nice for compatibility with Eddie's csv module which also represents
> records as lists.
>
I am of the same opinion.
Jiri
Jiri
Jiri
Works ok over here, with both mysql and sqlite. (sqlite needs to be told
'not null' explicitly, though, even on a 'primary key' field.)
Seems that the PostgreSQL ODBC driver is to blame for the SQLNULLs in
the IS_NULLABLE field. Apparently it doesn't implement all ODBC 3.0
SQLColumns() fields. In such cases you'll just have to use the
inspection capabilities provided by the DBMS itself, 'show columns' or
whatever.
I don't. Maybe you have CC set in your environment? Or an old/botched
make version?
Works. Did a few adjustments in the menagerie example, so that ppl don't
need to install one of those hulking databases, only SQLite ODBC is
needed, which is really easy to install on Windows.
Jiri, I'm ready to release. Ok?
Jiri
Many thanks,
Jiri
Hmm, I'm running 1.0.11 also, but I did an incremental upgrade IIRC.
Well, just setting CC=gcc in your msys environment should cure it.