Edgewall Software
Modify

Opened 9 years ago

Closed 6 years ago

#12029 closed defect (fixed)

TypeError: can't compare datetime.datetime to str

Reported by: miendo Owned by: Jun Omae
Priority: normal Milestone: 1.2.3
Component: query system Version: 1.2.2
Severity: normal Keywords:
Cc: simon.jakobi@… Branch:
Release Notes:

Query system returns None for time field being NULL rather than an empty string and fix query system crashing when time and/or changetime columns are NULL and saved query exists in the user's session.

API Changes:
Internal Changes:

Description

How to Reproduce

While doing a GET operation on /query, Trac issued an internal error.

(please provide additional details here)

Request parameters:

{'col': #u'id',
         u'summary',
         u'type',
         u'severity',
         u'due_assign',
         u'due_close',
         u'release_date'#,
 'group': u'owner',
 'order': u'due_close',
 'report': u'12',
 'status': #u'accepted', u'assigned', u'new', u'reopened'#}

User agent: Mozilla/4#0 #compatible; MSIE 7#0; Windows NT 6#1; WOW64; Trident/7#0; SLCC2; #NET CLR 2#0#50727; #NET CLR 3#5#30729; #NET CLR 3#0#30729; Media Center PC 6#0; InfoPath#3; #NET4#0C; #NET4#0E#

System Information

Trac 0#12
Babel 0#9#5
Genshi 0#6
mod_python 3#2#8
pysqlite 2#3#3
Python 2#4#3 ##1, Jan 9 2013, 06:47:03# ##br## #GCC 4#1#2 20080704 #Red Hat 4#1#2-54##
RPC 1#1#0
setuptools 0#6c7
SQLite 3#3#6
Subversion 1#6#15 #r1038135#
jQuery 1#4#2

Enabled Plugins

TracDecoratorPlugin 0#3#1
TracGanttCalendarPlugin 0#6#2
TracMenusPlugin 0#1
TracWysiwyg 0#12#0#2
TracXMLRPC 1#1#0

Python Traceback

Traceback #most recent call last#:
  File "/usr/lib/python2#4/site-packages/trac/web/main#py", line 513, in _dispatch_request
    dispatcher#dispatch#req#
  File "/usr/lib/python2#4/site-packages/trac/web/main#py", line 235, in dispatch
    resp = chosen_handler#process_request#req#
  File "/usr/lib/python2#4/site-packages/trac/ticket/query#py", line 933, in process_request
    return self#display_html#req, query#
  File "/usr/lib/python2#4/site-packages/trac/ticket/query#py", line 1052, in display_html
    data = query.template_data#context, tickets, orig_list, orig_time, req#
  File "/usr/lib/python2.4/site-packages/trac/ticket/query.py", line 732, in template_data
    elif ticket['changetime'] > orig_time:
TypeError: can't compare datetime.datetime to str

Attachments (0)

Change History (22)

comment:1 by Ryan J Ollos, 9 years ago

Issue previously reported in #10030, #10258 and #6088. Perhaps we need to consider having better protection against the possibility of a NULL or empty string in the changetime and time columns.

Maybe this happens with imported tickets, or tickets created through the XmlRpc interface? In 3 of the 4 tickets users have shown XmlRpcPlugin installed.

It would be useful to see a query of your database, like the one shown in comment:16:ticket:6088:

$ sqlite3 db/trac.db 
SQLite version 3.8.5 2014-08-15 22:37:57
Enter ".help" for usage hints.
sqlite> SELECT id FROM ticket WHERE changetime IS NULL OR changetime=''; 

comment:2 by Ryan J Ollos, 9 years ago

Milestone: next-dev-1.1.x

I can reproduce on the trunk at least, by setting changetime to NULL.

comment:3 by Ryan J Ollos, 9 years ago

Milestone: next-dev-1.1.xnext-dev-1.3.x

Narrowing focus for milestone:1.2. Please move ticket to milestone:1.2 if you intend to fix it.

comment:4 by simon.jakobi@…, 6 years ago

I ran into this issue at ghc.haskell.org/trac.

The following patch seems to fix it:

Index: trac/ticket/query.py
===================================================================
--- trac/ticket/query.py	(Revision 16436)
+++ trac/ticket/query.py	(Arbeitskopie)
@@ -775,9 +775,10 @@
             if orig_list:
                 # Mark tickets added or changed since the query was first
                 # executed
-                if ticket['time'] > orig_time:
+                if ticket['time'] and (ticket['time'] > orig_time):
                     ticket['added'] = True
-                elif ticket['changetime'] > orig_time:
+                elif (ticket['changetime'] and 
+                        (ticket['changetime'] > orig_time)):
                     ticket['changed'] = True
             if self.group:
                 group_key = ticket[self.group]

I've tested this manually by inserting NULL into the changetime and time fields.

comment:5 by Jun Omae, 6 years ago

Component: generalticket system
Milestone: next-dev-1.3.x1.0.17
Owner: set to Jun Omae
Status: newassigned

Thanks for the patch and helpful hints.

472 tickets has empty changetime on ghc.haskell.org/trac. See https://ghc.haskell.org/trac/ghc/query?changetime=19700101T000000Z..19700101T000001Z&col=changetime&order=id&max=10 (if the same error is occurred, clear last query in report page).

Last edited 6 years ago by Ryan J Ollos (previous) (diff)

comment:6 by Ryan J Ollos, 6 years ago

I think we could probably simplify using ticket.get('changetime') rather than ticket['changetime'] and ..., since the comparison will always evaluate to False if ticket.get('changetime') is None.

Version 0, edited 6 years ago by Ryan J Ollos (next)

in reply to:  5 comment:7 by Jun Omae, 6 years ago

Trac 0.5 is first released on Feb 23, 2004 (14 years ago) but the above tickets are opened on 2001…. See also source:/tags/trac-0.5.2/ChangeLog#L28.

comment:8 by Jun Omae, 6 years ago

Component: ticket systemquery system
Milestone: 1.0.171.2.3
Version: 0.121.2.2

Behaviors between 1.2.2 and 1.0.15 are different for changetime column being NULL.

Trac version changetime value in query
1.2.2 ''
1.0.15 datetime(1970, 1, 1, tzinfo=utc)

Reproduced it on Trac 1.2.2 but not on Trac 1.0.15.

Trac 1.2.2:

>>> import pprint
>>> from trac import __version__
>>> __version__
'1.2.2'
>>> from trac.test import EnvironmentStub
>>> from trac.ticket.query import Query
>>> env = EnvironmentStub()
>>> env.db_transaction("INSERT INTO ticket (summary,status,time,changetime) VALUES ('#12029','new',strftime('%s','now')*1000000,NULL)")
>>> env.db_query('SELECT * FROM ticket WHERE id=1')[0]
(1, None, 1519969856000000L, None, None, None, None, None, None, None, None, None, u'new', None, u'#12029', None, None)
>>> pprint.pprint(Query.from_string(env, 'id=1').execute())
[{u'_priority_value': '',
  u'changetime': '',
  u'id': 1,
  u'owner': '',
  u'priority': '',
  u'status': u'new',
  u'summary': u'#12029',
  u'time': datetime.datetime(2018, 3, 2, 5, 50, 56, tzinfo=<FixedOffset "UTC" 0:00:00>)}]

Trac 1.0.15:

>>> from trac import __version__
>>> __version__
'1.0.15'
>>> from trac.test import EnvironmentStub
env = EnvironmentStub()
env.db_transaction("INSERT INTO ticket (summary,status,time,changetime) VALUES ('#12029','new',strftime('%s','now')*1000000,NULL)")
env.db_query('SELECT * FROM ticket WHERE id=1')[0]
>>> from trac.ticket.query import Query
>>> env = EnvironmentStub()
>>> env.db_transaction("INSERT INTO ticket (summary,status,time,changetime) VALUES ('#12029','new',strftime('%s','now')*1000000,NULL)")
>>> env.db_query('SELECT * FROM ticket WHERE id=1')[0]
(1, None, 1519970122000000L, None, None, None, None, None, None, None, None, None, u'new', None, u'#12029', None, None)
>>> import pprint
>>> pprint.pprint(Query.from_string(env, 'id=1').execute())
[{u'changetime': datetime.datetime(1970, 1, 1, 0, 0, tzinfo=<FixedOffset "UTC" 0:00:00>),
  u'id': 1,
  u'owner': '',
  u'priority': '',
  u'priority_value': '',
  u'status': u'new',
  u'summary': u'#12029',
  u'time': datetime.datetime(2018, 3, 2, 5, 55, 22, tzinfo=<FixedOffset "UTC" 0:00:00>)}]

comment:9 by Jun Omae, 6 years ago

It seems the difference is introduced in [11331#file3] (#1942).

                             result['href'] = href.ticket(val)
                     elif name in self.time_fields:
-                        val = from_utimestamp(val)
+                        val = from_utimestamp(long(val)) if val else ''
                     elif field and field['type'] == 'checkbox':
                         try:

I guess tickets #1-587 on ghc are manually imported cause of changetime being NULL.

I consider we should add not-null constraint to time and changetime of ticket table to avoid such accidents.

comment:10 by anonymous, 6 years ago

(Those tickets contain comments with Logged In: YES user_id=... characteristic of being imported from sourceforge?)

in reply to:  10 comment:11 by Jun Omae, 6 years ago

Replying to anonymous:

(Those tickets contain comments with Logged In: YES user_id=... characteristic of being imported from sourceforge?)

First version of sourceforge2trac.py uses changetime='' at source:trunk/contrib/sourceforge2trac.py@475:260#L248.

in reply to:  6 comment:12 by simon.jakobi@…, 6 years ago

Replying to Ryan J Ollos:

I think we could probably simplify patch using ticket.get('changetime') rather than ticket['changetime'] and ..., since the comparison will always evaluate to False if ticket.get('changetime') is None.

ticket['changetime'] appears to be the empty string in my case.

comment:13 by Simon Jakobi <simon.jakobi@…>, 6 years ago

Cc: simon.jakobi@… added

comment:14 by Jun Omae, 6 years ago

Trac doesn't expect NULL in changetime column, which is lead by issue in first version of sourceforge2trac.py.

Please update changetime column with value of time column for records which have NULL in the changetime column.

UPDATE ticket SET changetime = time WHERE changetime IS NULL

in reply to:  14 comment:15 by Simon Jakobi <simon.jakobi@…>, 6 years ago

Replying to Jun Omae:

UPDATE ticket SET changetime = time WHERE changetime IS NULL

Thanks! This looks like an easy fix.

in reply to:  6 comment:16 by Jun Omae, 6 years ago

Replying to Ryan J Ollos:

I think we could probably simplify patch using ticket.get('changetime') rather than ticket['changetime'] and ..., since the comparison will always evaluate to False if ticket.get('changetime') is None.

TypeError is actually raised when datetime and None are compared.

$ ~/venv/trac/1.0.15/bin/python
Python 2.5.6 (r256:88840, Oct 21 2014, 22:49:55)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from datetime import datetime
>>> from trac.util.datefmt import utc
>>> ticket = {}
>>> orig_time = datetime.now(utc)
>>> ticket.get('changetime') > orig_time
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't compare datetime.datetime to NoneType

Also, the ticket dictionary always has time and changetime by add_cols(..., 'time', 'changetime') at source:/tags/trac-1.0.15/trac/ticket/query.py@:443#L422.

comment:17 by Ryan J Ollos, 6 years ago

Reporter: changed from miendo@… to miendo

in reply to:  8 comment:18 by Jun Omae, 6 years ago

Replying to Jun Omae:

Behaviors between 1.2.2 and 1.0.15 are different for changetime column being NULL.

Trac version changetime value in query
1.2.2 ''
1.0.15 datetime(1970, 1, 1, tzinfo=utc)

After [11331] (#1942), value of the time field is an empty string when the column has NULL but even if the column is a time field in custom field.

That is bad behavior. I think the time field should return None rather an empty string for the time field which has NULL value.

Last edited 6 years ago by Jun Omae (previous) (diff)

comment:20 by Ryan J Ollos, 6 years ago

Looks good. The change mirrors behavior of _db_str_to_datetime, used in _fetch_ticket.

Is the last call to mod.process_request(req) intentional, after the assertion, to ensure the exception is not raised? If so, a comment would be good to ensure it's not accidentally deleted later.

Unrelated, but noticed while study code. I propose the following refactoring:

  • trac/ticket/query.py

    git diff
    diff --git a/trac/ticket/query.py b/trac/ticket/query.py
    index 9123889b8..ccaa9db8a 100644
    a b def execute(self, req=None, cached_ids=None, authname=None, tzinfo=None,  
    313313
    314314            for row in cursor:
    315315                result = {}
    316                 for i in xrange(len(columns)):
    317                     name, field, val = columns[i], fields[i], row[i]
     316                for name, field, val in zip(columns, fields, row):
    318317                    if name == 'reporter':
    319318                        val = val or 'anonymous'
    320319                    elif name == 'id':

comment:21 by Jun Omae, 6 years ago

Thanks for the reviewing. Revised jomae.git@t12029_1.2.

Noticed minor issue while modifying code. Using added/changed/removed as custom fields, query results are highlighted. I've filed in #13007.

comment:22 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed in [16499] and merged in [16500].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.