Re: Problem with ReadOnly attribute

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.