Bug 486036

Summary: Custom field 'cf_pm_score' is not sorted numerically properly
Product: [Community] Bugzilla Reporter: David Lawrence <dkl>
Component: Query/Bug ListAssignee: David Lawrence <dkl>
Status: CLOSED NEXTRELEASE QA Contact:
Severity: medium Docs Contact:
Priority: low    
Version: 3.2CC: bpeck, kbaker, peterm, riek, sghosh
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 486142 (view as bug list) Environment:
Last Closed: 2009-02-18 16:37:58 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Patch to hack the order by for cf_pm_score to sort numerically (v1) nelhawar: review+

Description David Lawrence 2009-02-17 23:10:55 UTC
Description of problem:
The custom field called 'cf_pm_score' is created in the database as a freetext field type (varchar) and even though it only contains integer values it is not sorted correctly by numerical order. Therefore you get results such as:

http://hurl.test.redhat.com/G48Zdo

Changing to the column type in the database for bugs.cf_pm_score to INTEGER does fix the sorting issue but is not optimal as anything other than an integer entered there will cause a database error to occur.

Another fix is to hack Bugzilla/Search to alter the ORDER BY clause to do a type conversion on the fly of the cf_pm_score data to integer allowing the order by to work numerically.

I have a patch that does the latter and works well.

Dave

Comment 1 David Lawrence 2009-02-17 23:13:26 UTC
Created attachment 332312 [details]
Patch to hack the order by for cf_pm_score to sort numerically (v1)

Patch to hack the order by for cf_pm_score to sort numerically.

Please review
Dave

Comment 2 Noura El hawary 2009-02-18 05:00:17 UTC
Comment on attachment 332312 [details]
Patch to hack the order by for cf_pm_score to sort numerically (v1)

Hey Dave, I tested the patch on bz-web2 and works good.

Noura

Comment 3 Kevin Baker 2009-02-18 16:09:41 UTC
definitely a hack. 

Riek or Subhendu can confirm but AIUI this field will always be integer. Can we take a copy of the data, drop the custom field and re-add it as an int cf?

Bill Peck is hitting a simliar issue in beaker.

Comment 4 David Lawrence 2009-02-18 16:26:11 UTC
(In reply to comment #3)
> definitely a hack. 
> 
> Riek or Subhendu can confirm but AIUI this field will always be integer. Can we
> take a copy of the data, drop the custom field and re-add it as an int cf?
> 
> Bill Peck is hitting a simliar issue in beaker.

I agree but if I change the column to INT and someone per chance enters something with alpha character in it, the code currently thinks the columns is a varchar type which will produce a nasty DB error.

I would rather take the time do this right in another bug that creates a new FIELD_TYPE_INTEGER that has the proper sanity checks before inserting/updating the data in the db. Currently the code will still think that the column is a FIELD_TYPE_FREETEXT.

This order by hack will get his reports working until we can add the new functionality and properly convert the cf_pm_score over to FIELD_TYPE_INTEGER.

Dave

Comment 5 David Lawrence 2009-02-18 16:37:58 UTC
Thanks Noura. Checked into CVS for next update.

Am cloning this issue into a new bug to help us track request for a proper FIELD_TYPE_INTEGER.

Dave

Comment 6 Kevin Baker 2009-02-18 17:12:50 UTC
(In reply to comment #4)

> I agree but if I change the column to INT and someone per chance enters
> something with alpha character in it, the code currently thinks the columns is
> a varchar type which will produce a nasty DB error.

Ah, I thought bugzilla had two types of custom fields, free text & integer. But on closer inspection, no. It seems the 'Multi-Value Text Box (Integer)' is integer specific. Was that a field we added?
 
> I would rather take the time do this right in another bug that creates a new
> FIELD_TYPE_INTEGER that has the proper sanity checks before inserting/updating
> the data in the db. Currently the code will still think that the column is a
> FIELD_TYPE_FREETEXT.

Did the upstream take our last cf patch?

> This order by hack will get his reports working until we can add the new
> functionality and properly convert the cf_pm_score over to FIELD_TYPE_INTEGER.

understood

Comment 7 David Lawrence 2009-02-18 17:37:44 UTC
(In reply to comment #6)
> Ah, I thought bugzilla had two types of custom fields, free text & integer. But
> on closer inspection, no. It seems the 'Multi-Value Text Box (Integer)' is
> integer specific. Was that a field we added?
> 

Yeah that was originally created for issue trackers but could be used for any list of integers.

Technically we could use the multi integer field for pm_score even though it will always be one value (unless they want to have more than one score per bug).

It would require a migration script to move the data over once the new multi value table is created.

> 
> Did the upstream take our last cf patch?
> 

No. They always want something is more big picture. I submitted the patch for approval and then they got aspirations of improving on it to also allow it to be used for current fields as well such as depends on and blocks. And then it spiralled downward into a black hole.

Dave