On Wed, 12 Dec 2012 16:31:44 +0100, Jens Rehsack <rehs...@cpan.org> wrote:
> On 01.12.12 23:14, Tim Bunce wrote: > > On Thu, Nov 29, 2012 at 10:15:19PM +0100, Jens Rehsack wrote: > >> > >> But back to the issue - now it seems dbm_tables is fetched earlier > >> in some cases than f_dir which causes an invocation of > >> DBI::DBD::SqlEngine::Table::get_table_meta (including > >> DBI::DBD::SqlEngine::Table::bootstrap_table_meta) before f_dir > >> has been set. This causes $dbh->{sql_meta}->{fred}->{f_dir} being > >> initialized to the default value of $dbh->{f_dir} which is > >> always cur_dir(). > >> > >> Looking into DBI::DBD::SqlEngine::dr::connect around line 180 > >> it could be seen that there's already some magic for "some > >> settings must be done before others". > >> > >> A quick fix for "now" could be: keys: ("sql_meta", $dbh->{dbm_meta}, > >> ...) must be initialized last - after any other k/v pair from %$attrs. > >> Another quick should could be forbid setting meta info during connect(), > >> as it's documented - but this would be a hugh step backwards in my > >> effort making DBI::DBD::SqlEngine and derived DBD's usable through > >> Gofer proxy. So I'd prefer the first quick shot ... > >> > >> For longer way, I'd like to refactor the order procedure to a > >> more flexible way: > >> $dbh->{dbd_init_order} = [ > >> [qw(Profile RaiseError PrintError AutoCommit)], > >> [qw(ReadOnly ...)], > >> ... > >> [qw(sql_meta dbm_tables ...)] > >> ]; > >> > >> Remaining question in that proposal: where's the fence between "must > >> be last" and "insert unnamed attrs here"? > > > > I don't know, but I would like a working DBI for 5.17.6+ before too long. > > Well, we developed two patches for now: > > 1) Merijn's patch - http://pasta.test-smoke.org/385 > > + looks simple, easy to understand + easy to extend + easy to override + one single point of maintenance > - makes assumptions about attribute names of derived DBD's > (e.g. dbm_tables, but this can be easily renamed by assigning > new name for it to $dbh->{dbm_meta} (by driver author), eg. > $dbh->{dbm_meta} = "dbm_sources") which makes me/us/you want more documentation about what DBI/DBD guarantees, expects, supports etc. there is no real "you should do this" or "this is officially off-bound" > - increases complexitiy when more issues came up with similar > patterns (eg. /_meta$/ can't be set from outside, but from > derived driver) but still offers easy documentation for every single entry > 2) Jens' patch - http://pasta.test-smoke.org/387 > > + backward compatible to ancient attributes (csv_file, csv_ext, ...) though I value this point, we are talking about DBD's we all control. The newest DBD::CSV and DBD::DBM will require the new DBI so conflicts are not likely to happen. To be honest, I - as maintainer of DBD::CSV - didn't even know csv_file was still supported > + no assumptions on derived DBD's a very good point > + no false positives on pattern match if everything is well-documented, then false positives should not happen or better cause havoc > - more complicated for quick review can be solved by well-chosen inline docs - This patch will probably need to have extra code in (many) other places, which my patch tried to avoid > Any other +/- comments? Yes please, don't feel shy! Tell us! > In general both versions would fix the root cause of RT#81516, > while the maintainers (Merijn, me) cannot choose the right one. What he said > Jens -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.17 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/