Modify

Opened 3 years ago

Closed 10 months ago

Last modified 10 months ago

#11015 closed defect (fixed)

db upgrade fails when using postgres backend

Reported by: kyle.james.oconnor@… Owned by: sandinak
Priority: normal Component: DirectoryAuthPlugin
Severity: major Keywords:
Cc: rjollos 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@… 3 years ago.
this is a hack
postgres_upsert.diff (6.1 KB) - added by kyle.james.oconnor@… 3 years ago.
emulates upsert

Download all attachments as: .zip

Change History (15)

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

this is a hack

comment:1 Changed 3 years ago by anonymous

  • Trac Release set to 1.0

comment:2 Changed 3 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 3 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 3 years ago by kyle.james.oconnor@…

emulates upsert

comment:4 Changed 3 years ago by rjollos

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

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 10 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 10 months ago by bebbo

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

added a separate schema for PostreSQL

comment:9 Changed 10 months ago by rjollos

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 10 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 10 months ago by rjollos

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

comment:12 Changed 10 months ago by bebbo

In 14838:

refs #11015

using text instead of blob...

comment:13 Changed 10 months ago by bebbo

In 14839:

refs #11015

reverting r14838 ... does not work

Add Comment

Modify Ticket

Action
as closed The owner will remain sandinak.
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.