Database schema is incorrect or casts are needed (PostgreSQL 8.4 database)

[dbely]

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:

1. 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?

2. 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.

[Oliver Metz]

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.

[dbely]

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.

[stephen.anderson@…]

Alternate patch that should work on everything except SQLite

[anonymous]

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.

[stephen.anderson@…]

Updated (merged) patch that should work on all DBs, and fixes the error on ticket delete

[stephen.anderson@…]

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)

[Stephan Geulette]

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...

[anonymous]

See also #10384

[Ryan J Ollos]

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.

[Ryan J Ollos]

There are 3 issues discussed in this ticket:

1. The authenticated variable is not properly cast from a bool to an int in model.py. This issue is documented in #10384.
2. There is concern that non-naive timestamps are not correctly cast to bigint. This issue is documented in #7975.
3. 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.

[Steffen Hoffmann]

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.

[Steffen Hoffmann]

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.

[Steffen Hoffmann]

(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.)

[Steffen Hoffmann]

(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.

[Steffen Hoffmann]

(In [12304]) TracAnnouncer: Correctly convert id for ticket resources, refs #8065 and #9154.

[Steffen Hoffmann]

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...

Hope, that all concerns are addressed even more portable now. If not, please follow-up on #10384 as hinted above.