Re: Column names with apr_dbd
On Wed, 2006-02-22 at 05:51 +, Nick Kew wrote: If a pool is needed, it should be simple enough to make it a member of the apr_dbd_results_t struct. But since get_row at the same level of the API has a pool argument, I agree your proposal looks like the best solution. Actually, when I think about it now, my idea of passing the pool to get_name seems really bogus. What should happen instead for SQLite3 is apr_pstrdup of column names inside dbd_sqlite3_select (i.e. at present, this part of the code in the driver is buggy, but since it doesn't get used, nobody noticed yet). If sqlite3_finalize does what I think it does (frees memory), then it is just by pure luck that I'm able to keep values of column names by copying them to the pool in get_name (i.e. that memory has been freed long ago). Ah well, I think there will have to be take 3 on this :-( -- Bojan
Re: Column names with apr_dbd
Quoting Bojan Smojver [EMAIL PROTECTED]: What should happen instead for SQLite3 is apr_pstrdup of column names inside dbd_sqlite3_select (i.e. at present, this part of the code in the driver is buggy, but since it doesn't get used, nobody noticed yet). More on this point - the new code should introduce column names (probably char **) in the result set, so that memory for column names gets allocated only once (no need to burden each row with copies of columns names). Then the columns' name members can just point to that. -- Bojan
Re: Column names with apr_dbd
On Wednesday 22 February 2006 07:26, you wrote: On Wed, 2006-02-22 at 05:51 +, Nick Kew wrote: If a pool is needed, it should be simple enough to make it a member of the apr_dbd_results_t struct. But since get_row at the same level of the API has a pool argument, I agree your proposal looks like the best solution. Cool. I'll work on that. I also noticed that some of the patches I sent regarding this may be a bit naive, especially for async queries, so I try to rework and resend. :-) I haven't found time to review them yet - don't know if anyone else has. But async queries shouldn't be an issue here AFAICS. Keep bugging us! -- Nick Kew
Re: Column names with apr_dbd
Quoting Nick Kew [EMAIL PROTECTED]: But async queries shouldn't be an issue here AFAICS. Sorry, I think I wasn't specific enough. It appears to me that PGSQL will be the one having a problem here for async queries. Basically, for async _*select, the res-res is NULL until get_row is called. If someone calls get_name before that (something they should be able to do), my current patch would most likely segfault (PGSQL docs aren't entirely clear on this, but it's better to be safe). Instead, I'll test if res-res is NULL and return NULL if it is. The other code (apart from Oracle, which I don't understand) shouldn't have that problem, as res-res comes into existence when _*select functions get called. Keep bugging us! You can expect another lot of patches today, with the pool argument rework, MYSQL typo fixes and async solution for PGSQL. -- Bojan
Re: Column names with apr_dbd
Quoting Nick Kew [EMAIL PROTECTED]: +APU_DECLARE(const char*) apr_dbd_get_name(const apr_dbd_driver_t *driver, + apr_dbd_results_t *res, int col); I'm having a feeling (after doing a bit of testing on this) that we should also include a pool in the list of arguments here. SQLite3, for instance, returns objects that disappear (due to the call to sqlite3_finalize) unless they are copied into a user specified pool. Some databases may not need to use this (e.g. PGSQL), so they can just ignore the pool. Does that make sense? -- Bojan
Re: Column names with apr_dbd
On Wednesday 22 February 2006 02:03, Bojan Smojver wrote: Quoting Nick Kew [EMAIL PROTECTED]: +APU_DECLARE(const char*) apr_dbd_get_name(const apr_dbd_driver_t *driver, + apr_dbd_results_t *res, int col); I'm having a feeling (after doing a bit of testing on this) that we should also include a pool in the list of arguments here. If a pool is needed, it should be simple enough to make it a member of the apr_dbd_results_t struct. But since get_row at the same level of the API has a pool argument, I agree your proposal looks like the best solution. Thanks:-) -- Nick Kew
Re: Column names with apr_dbd
On Mon, 2006-02-13 at 19:54 +, Nick Kew wrote: It'll have to wait for 1.3, 'cos it affects the API (despite not breaking anything:-) Actually, it just occurred to me that we could do this in a completely backward compatible manner, with (admittedly) somewhat ugly approach. We could modify apr_dbd_get_entry to allow negative numbers (col is signed int, so this should not be a problem), where -1 means get the name of the first column and so on. So, for APR 1.2.x, pseudo code would be (inside get_entry): if (col 0) get_name(get_result_from_row(row), -col - 1); The driver structure would not expose get_name function until 1.3, of course, so everything would remain OK for inclusion in 1.2.x. Would this be something APR committers would be willing to entertain? -- Bojan
Re: Column names with apr_dbd
On Tue, 2006-02-14 at 20:09 +1100, Bojan Smojver wrote: if (col 0) get_name(get_result_from_row(row), -col - 1); This should be, of course: return get_name(get_result_from_row(row), -col - 1); -- Bojan
Re: Column names with apr_dbd
That still doesn't work, because invoking with -1 on apr 1.2.2 v.s. 1.2.4 would be broken. You can't change the binary or functional API between subversions. Bill Bojan Smojver wrote: On Mon, 2006-02-13 at 19:54 +, Nick Kew wrote: It'll have to wait for 1.3, 'cos it affects the API (despite not breaking anything:-) Actually, it just occurred to me that we could do this in a completely backward compatible manner, with (admittedly) somewhat ugly approach. We could modify apr_dbd_get_entry to allow negative numbers (col is signed int, so this should not be a problem), where -1 means get the name of the first column and so on. So, for APR 1.2.x, pseudo code would be (inside get_entry): if (col 0) get_name(get_result_from_row(row), -col - 1); The driver structure would not expose get_name function until 1.3, of course, so everything would remain OK for inclusion in 1.2.x. Would this be something APR committers would be willing to entertain?
Re: Column names with apr_dbd
On Tue, 2006-02-14 at 04:29 -0600, William A. Rowe, Jr. wrote: That still doesn't work, because invoking with -1 on apr 1.2.2 v.s. 1.2.4 would be broken. You can't change the binary or functional API between subversions. Darn! Didn't think of that one :-( -- Bojan
Re: Column names with apr_dbd
On Monday 13 February 2006 07:16, Bojan Smojver wrote: Is there a way to get result set column names with apr_dbd? I couldn't find one... There isn't. But you're not the first to ask for this feature, so I guess it's a strong candidate for inclusion in a future update. If the feature really isn't there, would this be something where patches would be welcome? Or is this a design decision related to some underlying database compatibility/capability problem? FWIW, we have an uncommitted patch that looks like this. Feel free to improve it. --- include/apr_dbd.h.orig +++ include/apr_dbd.h @@ -210,6 +210,16 @@ APU_DECLARE(const char*) apr_dbd_get_entry(const apr_dbd_driver_t *driver, apr_dbd_row_t *row, int col); +/** apr_dbd_get_name: get an entry name from a result set + * + * @param driver - the driver + * @param res - result set pointer + * @param col - entry number + * @return name of the entry, or NULL if col is out of bounds. + */ +APU_DECLARE(const char*) apr_dbd_get_name(const apr_dbd_driver_t *driver, + apr_dbd_results_t *res, int col); + /** apr_dbd_error: get current error message (if any) * * @param driver - the driver -- Nick Kew
Re: Column names with apr_dbd
On Mon, Feb 13, 2006 at 10:52:24AM +, Nick Kew wrote: On Monday 13 February 2006 07:16, Bojan Smojver wrote: Is there a way to get result set column names with apr_dbd? I couldn't find one... There isn't. But you're not the first to ask for this feature, so I guess it's a strong candidate for inclusion in a future update. +1 from the peanut gallery - this would indeed be very practical. vh Mads Toftum -- `Darn it, who spiked my coffee with water?!' - lwall
Re: Column names with apr_dbd
On Mon, 2006-02-13 at 10:52 +, Nick Kew wrote: FWIW, we have an uncommitted patch that looks like this. Feel free to improve it. Thanks. I'll work on the underlying drivers in order to get this going. Any comments in relation to apr_dbd_init situation (my other e-mail)? -- Bojan
Re: Column names with apr_dbd
Bojan Smojver wrote:
On Mon, 2006-02-13 at 10:52 +, Nick Kew wrote:
FWIW, we have an uncommitted patch that looks like this.
Feel free to improve it.
Thanks. I'll work on the underlying drivers in order to get this going.
Here's a patch for the apr_dbd_oracle driver that has the
get_name function implemented. (I hope to get back to some
delayed work on that driver in a month or two.)
Chris.
===
--- apr_dbd_oracle.c.orig 2006-02-13 14:28:27.160654000 -0500
+++ apr_dbd_oracle.c2006-02-13 14:33:24.668866000 -0500
@@ -181,6 +181,7 @@
char *stringval;
OCILobLocator *lobval;
} buf;
+const char *name;
} define_arg;
struct apr_dbd_prepared_t {
@@ -525,6 +526,16 @@
}
#endif
+static const char *dbd_oracle_get_name(const apr_dbd_results_t *res, int n)
+{
+define_arg *val = res-statement-out[n];
+
+if ((n 0) || (n = res-statement-nout)) {
+return NULL;
+}
+return val-name;
+}
+
static int dbd_oracle_get_row(apr_pool_t *pool, apr_dbd_results_t *res,
apr_dbd_row_t **rowp, int rownum)
{
@@ -988,6 +999,8 @@
int_errorcode;
ub2 paramtype[DBD_ORACLE_MAX_COLUMNS];
ub2 paramsize[DBD_ORACLE_MAX_COLUMNS];
+const char *paramname[DBD_ORACLE_MAX_COLUMNS];
+ub4 paramnamelen[DBD_ORACLE_MAX_COLUMNS];
/* Perl uses 0 where we used 1 */
sql-status = OCIStmtExecute(sql-svc, stmt-stmt, sql-err, 0, 0,
NULL, NULL, OCI_DESCRIBE_ONLY);
@@ -1016,6 +1029,10 @@
sql-status = OCIAttrGet(parms, OCI_DTYPE_PARAM,
paramsize[stmt-nout],
0, OCI_ATTR_DATA_SIZE, sql-err);
+sql-status = OCIAttrGet(parms, OCI_DTYPE_PARAM,
+ paramname[stmt-nout],
+ paramnamelen[stmt-nout],
+ OCI_ATTR_NAME, sql-err);
++stmt-nout;
}
}
@@ -1033,6 +1050,8 @@
for (i=0; istmt-nout; ++i) {
stmt-out[i].type = paramtype[i];
stmt-out[i].len = stmt-out[i].sz = paramsize[i];
+stmt-out[i].name = apr_pstrmemdup(stmt-pool,
+ paramname[i], paramnamelen[i]);
switch (stmt-out[i].type) {
default:
switch (stmt-out[i].type) {
@@ -1890,6 +1909,7 @@
dbd_oracle_pvquery,
dbd_oracle_pvselect,
dbd_oracle_pquery,
-dbd_oracle_pselect
+dbd_oracle_pselect,
+dbd_oracle_get_name
};
#endif
===
Re: Column names with apr_dbd
On Monday 13 February 2006 19:10, Bojan Smojver wrote: On Mon, 2006-02-13 at 10:52 +, Nick Kew wrote: FWIW, we have an uncommitted patch that looks like this. Feel free to improve it. Thanks. I'll work on the underlying drivers in order to get this going. It'll have to wait for 1.3, 'cos it affects the API (despite not breaking anything:-) Any comments in relation to apr_dbd_init situation (my other e-mail)? I think we should fix it by calling apr_dbd_init from apr_initialize. I guess that didn't happen originally 'cos of the sequencing of when things were developed and committed, and karma. -- Nick Kew
Re: Column names with apr_dbd
Quoting Nick Kew [EMAIL PROTECTED]: It'll have to wait for 1.3, 'cos it affects the API (despite not breaking anything:-) That's cool. Whoever (that would be me :-) wants the column names will be able to patch their own copy of APR before the 1.3 release. I think we should fix it by calling apr_dbd_init from apr_initialize. I guess that didn't happen originally 'cos of the sequencing of when things were developed and committed, and karma. Eh, I didn't think of that at all. Does it make any difference that DBD is part of Utils, not APR itself? What if someone doesn't want APR Utils, but wants APR - wouldn't that create a dependency of APR on APR Utils? Or maybe that's already the case... Anyhow, if that goes through, maybe the existing function apr_dbd_init can just be a do-nothing deprecated thingy (so that all exisiting callers don't have to rewrite their code) and apr_initialize actually calls something like apr_dbd_init2? -- Bojan
