Re: the return of tables ()

On Mon, 1 Sep 2008 12:07:53 +0100, Tim Bunce <[email protected]>
wrote:

> On Mon, Sep 01, 2008 at 08:17:38AM +0200, H.Merijn Brand wrote:
> > On Sun, 31 Aug 2008 22:52:32 +0100, Tim Bunce <[email protected]>
> > wrote:
> > 
> > > On Wed, Aug 20, 2008 at 07:38:36PM +0200, H.Merijn Brand wrote:
> > > > In my quest of improving on DBD::Unify, I implemented - as per DBI
> > > > documentation suggestion:
> > > > 
> > > > "Other database handle methods
> > > > 
> > > >  As with the driver package, other database handle methods may follow
> > > >  here. In particular you should consider a (possibly empty) disconnect ()
> > > >  method and possibly a quote () method if DBI's default isn't correct for
> > > >  you. You may also need the type_info_all () and get_info () methods, as
> > > >  described elsewhere in this document."
> > > > 
> > > > the methods get_info () and type_info_all (), which I generated on Windows
> > > > with Strawberry perl as that is the only place where I got ODBC working
> > > > in a somewhat reliable way, exactly like the docs show me. Note here that
> > > > 
> > > > http://search.cpan.org/~timb/DBI-1.607/lib/DBI/DBD.pm#Generating_the_get_info_method
> > > > 
> > > > shows the perl command without quotes and parens, so it is completely
> > > > useless as an example
> > > > 
> > > > perl -MDBI::DBD::Metadata -we "write_getinfo_pm (qw{ dbi:ODBC:foo_db username password Driver })"
> > > > 
> > > > would be a portable solution to not mix up the quotes on WinShit
> > > 
> > > Patches welcome. Want a commit bit (if you don't have one already)?
> > 
> > These are the patches I feel comfortable with, as they obviously will
> > not clash with *opinions*. Is the repo in git? in that case, I'll take
> > the bit. If it is in svn, I'll pass.
> 
> svn, pity.

OK, patch:
--8<---
diff -pu lib/DBI/DBD.pm.org lib/DBI/DBD.pm
--- lib/DBI/DBD.pm.org  2008-09-01 15:22:45.000000000 +0200
+++ lib/DBI/DBD.pm      2008-09-01 15:24:48.000000000 +0200
@@ -2632,8 +2632,8 @@ database.

 With the pre-requisites in place, you might type:

-    perl -MDBI::DBD::Metadata -e write_getinfo_pm \
-            dbi:ODBC:foo_db username password Driver
+    perl -MDBI::DBD::Metadata -we \
+       "write_getinfo_pm (qw{ dbi:ODBC:foo_db username password Driver })"

 The procedure writes to standard output the code that should be added to
 your F<Driver.pm> file and the code that should be written to
@@ -2657,8 +2657,8 @@ L</Generating the get_info method>.

 With the pre-requisites in place, you might type:

-    perl -MDBI::DBD::Metadata -e write_typeinfo \
-            dbi:ODBC:foo_db username password Driver
+    perl -MDBI::DBD::Metadata -we \
+       "write_typeinfo (qw{ dbi:ODBC:foo_db username password Driver })"

 The procedure writes to standard output the code that should be added to
 your F<Driver.pm> file and the code that should be written to
-->8---

> > > > Anyway, I put the generated files in place and my tests started to fail.
> > > > I did expect some fails, but not this one:
> > > > 
> > > > As get_info (29) now returns a TRUE value, the 'tables ()' method is
> > > > using a different strategy to build the list it returns:
> > > > 
> > > >     sub tables
> > > >     {
> > > >         my ($dbh, @args) = @_;
> > > >         my $sth    = $dbh->table_info (@args[0..4]) or return;
> > > >         my $tables = $sth->fetchall_arrayref or return;
> > > >         my @tables;
> > > > »       if ($dbh->get_info (29)) { # SQL_IDENTIFIER_QUOTE_CHAR
> > > > »           @tables = map { $dbh->quote_identifier (@{$_}[0,1,2]) } @$tables;
> > > > »           }
> > > >         else {          # temporary old style hack (yeach)
> > > >             @tables = map {
> > > >                 my $name = $_->[2];
> > > >                 if ($_->[1]) {
> > > >                     my $schema = $_->[1];
> > > >                     # a sad hack (mostly for Informix I recall)
> > > >                     my $quote = ($schema eq uc $schema) ? '' : '"';
> > > >                     $name = "$quote$schema$quote.$name";
> > > >                     }
> > > >                 $name;
> > > >                 } @$tables;
> > > >             }
> > > >         return @tables;
> > > >         } # tables
> > > > 
> > > > With a true value for get_info (29), tables () uses the first block,
> > > > where it used to use the second block.
> > > > 
> > > > Unify has no support for CATALOG's, so the values in info are not
> > > > defined, but still used in the map, causing all my tables no showing up
> > > > with and empty "". in front of it, which is illegal to the database :(
> > > 
> > > Seems like your quote_identifier() method is doing the wrong thing.
> > > The docs for quote_identifier say:
> > 
> > I do not have a quote_identifier () method. I rely on DBI doing the
> > right thing.
> 
> I believe the DBI's quote_identifier does the right thing.
> 
> I'd guess that the CATALOG value returned by table_info is an empty
> string but should be an undef.

Not defined at all, so I *guess* that is the same as undefined. Look,
these are all commented out:

lib/DBD/Unify/GetInfo.pm:#   114 => 0,     # SQL_CATALOG_LOCATION
lib/DBD/Unify/GetInfo.pm:# 10003 => undef, # SQL_CATALOG_NAME
lib/DBD/Unify/GetInfo.pm:#    41 => "",    # SQL_CATALOG_NAME_SEPARATOR
lib/DBD/Unify/GetInfo.pm:#    42 => "",    # SQL_CATALOG_TERM
lib/DBD/Unify/GetInfo.pm:#    92 => 0,     # SQL_CATALOG_USAGE
lib/DBD/Unify/GetInfo.pm:#    34 => 18,    # SQL_MAXIMUM_CATALOG_NAME_LENGTH
lib/DBD/Unify/GetInfo.pm:#    34 => 18,    # SQL_MAX_CATALOG_NAME_LEN

> > IIRC quote_identifier () was not in the list of required
> > methods for DBD
> 
> It's not required unless the DBI's default doesn't do thing right thing
> for a particular driver.

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/