Bojan Smojver wrote: > On Tue, 2006-09-05 at 23:39 -0700, Chris Darroch wrote: [snip] >> Now, I may very well be missing something here, but this seems >> backwards to me. I would have thought that you'd want to pass >> an apr_dbd_type_e *type parameter instead, and then datum_get() >> would write into that location the type of the column, and >> write into the void *data parameter the actual data, formatted >> as one would expect for the returned type. So if the column >> contains integers, you'd get a type of APR_DBD_TYPE_INT and >> a pointer to a signed int, and so forth. > > I was thinking about doing it this way as well and maybe it's the right > way - not sure. Here is why I chose to do it the way I did: > > In majority (or all?) of my own database related work, I would know the > column type ahead of getting the data, because I was usually the one > that created the table, had prior knowledge of its structure or could > find out in some SQL way. So, I was thinking, why wait until after the > call to figure out what the column type is (and have another if/switch > construct after datum_get()) - let's specify it upfront. > > The next thing that felt good about this approach is that the storage is > entirely managed by the caller. So, I already have my ints, shorts, > longs, brigades and other data types allocated - then I call datum_get() > with pointers to the allocated data, and I get everything populated and > ready to be used (minus the "is it null?" bit - see below). > > And, it also gives us the opportunity to convert various data on the fly > with datum_get(), without any extra programming effort on caller's > behalf (to a degree, of course).
OK, I see where you're coming from ... I see the value of having the caller request the data format, and letting any problems be on their head (if they ask for a integer and the string value from the DB contains "foo", well, that's their problem). I guess my remaining hestitation relates to the fact that there's a lot of reliance on atoi() and friends here, and that with certain drivers you might see numeric data converted from the DB's internal format to a string (because the driver asks for string data values), using the DB's internal conversion mechanism, and then converted back to a numeric value by atoi(), apr_atoi64(), etc. because the caller asked the driver for a specific numeric format. I'm not a floating-point expert, I confess ... could there be subtle data conversion problems lurking in there? OTOH, if all the drivers work the same way and all use atoi() and friends, then any variations between drivers in extracting some known floating-point value from the DBs would have to be caused by the DBs' internal conversions to strings ... does that let APR off the hook? I just don't know, myself. > In the patches, I return APR_ENOENT from datum_get() if I find a null > value, so no need for apr_dbd_is_null() call. Except that I've forgotten > to do this for SQLite2... Sorry, I'll try to fix that. Gotcha, missed that, sorry! > In other words, in _prepare, we get in %p<something>, which means CLOB, > LOB, FILE or any other additional types that Oracle (and other DBs) may > support. > > Then, in the _p[b]query/select (since we know the type from _prepare), > we get ourselves just a regular apr_dbd_blob_t, but we know to call the > correct bind functions and use the correct parts of it (i.e. length, or > table name, table name etc.). The apr_dbd_blob_t is just a data delivery > mechanism. We could have others - for instance apr_dbd_file_t (for > APR_DBD_TYPE_BFILE) [...] I've never used Oracle BFILEs, but I think you would actually need to pass a directory "alias" (a string) and the filename; these would then go to the OCILobFileSetName() function. I'm not really sure what's best here. Because these structures for LOB data will necessarily vary across drivers, my hestitation remains about putting the structure definition right in apr_dbd.h. I *think* that appending fields to structures might be permitted across minor version changes, without breaking the ABI, but I'm not 100% sure. That might suffice to handle any new fields required by support for additional data types and/or databases in the future. But I'd feel better if an API/ABI rules guru weighed in on that. :-) At any rate, thanks so much for the work on this binary arguments patch! I'm sorry to be stuck in another project for work and to have so little time right now to devote to this. Thanks again! Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
