Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9250 closed defect (wontfix)

fresh install rolls back TimingAndEstimationPlugin_Db_Version in system table in sqlite

Reported by: pks@… Owned by: Russ Tyndall
Priority: lowest Component: TimingAndEstimationPlugin
Severity: normal Keywords: install upgrade dbhelper sqlite
Cc: Trac Release: 0.12

Description

to reproduce:

  • create a new environment with sqlite database
  • follow setup instructions
    • trac-admin /some/env upgrade output looks good
  • run trac-admin /some/env upgrade a second time
    • upgrade assumes a full install is needed for lack of TimingAndEstimationPlugin_Db_Version
    • upgrade fails creating the billing table a second time

narrowed down cause:

  • dbhelper::db_table_exists() called by CustomReportManager::upgrade() somehow causes the system table entry for TimingAndEstimationPlugin_Db_Version to be rolled back.
  • since it's a fresh install there is no custom_report table
  • dbhelper::db_table_exists() correctly returns false in the response to the exception for select from the non-existent table - but has the side effect of rolling back
  • absence of the db version entry causes upgrade checks to assume full install is required again

FWIW:

trac 0.12.2
python 2.5.2
sqlite 3.4.2

We will live for now - since we'll be using sqlite for the foreseeable future, we just made dbhelper::db_table_exists() return early as follows:

def db_table_exists(env,  table):
    return get_scalar(env, ("select count(*) from sqlite_master where type = 'table' and name = '%s'" % (table))) > 0;

Obviously not a viable solution, but could be made conditional if

  • dbhelper can query database backend
  • nested transactions as used by dbhelper fail only for sqlite

As such it would serve as a work around for many environments

Sorry if notation is substandard - spent half as much time looking at python code today as I have in entire my career prior - when I read 'dive into python' years ago ;)

Thanks for a plugin worth debugging!

Attachments (0)

Change History (4)

comment:1 Changed 6 years ago by Russ Tyndall

Thanks very much for the detailed bug report!

dbhelper::db_table_exists() called by CustomReportManager::upgrade() somehow causes the system table entry for TimingAndEstimationPlugin_Db_Version to be rolled back.

I am unclear on how this could occur, but I will definitely look into it further.

dbhelper::db_table_exists() correctly returns false in the response to the exception for select from the non-existent table - but has the side effect of rolling back

This should only be rolling back to the save point but... I found a quote on the internet claiming "SAVEPOINT first appeared in SQLITE V 3.6.8" so this is very likely the ultimate cause of the issues.


I will look at special casing this function for sqlite perhaps, or investigate if this function (table_exists) appears in trac now.

I'm glad you were able to get it working for you on your version of sqlite

Cheers,
Russ

comment:2 in reply to:  1 Changed 6 years ago by anonymous

Priority: normallowest

Replying to bobbysmith007:

This should only be rolling back to the save point but... I found a quote on the internet claiming "SAVEPOINT first appeared in SQLITE V 3.6.8" so this is very likely the ultimate cause of the issues.

Right on! Just tried a vanilla install with sqlite 3.7.4 and problem did not manifest.

I will look at special casing this function for sqlite perhaps, or investigate if this function (table_exists) appears in trac now.

Much appreciated - but your reference to version info had me look at sqlite release history and find that

  • 3.4.2 (used) is over four years old now
  • 3.6.8 (required) was released over two and half years ago
  • 3.7.4 (tested) fine is going on a year old

I'm not really qualified to guess how common such old installations are, but 4+ years makes me wish I hadn't made an assertion about a potential fix to 'many' installations.

If I were the only developer (or one of two/three) on a project and this 'issue' came up, I would probably

  • recommend that the reporter move on with life every other year or so
  • note a (quite reasonable) 3.6.8+ requirement for sqlite backed environments in the docs
  • resolve: wontfix
  • get back to what I was doing before I got pestered

That said, I'm not the developer ... just making sure I don't waste yet more of your time by sending you down a rabbit hole for ticket system public relations' sake ...

Thanks much again for your time, and yet again for the plugin!

Patrick

comment:3 Changed 6 years ago by Russ Tyndall

Resolution: wontfix
Status: newclosed

You rock! I updated the docs noting the increased sqlite3 version requirement.

Thanks for your verifying the fix and for your understanding, I am heading back to what I was doing :)

Cheers,

Russ

comment:4 Changed 6 years ago by Russ Tyndall

(In [10766]) Changed def of table_exists to special case sqlite, hopefully will prevent problems with nested transactions <ver 1.2.0> re #9259 and #9250

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Russ Tyndall.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.