On Fri, Sep 27, 2013 at 09:56:38PM +0100, Martin J. Evans wrote: > On 27/09/2013 21:01, Tim Bunce wrote: > >> > >>I don't see a way in DBI's pod to report this back i.e., there is no > >>return value mentioned. > >> > >>Should I just issue a warning if I get "option value changed - 01S02"? > > > >>The reason this has come up is that I have the following test: > >> > >> $dbh->{ReadOnly} = 1; > >> is($dbh->{ReadOnly}, 1, 'ReadOnly set'); > >> $dbh->{ReadOnly} = 0; > >> is($dbh->{ReadOnly}, 0, 'ReadOnly cleared'); > >> > >>and the SQLite ODBC driver is failing it because any setting of > >>SQL_ACCESS_MODE returns SQL_SUCCESS_WITH_INFO, option value changed, > >>01S02 and when you go to retrieve it back it is not what you set. > > > >By "issue a warning" do you mean set err to "0", errstr to "option value > >changed..." and state to "01S02"? If so, yes, that seems like the right > >thing to do. > > Yes, that is what I was proposing. > > >The test can then be updated to check for that. > > Exactly. > > >I'd be happy for you to patch the DBI docs along those lines. > > So the change would say that not all DBDs can necessarily set > ReadOnly and if they can't, they will issue a warning?
Yes, though "issue" isn't quite the right word. More like "record" a warning in err (that'll then get "issued" via PrintWarn mechanism). > As for changing the docs, I can issue a pull request as I opted out > of write access to DBI on github on the basis if I didn't trust > myself with git (which I didn't), neither should you - so Merijn > removed me. PR's are fine. Thanks. > >Whether $dbh->{ReadOnly} should remain false after an attempt to set > >it true has 'failed' seems more tricky. If it's false then other code > >can't tell that the application declared itself to not want to make > >changes. I'm inclined to let it stay true. > > Yes, good point but in this case, setting ReadOnly true results in > reading ReadOnly as false (as the driver could not set > SQL_ACCESS_MODE to true). So, any subsequent code reading ReadOnly > does not know the application attempted to set ReadOnly true. So, if > I've understood correctly then setting ReadOnly to true should > return true even if the driver could not do that (Option value > changed - 01S02). I can do this in DBD::ODBC but it requires > maintaining state that ReadOnly was set to true but not acted on in > the underlying driver - whereas what I do now is set > SQL_ACCESS_MODE, ignore whether it works or not and if someone asks > what SQL_ACCESS_MODE (ReadOnly) is I simply call SQLGetConnectAttr > which in this case returns false. > > In summary: > > 1. setting ReadOnly should warn if setting ReadOnly cannot be achieved. Yes (modulo the note above). > 2. If ReadOnly has been set (even if unsuccessfully in the driver) > true should be returned from $dbh->{ReadOnly} I think that's best. It should be simple to implement: just use the default ReadOnly attribute implementation but hook into the set. (ODBC could provide a private method to access the raw SQLGetConnectAttr API if needed.) Tim.