#8065 closed defect (duplicate)
Database schema is incorrect or casts are needed (PostgreSQL 8.4 database)
Reported by: | dbely | Owned by: | Robert Corsaro |
---|---|---|---|
Priority: | normal | Component: | AnnouncerPlugin |
Severity: | critical | Keywords: | patch database API |
Cc: | Steffen Hoffmann | Trac Release: | 0.12 |
Description
The database schema you use during upgrade seems to have incorrect field types:
SCHEMA = [ Table('subscription', key='id')[ Column('id', auto_increment=True), Column('time', type='int64'), Column('changetime', type='int64'), Column('class'), Column('sid'), Column('authenticated', type='int'), Column('distributor'), Column('format'), Column('priority', type='int'), Column('adverb') ], Table('subscription_attribute', key='id')[ Column('id', auto_increment=True), Column('sid'), Column('authenticated', type='int'), Column('class'), Column('realm'), Column('target') ] ]
Specifically:
- What type should have 'authenticated' field? It's declared as int but used as bool. Due to that in many places I have a crash like following:
File "/usr/local/lib/python2.6/dist-packages/TracAnnouncer-0.12.1.dev-py2.6.egg/announcer/model.py", line 350, in do_select """, (sid,authenticated,klass)) File "/usr/lib/python2.6/dist-packages/trac/db/util.py", line 122, in execute return self.cursor.execute(sql_escape_percent(sql), args) ProgrammingError: operator does not exist: integer = boolean LINE 5: AND authenticated=true ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
Or maybe the filed type is correct but explicit type casts should be added indeed?
- What type should have 'time' fields?
File "/usr/local/lib/python2.6/dist-packages/TracAnnouncer-0.12.1.dev-py2.6.egg/announcer/model.py", line 79, in do_insert subscription['class'])) File "/usr/lib/python2.6/dist-packages/trac/db/util.py", line 122, in execute return self.cursor.execute(sql_escape_percent(sql), args) ProgrammingError: column "time" is of type bigint but expression is of type timestamp with time zone LINE 5: VALUES (CURRENT_TIMESTAMP, CURRENT_TIMESTAM... ^ HINT: You will need to rewrite or cast the expression.
I have fixed that with
VALUES (EXTRACT(EPOCH FROM CURRENT_TIMESTAMP), EXTRACT(EPOCH FROM CURRENT_TIMESTAMP), %s, %s, %s, %s, %s, %s, %s)
But I don't know if it's the correct way.
Probably there are other similar bugs that should be fixed as well.
Attachments (3)
Change History (17)
comment:1 follow-up: 2 Changed 14 years ago by
Changed 14 years ago by
Attachment: | fix_postgres_8_4_issues.patch added |
---|
comment:2 Changed 14 years ago by
Replying to olistudent:
I tried to fix the issues you mentioned above. Attached patch is what I'm running at the moment. Don't know if I catched up all errors...
btw. The authenticated field in trac's session table is int, too.
Thanks. This part looks like to be working. Since then I've found another similar problem: "delete ticket <number>" command crashes trac-admin with error
File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.2dev_r10363-py2.6.egg/trac/ticket/admin.py", line 841, in do_remove ticket.delete() File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.2dev_r10363-py2.6.egg/trac/ticket/model.py", line 421, in delete listener.ticket_deleted(self) File "/usr/local/lib/python2.6/dist-packages/TracAnnouncer-0.12.1.dev-py2.6.egg/announcer/subscribers.py", line 833, in ticket_deleted self.env, klass, 'ticket', ticket.id) File "/usr/local/lib/python2.6/dist-packages/TracAnnouncer-0.12.1.dev-py2.6.egg/announcer/model.py", line 326, in delete_by_class_realm_and_target @env.with_transaction(db) File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.2dev_r10363-py2.6.egg/trac/db/api.py", line 73, in transaction_wrapper fn(ldb) File "/usr/local/lib/python2.6/dist-packages/TracAnnouncer-0.12.1.dev-py2.6.egg/announcer/model.py", line 334, in do_delete """, (realm, klass, target)) File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.2dev_r10363-py2.6.egg/trac/db/util.py", line 65, in execute return self.cursor.execute(sql_escape_percent(sql), args) ProgrammingError: operator does not exist: text = integer LINE 5: AND target = 508 ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. ProgrammingError: operator does not exist: text = integer
I have fixed this with
def delete_by_class_realm_and_target(cls, env, klass, realm, target, db=None): @env.with_transaction(db) def do_delete(db): cursor = db.cursor() cursor.execute(""" DELETE FROM subscription_attribute WHERE realm = %s AND class = %s AND target = %s - """, (realm, klass, target)) + """, (realm, klass, str(target)))
but probably that's not the only place affected.
Changed 14 years ago by
Attachment: | fix_postgres_8_4_alternate_nosqlite.patch added |
---|
Alternate patch that should work on everything except SQLite
comment:3 Changed 14 years ago by
One problem with olistudent's patch is I don't think EXTRACT(EPOCH...) works on anything but Postgres. I've attached an alternate patch that calculates times the same way that tickets do.
The problem with it is my patch uses a table type of "boolean", which the Trac db adapter for SQLite currently can't handle - SQLite turns booleans into numeric values, and the adapter can't cast them back.
Changed 14 years ago by
Attachment: | fix_postgres_8_4_alternate_nosqlite.2.patch added |
---|
Updated (merged) patch that should work on all DBs, and fixes the error on ticket delete
comment:4 Changed 14 years ago by
I've merged olistudent's changes together with mine and uploaded it as a new patch, which should:
- Work on all DBs
- Fix the ticket delete problem (which was caused by trying to refer to page.name instead of ticket.id within the Subscriber)
comment:5 follow-ups: 7 10 14 Changed 12 years ago by
Hi, I have all those problems and the proposed patch corrects all. Why doesn't patch the code ? I have tried to commit but it's forbidden...
comment:7 Changed 12 years ago by
Cc: | Steffen Hoffmann added; anonymous removed |
---|
Replying to sgeulette:
Hi, I have all those problems and the proposed patch corrects all. Why doesn't patch the code ? I have tried to commit but it's forbidden...
The plugin hasn't been maintained in some time, but now the goal must be to make sure we are fixing it the correct way, and that it works on all supported DBs, rather than pushing a fix which corrects one problem and creates another. For that, you will have to bear with us, while we make sure to get the correct fix in place. We appreciate your patience and feedback when we get a fix in place.
comment:8 follow-up: 9 Changed 12 years ago by
Resolution: | → duplicate |
---|---|
Status: | new → closed |
There are 3 issues discussed in this ticket:
- The
authenticated
variable is not properly cast from abool
to anint
inmodel.py
. This issue is documented in #10384. - There is concern that non-naive timestamps are not correctly cast to
bigint
. This issue is documented in #7975. - Error deleting tickets, which also appears to be a casting problem. This issue is documented in #8577.
Since all of the issues in this ticket have been captured at finer granularity in other tickets, this one is being closed as a duplicate, and concerned parties are asked to follow the cited tickets.
comment:9 Changed 12 years ago by
Keywords: | patch database API added |
---|
Replying to rjollos: ...
Since all of the issues in this ticket have been captured at finer granularity in other tickets, this one is being closed as a duplicate, and concerned parties are asked to follow the cited tickets.
Thanks Ryan for starting ticket triage and linking related issues for ease of tracking issues down to the root cause and resolving it.
Ok, but should anyone still want to re-open, please change ticket owner to myself, or expect no response for quite a long time.
comment:10 Changed 12 years ago by
Replying to sgeulette:
Hi, I have all those problems and the proposed patch corrects all. Why doesn't patch the code ? I have tried to commit but it's forbidden...
Almost there. I really appreciate the work done in this ticket. The fix is quite complete, but i.e. it doesn't care for existing wrong values in the Trac db table, right? So this fix has to go in concert with a clean-up script to set a known-good state. I'll have it ready in a few days as part of a major db code rework. Stay tuned.
comment:11 Changed 12 years ago by
(In [12302]) TracAnnouncer: Part 7 of 7 - Finally: Go from present to future, refs #5774, #7975, #8065, #9742 and #10384.
Now we've bridged the gap and provide an upgrade path for each historic schema
revision of this plugin, while data migration is incomplete yet, especially
regarding subscription attributes stored in session_attribute
(before v3).
Due to component name changes the conversion will be rather complicated, and
therefore corresponding research and development is postponed to a future date
and will largely depend on explicit demand as well.
(Due to erroneously writing #8056 this didn't get here.)
comment:12 Changed 12 years ago by
(In [12303]) TracAnnouncer: Convert type to match db table definitions, refs #7975, #8065 and #10384.
These changes are based on work by olistudent, Stephen Anderson and rea. I made sure, that we respect PEP8 as well, at least as far as Trac core does.
Thanks to all of you for testing, reports and suggestions towards a portable fix, and - ultimately - patience to get it finally resolved.
I tried to fix the issues you mentioned above. Attached patch is what I'm running at the moment. Don't know if I catched up all errors...
btw. The authenticated field in trac's session table is int, too.