386340 - Extra comma in SQL Query When Listing Test Plans

Status: RESOLVED FIXED

(Testopia :: General, defect)

Description

[Dustin Moore]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: CVS Tag TESTOPIA_1_2_BUGZILLA-222

I'm trying to get Testopia up to work with Postgres-8.1. I updated to the CVS version but it seems like there are major problems. I can add a test plan
ok but if I go to "My Plans" to see a list of plans the cgi sends a busted SQL statement to the server. The SQL line has an extra comma at the end of it! 
I looked around the code but it seems like the generation of this SQL statement is spread all over the place. I'm going to need some help to find the offending bit. The error is listed below:


Software error:
DBD::Pg::db selectcol_arrayref failed: ERROR:  syntax error at end of input at character 246
 [for Statement "SELECT test_plans.plan_id FROM test_plans  INNER JOIN profiles AS map_author ON (test_plans.author_id = map_author.userid) WHERE test_plans.isactive = 1 AND ((map_author.login_name IN ('[e-mail]'))) GROUP BY test_plans.plan_id, "] at Bugzilla/Testopia/Table.pm line 121
	Bugzilla::Testopia::Table::init('Bugzilla::Testopia::Table=HASH(0x90dd7ac)', 'plan', 'tr_list_plans.cgi', 'Bugzilla::CGI=HASH(0x8e5e818)', 'undef', 'SELECT test_plans.plan_id FROM test_plans  INNER JOIN profile...') called at Bugzilla/Testopia/Table.pm line 88
	Bugzilla::Testopia::Table::new('Bugzilla::Testopia::Table', 'plan', 'tr_list_plans.cgi', 'Bugzilla::CGI=HASH(0x8e5e818)', 'undef', 'SELECT test_plans.plan_id FROM test_plans  INNER JOIN profile...') called at /var/www3/bugzilla-2.22/tr_list_plans.cgi line 138

For help, please send mail to the webmaster (webmaster@localhost), giving this error message and the time and date of the error. 


Reproducible: Always

Steps to Reproduce:
1. Go to My Bugs
2.
3.
Actual Results:  
Got the error message


Software error:
DBD::Pg::db selectcol_arrayref failed: ERROR:  syntax error at end of input at character 246
 [for Statement "SELECT test_plans.plan_id FROM test_plans  INNER JOIN profiles AS map_author ON (test_plans.author_id = map_author.userid) WHERE test_plans.isactive = 1 AND ((map_author.login_name IN ('[e-mail]'))) GROUP BY test_plans.plan_id, "] at Bugzilla/Testopia/Table.pm line 121
	Bugzilla::Testopia::Table::init('Bugzilla::Testopia::Table=HASH(0x90dd7ac)', 'plan', 'tr_list_plans.cgi', 'Bugzilla::CGI=HASH(0x8e5e818)', 'undef', 'SELECT test_plans.plan_id FROM test_plans  INNER JOIN profile...') called at Bugzilla/Testopia/Table.pm line 88
	Bugzilla::Testopia::Table::new('Bugzilla::Testopia::Table', 'plan', 'tr_list_plans.cgi', 'Bugzilla::CGI=HASH(0x8e5e818)', 'undef', 'SELECT test_plans.plan_id FROM test_plans  INNER JOIN profile...') called at /var/www3/bugzilla-2.22/tr_list_plans.cgi line 138

For help, please send mail to the webmaster (webmaster@localhost), giving this error message and the time and date of the error. 

Expected Results:  
I would of liked to see a list of Bugs...

Comment 1

[Dustin Moore]

On line 1299 or so of Bugzilla/Testopia/Search.pm it says 
$query .= " " . $dbh->sql_group_by("test_${obj}s.${obj}_id", join(', ', @groupby));

in the case that @groupby is empty ($#groupby == -1) I think the second option with the join statement ALWAYS adds a ", " after the first option. If I replace this line with the following:

if ($#groupby > 0) {
$query .= " " . $dbh->sql_group_by("test_${obj}s.${obj}_id", join(', ', @groupby));
} else {
$query .= " " . $dbh->sql_group_by("test_${obj}s.${obj}_id");
}

my problem is resolved.

I don't know how everyone gets along without this change...

Comment 2

[Greg Hendricks]

This was fixed prior to releasing 1.2.2. If you run cvs_update.sh you should get the fix. 

Incidentally, you nailed it on the head and the fix is very similar:

    if (scalar @groupby){
        $query .= " " . $dbh->sql_group_by("test_${obj}s.${obj}_id", join(', ', @groupby));
    }
    else {
        $query .= " " . $dbh->sql_group_by("test_${obj}s.${obj}_id");
    }