Re: [snafu] isolation-level change in 2.4.2 - Mailing list psycopg
| From | Daniele Varrazzo |
|---|---|
| Subject | Re: [snafu] isolation-level change in 2.4.2 |
| Date | |
| Msg-id | CA+mi_8afwMw1qQu5ZgUO5MpFYd1=qXa+-4o0movMrzwTCfBPiQ@mail.gmail.com Whole thread Raw |
| In response to | Re: [snafu] isolation-level change in 2.4.2 (Federico Di Gregorio <fog@dndg.it>) |
| Responses |
Re: [snafu] isolation-level change in 2.4.2
(Federico Di Gregorio <fog@dndg.it>)
Re: [snafu] isolation-level change in 2.4.2 (Marko Kreen <markokr@gmail.com>) Re: [snafu] isolation-level change in 2.4.2 (Federico Di Gregorio <fog@dndg.it>) |
| List | psycopg |
On Wed, Dec 14, 2011 at 2:03 PM, Federico Di Gregorio <fog@dndg.it> wrote: > On 14/12/11 14:56, Marko Kreen wrote: >> >> In 2.4.2 you shuffled numeric codes for isolation levels freely, >> because "everybody should be using symbolic codes". >> >> Generally that would be true, except this case: psycopg 1.x >> did not have symbolic codes, so numeric codes were >> part of public API. So old code that were written >> for psycopg1 will not work anymore with psycopg2. >> >> What makes is especially bad is that such code will not fail >> clearly, but instead will result in silent data corruption. >> We experienced that with Skytools. >> >> Also note that even in psycopg2, the .set_isolation_level() >> is part of core API, but the constants are under psycopg2.extensions. >> So even pure-2.x code may be using numerical constants. >> >> >> I suggest 2 ways to fix it properly: >> >> 1) Use numeric codes out of range compared to 1.x: >> >> ISOLATION_LEVEL_AUTOCOMMIT = 10 >> ISOLATION_LEVEL_READ_UNCOMMITTED = 11 >> ISOLATION_LEVEL_READ_COMMITTED = 12 >> ISOLATION_LEVEL_REPEATABLE_READ = 13 >> ISOLATION_LEVEL_SERIALIZABLE = 14 >> >> and give errors to old codes - that would give clear detection >> that somebody is using old codes. >> >> 2) Use codes that are compatible with 1.x: >> >> ISOLATION_LEVEL_AUTOCOMMIT = 0 >> ISOLATION_LEVEL_READ_UNCOMMITTED = 1 >> ISOLATION_LEVEL_READ_COMMITTED = 1 >> ISOLATION_LEVEL_REPEATABLE_READ = 2 >> ISOLATION_LEVEL_SERIALIZABLE = 3 >> >> the REP_READ=2, SER=3 change is deliberate, because before 9.1 >> they were equal, and in 9.1+ the REP_READ corresponds >> to old SER. The SER level in 9.1 is new, and unlikely >> to be expected by old code. >> >> I would suggest even releasing 2.4.4 quite soon with >> this fixed, to avoid anybody else tripping on this bug. > > > You're right. I rather like (2) and I'll do the fix unless Daniele has a > very good reason to go with (1). Honestly this looks much more a bug in skytools than in psycopg for me. The use of symbolic constants is there exactly to allow these values to change. However, my fault: I've never maintained psycopg 1, so I wasn't aware the numbers had ever been part of the api. The only widespread use of a numeric value I've seen (including many official psycopg2 examples) is for autocommit=0, so when I've changed the values I've cared to keep that value stable only. For this reason, my preference between the solutions goes to 2 too (in the unlikely case READ UNC started meaning anything for Postgres we can differentiate it from 1). Are you making the patch or shall I do it myself? If you change the values, take care of this check too: <https://github.com/dvarrazzo/psycopg/blob/devel/psycopg/connection_int.c#L1042>. In particular, before releasing, let me run the test against all the supported PG versions: these branch are exhaustively checked. -- Daniele