Edgewall Software
Modify

Ticket #6808 (closed defect: fixed)

Opened 4 years ago

Last modified 3 years ago

Exception: column name is not unique

Reported by: geert@… Owned by: ecarter
Priority: high Milestone: 0.11
Component: admin/web Version: devel
Severity: normal Keywords: exception column name not unique
Cc:
Release Notes:
API Changes:

Description (last modified by cboos) (diff)

In the admin section, add a version that already exists.

Python Traceback

Traceback (most recent call last):
  File "/usr/local/lib/python2.4/site-packages/trac/web/main.py", line 406, in dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/lib/python2.4/site-packages/trac/web/main.py", line 237, in dispatch
    resp = chosen_handler.process_request(req)
  File "/usr/local/lib/python2.4/site-packages/TracWebAdmin-0.1.2-py2.4.egg/webadmin/web_ui.py", line 109, in process_request
    path_info)
  File "/usr/local/lib/python2.4/site-packages/TracWebAdmin-0.1.2-py2.4.egg/webadmin/ticket.py", line 244, in process_admin_request
    ver.insert()
  File "/usr/local/lib/python2.4/site-packages/trac/ticket/model.py", line 723, in insert
    (self.name, self.time, self.description))
  File "/usr/local/lib/python2.4/site-packages/trac/db/util.py", line 50, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
  File "/usr/local/lib/python2.4/site-packages/trac/db/sqlite_backend.py", line 56, in execute
    args or [])
  File "/usr/local/lib/python2.4/site-packages/trac/db/sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
IntegrityError: column name is not unique

Attachments

Change History

comment:1 follow-up: Changed 4 years ago by osimons

  • Milestone changed from 0.10.5 to 0.11.1
  • Version changed from 0.10.4 to devel

It exists in trunk as well, and is the same for other enums as well. It also happens from both webadmin and trac-admin.

2 alternatives:

  1. Close it as duplicate of #6348 to handle these exceptions better in a generic manner, as done for #6095.
  2. Look for errors in the code, as it seems like it should support checking for existing names.

Regarding 2, here is what I see in the current trunk trac.ticket.model code (class Version is identical in implementation):

class AbstractEnum(object):
...
    def __init__(self, env, name=None, db=None):
...
    exists = property(fget=lambda self: self._old_value is not None)
...
    def insert(self, db=None):
        assert not self.exists, 'Cannot insert existing %s' % self.type

Just did a quick check in trac.ticket.admin.py code for adding versions, and the reason why this does not trigger is that we do not instantiate by passing the name to __init__() - so self._old_value will not get set:

  • trac/ticket/admin.py

     
    232232            if req.method == 'POST': 
    233233                # Add Version 
    234234                if req.args.get('add') and req.args.get('name'): 
    235                     ver = model.Version(self.env) 
    236                     ver.name = req.args.get('name') 
     235                    ver = model.Version(self.env, name=req.args.get('name')) 
    237236                    if req.args.get('time'): 
    238237                        ver.time = parse_date(req.args.get('time')) 
    239238                    ver.insert() 

However, doing that it seems that __init__() does not have a fallback of None if it does not exist (no row fetched from datababase), so trying to add a new valid version gives Trac Error:

Version 3.0 does not exist.

Catch-22 it seems… I think we should:

  • Use the pattern of model.Version(self.env, name=req.args.get('name')) in all trac-admin and webadmin code.
  • Modifying __init__() in the various model classes to have a fallback if there is no actual existing instance of that name. Why would we want to raise a TracError in the __init__() code?
  • The code that needs to check for existence should use the the_version.exists property after object creation.

comment:2 in reply to: ↑ 1 Changed 4 years ago by cboos

Replying to osimons:

trying to add a new valid version gives Trac Error:

Version 3.0 does not exist.

That's the usual Trac approach for handling the data model objects: when the constructor gets an identifier, it expects to be able to fetch an existing instance for that identifier. If not, it will raise a TracError. Some have already pointed out the trouble with this approach, see #4130.

comment:3 Changed 4 years ago by cboos

  • Milestone changed from 0.11.1 to 0.12
  • Priority changed from low to high

#6952 closed as duplicate, as the same consideration applies for all enumerations.

I think we should do #6348 to handle this issue for 0.11.1 and solve the "Catch-22" for 0.22 ;-)

comment:4 Changed 4 years ago by cboos

  • Description modified (diff)

#6855 and #6983 were closed as duplicates.

comment:5 follow-up: Changed 4 years ago by ecarter

The reported error is now fixed in trunk [6772], with testcases in sandbox/testing [6773] [6775]. There may be a better implementation, but I suggest we close this one.

comment:6 in reply to: ↑ 5 Changed 4 years ago by osimons

Replying to ecarter:

The reported error is now fixed in trunk [6772], with testcases in sandbox/testing [6773] [6775]. There may be a better implementation, but I suggest we close this one.

+1. Let's use #6348 to note the various duplicates and variations of this error - and to hopefully solve it for all.

comment:7 Changed 3 years ago by cboos

  • Milestone changed from 0.13 to 0.11
  • Resolution set to fixed
  • Status changed from new to closed

Closing, as the specific error with enums was fixed in r6772 and the more general fix will be addressed in #6348.

comment:8 Changed 3 years ago by cboos

  • Owner changed from cmlenz to ecarter
View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from ecarter. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.