Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9250 closed defect (wontfix)

fresh install rolls back TimingAndEstimationPlugin_Db_Version in system table in sqlite

Reported by: pks@… Owned by: bobbysmith007
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 follow-up: Changed 3 years ago by bobbysmith007

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 3 years ago by anonymous

  • Priority changed from normal to lowest

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 3 years ago by bobbysmith007

  • Resolution set to wontfix
  • Status changed from new to closed

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 3 years ago by bobbysmith007

(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

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.