Re: thorny problem with table_info implementation
On Mon, Sep 02, 2013 at 08:53:18PM +0100, Martin J. Evans wrote:
On 02/09/2013 20:27, Martin J. Evans wrote:
Now, table_info() with no arguments is not mentioned in the DBI pod so
you could say that is undefined behaviour but it is a little late for
that now as DBD::ODBC uses it in test code and as far as I know, so do
others. However, to fix the initial bug I cannot know when empty strings
are supposed to be undef/NULL and when they are supposed to be empty
strings. So I can fix the bug as shown but only if I change DBD::ODBC
test code from table_info() to table_info('%', '%', '%', '%') and I
cannot change anyone elses code.
Correction, in that last paragraph (I did warn you), I should have
said table_info() to table_info('%', '%', '%') i.e.,
table_info('%', '', '') returns catalogs only
table_info('', '%', '') returns schemas only
table_info('', '', '', '%') returns types only
table_info('%', '%', '%') returns everything
table_info() did return everything due to workaround in DBD::ODBC
but fixing bug ends up in table_info() returning nothing.
How about changing your test code to table_info('%', '%', '%', '%')
and treat table_info() a special case that triggers a warning?
Tim.
Re: thorny problem with table_info implementation
On 03/09/13 09:38, Tim Bunce wrote:
On Mon, Sep 02, 2013 at 08:53:18PM +0100, Martin J. Evans wrote:
On 02/09/2013 20:27, Martin J. Evans wrote:
Now, table_info() with no arguments is not mentioned in the DBI pod so
you could say that is undefined behaviour but it is a little late for
that now as DBD::ODBC uses it in test code and as far as I know, so do
others. However, to fix the initial bug I cannot know when empty strings
are supposed to be undef/NULL and when they are supposed to be empty
strings. So I can fix the bug as shown but only if I change DBD::ODBC
test code from table_info() to table_info('%', '%', '%', '%') and I
cannot change anyone elses code.
Correction, in that last paragraph (I did warn you), I should have
said table_info() to table_info('%', '%', '%') i.e.,
table_info('%', '', '') returns catalogs only
table_info('', '%', '') returns schemas only
table_info('', '', '', '%') returns types only
table_info('%', '%', '%') returns everything
table_info() did return everything due to workaround in DBD::ODBC
but fixing bug ends up in table_info() returning nothing.
How about changing your test code to table_info('%', '%', '%', '%')
because that does not work. It would need to be table_info('%','%','%') (or
table_info(undef, undef, undef, undef) for it to work. In ODBC the table type
needs to be null or all types to get everything. Obviously changing the test
code is easy.
and treat table_info() a special case that triggers a warning?
hmm, I warned I might be having a bad day. As soon as I read that I thought how
do I do that as I'd forgotten there is a table_info method in DBD::ODBC itself.
It appears all the problems are DBD::ODBC specific as I missed the code that
set the parameters to q{} if not defined.
Tim.
Thanks and sorry for noise.
Martin
Re: thorny problem with table_info implementation
On 03/09/2013 11:43, Martin J. Evans wrote:
On 03/09/13 09:38, Tim Bunce wrote:
On Mon, Sep 02, 2013 at 08:53:18PM +0100, Martin J. Evans wrote:
On 02/09/2013 20:27, Martin J. Evans wrote:
Now, table_info() with no arguments is not mentioned in the DBI pod so
you could say that is undefined behaviour but it is a little late for
that now as DBD::ODBC uses it in test code and as far as I know, so do
others. However, to fix the initial bug I cannot know when empty
strings
are supposed to be undef/NULL and when they are supposed to be empty
strings. So I can fix the bug as shown but only if I change DBD::ODBC
test code from table_info() to table_info('%', '%', '%', '%') and I
cannot change anyone elses code.
Correction, in that last paragraph (I did warn you), I should have
said table_info() to table_info('%', '%', '%') i.e.,
table_info('%', '', '') returns catalogs only
table_info('', '%', '') returns schemas only
table_info('', '', '', '%') returns types only
table_info('%', '%', '%') returns everything
table_info() did return everything due to workaround in DBD::ODBC
but fixing bug ends up in table_info() returning nothing.
How about changing your test code to table_info('%', '%', '%', '%')
because that does not work. It would need to be table_info('%','%','%')
(or table_info(undef, undef, undef, undef) for it to work. In ODBC the
table type needs to be null or all types to get everything. Obviously
changing the test code is easy.
and treat table_info() a special case that triggers a warning?
hmm, I warned I might be having a bad day. As soon as I read that I
thought how do I do that as I'd forgotten there is a table_info method
in DBD::ODBC itself. It appears all the problems are DBD::ODBC specific
as I missed the code that set the parameters to q{} if not defined.
Tim.
Thanks and sorry for noise.
Martin
Pushed changes to github and will try and find time to release updates
tomorrow. This started from
http://stackoverflow.com/questions/18450002/retrieving-available-table-types-schemas-and-catalogs-using-dbiodbc-table-i.
This actually found a bug in the easysoft sql server odbc driver re:
calling SQLTables with % as the table_type - ask easysoft support for an
updated driver if you need this fix. If no complaints I will release a
new DBD::ODBC with new test cases in the next week.
Martin
--
Martin J. Evans
Wetherby, UK
Re: thorny problem with table_info implementation
On 02/09/2013 20:27, Martin J. Evans wrote:
Hi,
Firstly, I've just come back from holidays, I'm distracted by other
things right now and might not be back in the swing of things properly -
so beware.
table_info maps to the ODBC API SQLTables in DBD::ODBC. DBI seems to
have picked some functionality for table_info from ODBC in that if '%'
is passed for one of catalog, schema and table and the other 2 are empty
strings the result only contains a list of catalogs, schemas or tables.
Someone just reported to me that in DBD::ODBC if you call
table_info('%','','') you get all catalogs, schemas and tables back but
you should get only a list of catalogs. I tracked this down to the
following code in DBD::ODBC (ANSI case here, unicode one is slightly
more complex):
if (SvOK(catalog)) acatalog = SvPV_nolen(catalog);
if (SvOK(schema)) aschema = SvPV_nolen(schema);
if (SvOK(table)) atable = SvPV_nolen(table);
if (SvOK(table_type)) atype = SvPV_nolen(table_type);
rc = SQLTables(imp_sth-hstmt,
(acatalog *acatalog) ? acatalog : NULL,SQL_NTS,
(aschema *aschema) ? aschema : NULL, SQL_NTS,
(atable atable) ? atable : NULL, SQL_NTS,
(atype *atype) ? atype : NULL,
SQL_NTS/* type (view, table, etc) */
);
What is happening here is that whatever is passed to table_info, if it
is defined and NOT the empty string we pass the string else NULL and for
SQLTables NULL is very different from the empty string (see special
cases above). However, if you call:
$dbh-table_info('%', '', '') to just get catalogs what is passed to
SQLTables is '%', NULL, NULL and this causes all tables to be returned
instead of only catalogs.
The fix seems obvious, remove the test for *acatalog etc which I did and
it fixes the problem \o/
except, DBD::ODBC test code calls table_info() (and for all I know other
people do too) and that ends up in DBI calling table_info with 4 empty
strings. Empty strings are not the same as undef/NULL and it results in
SQLTables returning no tables at all when before it would return all
tables.
As far as I can see, at some point in the past, someone (maybe me)
realised this and changed DBD::ODBC to workaround this issue instead of
questioning what DBI passes.
Now, table_info() with no arguments is not mentioned in the DBI pod so
you could say that is undefined behaviour but it is a little late for
that now as DBD::ODBC uses it in test code and as far as I know, so do
others. However, to fix the initial bug I cannot know when empty strings
are supposed to be undef/NULL and when they are supposed to be empty
strings. So I can fix the bug as shown but only if I change DBD::ODBC
test code from table_info() to table_info('%', '%', '%', '%') and I
cannot change anyone elses code.
Any suggestions?
Martin
Correction, in that last paragraph (I did warn you), I should have said
table_info() to table_info('%', '%', '%') i.e.,
table_info('%', '', '') returns catalogs only
table_info('', '%', '') returns schemas only
table_info('', '', '', '%') returns types only
table_info('%', '%', '%') returns everything
table_info() did return everything due to workaround in DBD::ODBC but
fixing bug ends up in table_info() returning nothing.
Martin
--
Martin J. Evans
Wetherby, UK
