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 .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from bobbysmith007. Next status will be '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.