Modify

Opened 4 years ago

Closed 20 months ago

Last modified 20 months ago

#11015 closed defect (fixed)

db upgrade fails when using postgres backend

Reported by: kyle.james.oconnor@… Owned by: branson
Priority: normal Component: DirectoryAuthPlugin
Severity: major Keywords:
Cc: Ryan J Ollos Trac Release: 1.0

Description

Attempting to use Postgres 9.1 backend with this plugin version 1.0.2 (Trac 1.0, AccountManager 0.4.3) gives the following error:

The upgrade failed. Please fix the issue and try again.

ProgrammingError: type "blob" does not exist
LINE 4:     "data" blob

it looks like "blob" is not a valid type for postgres. If I replace "blob" with "bytea" (patch) it works but how do we handle this in a cross database way? I have to believe there is also code in the trac DB api that can handle these types but I could not find it.

Attachments (2)

postgres_blob_datatype.diff (422 bytes) - added by kyle.james.oconnor@… 4 years ago.
this is a hack
postgres_upsert.diff (6.1 KB) - added by kyle.james.oconnor@… 4 years ago.
emulates upsert

Download all attachments as: .zip

Change History (15)

Changed 4 years ago by kyle.james.oconnor@…

Attachment: postgres_blob_datatype.diff added

this is a hack

comment:1 Changed 4 years ago by anonymous

Trac Release: 1.0

comment:2 Changed 4 years ago by kyle.james.oconnor@…

Even after my hack to switch to 'bytea', this plugin fails spectacularly when using postgres as a backend. I've tried finagling things around for a while now but I'm not familiar with any of the specifics (of Trac, db api, or postgres) and it would appear that a lot of the SQL statements in this plugin are not compatible with Postgres.

In the least, the INSERT OR REPLACE INTO statements need to be replaced with something but I couldn't find a clean way to do it. Help would be appreciated as I would really like to use this plugin in my Trac deployments with Postgres.

comment:3 Changed 4 years ago by kyle.james.oconnor@…

As I mentioned above (and someone discovered the problem for MySQL too in #10632), Postgres does not support a built-in upsert statement. So I have replaced that with an update/insert (in postgres_upsert.diff) which should be more portable but I've only tested it on Postgres as of now. Still need to a solution to the blob/bytea though.

Changed 4 years ago by kyle.james.oconnor@…

Attachment: postgres_upsert.diff added

emulates upsert

comment:4 Changed 4 years ago by Ryan J Ollos

Kyle, I think you are on the right path with your patch. Since all of our recent improvements to the plugin have been reverted by the author, I'm not going to waste my time on this one anymore. I'd suggest just forking it if you continue to have trouble and the issues don't get fixed.

comment:5 Changed 4 years ago by kyle.james.oconnor@…

Okay...well that is a bummer but thanks for the heads up. I was maintaining my own version to write and test the changes so I'll just stick with that. Thanks for your assistance!

comment:6 Changed 4 years ago by Ryan J Ollos

It is particularly sad that we have such a completely broken plugin on trac-hacks and the author reverts fundamental fixes like [12776]/#10618, without even taking a moment to communicate with us. That defect shows that the developer hasn't taken the least bit of care in testing the plugin before posting it to the community, and to revert such a fundamental fix shows a complete lack of concern for others using the plugin.

Regarding your patch, the only concern I had with postgres_upsert.diff was that WHERE NOT EXISTS doesn't work on SQLite, as far as I can see (haven't tested the code). The solution there might be to just pull some of the SQL logic out into the Python code, like the PrivateCommentsPlugin does.

I have no clue what a good cross-db replacement for the blob data type is, but you might want to ask on the t:MailingList.

comment:7 Changed 20 months ago by bebbo

In 14830:

refs #12068
refs #11686
refs #11361
refs #11307
refs #11304
refs #10878
refs #10715
refs #10667
refs #10632
refs #10631
refs #11015

various fixes

comment:8 Changed 20 months ago by bebbo

Resolution: fixed
Status: newclosed

added a separate schema for PostreSQL

comment:9 Changed 20 months ago by Ryan J Ollos

With proper use of the Trac database API it shouldn't be necessary to define different schemas for each database type. See trac:TracDev/DatabaseApi for more info.

comment:10 Changed 20 months ago by bebbo

Uhm, I checked this resource and found nothing for binary blobs.

The code then was inspired by trac/upgrades/db16.py

comment:11 Changed 20 months ago by Ryan J Ollos

You'd need to figure out how to design the table to avoid blob types.

comment:12 Changed 20 months ago by bebbo

In 14838:

refs #11015

using text instead of blob...

comment:13 Changed 20 months ago by bebbo

In 14839:

refs #11015

reverting r14838 ... does not work

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain branson.
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.