[Kamailio-Devel] [ openser-Patches-2665569 ] db_text support for order-by clause, uses qsort

[SourceForge.net]

Patches item #2665569, was opened at 2009-03-05 15:59
Message generated for change (Comment added) made by henningw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743022&aid=2665569&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: ver devel
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Edgar Holleis (ccaracalla)
>Assigned to: Henning Westerholt (henningw)
Summary: db_text support for order-by clause, uses qsort

Initial Comment:
- parses db_key_t _o as if it was an SQL-fragment
- sort by multiple columns supported
- SQL keywords ASC and DESC supported
- supports sorting by columns that would not normally be retrieved
- removes extra columns before returning result
- uses C-libraries qsort function
- patch is against 1.5.0, but patch against 1.3.* also available
- patch includes fixes for issues #2658723 and #2658593

----------------------------------------------------------------------

>Comment By: Henning Westerholt (henningw)
Date: 2009-03-16 17:29

Message:
I looked today a bit longer at this and also discussed the alternatives.
The qsort compare function here is a bit special as you said, as it loops
over the data, which is not the case in other instances where it also used,
for example in cr. All alternatives that could be uses instead have some
different drawbacks, so i think lets choose the approach you proposed in
the patch.

I've added two error logs to this special condition that it can be better
detected, but otherwise commited it unchanged, thanks.

----------------------------------------------------------------------

Comment By: Edgar Holleis (ccaracalla)
Date: 2009-03-09 13:40

Message:
About setjmp/longjmp: The broader topic is how to get out of qsort. A flag
could be used to signal error, and let qsort finish anyway. The problem
there is that the comparison function shouldn't violate the propositions
imposed by the assumption that the data can be sorted, lest we want qsort
to loop forever. But we could make an error-producing row be bigger (or
smaller) than every non-error-producing row, but the same as all other
error-producing rows.

The other question is whether the error condition can actually occur
during normal operation, anyway. I'd say that the error condition occurring
is an indication for another bug in dbtext, an assert-statement might be
appropriate. 

I decided to handle the error only, because dbt_cmp_val can return it. The
other function calling dbt_cmp_val, dbt_row_match, silently ignores it.

I'd say it is your decision: setjmp, error-flag, assert, or silently
ignore?

----------------------------------------------------------------------

Comment By: Henning Westerholt (henningw)
Date: 2009-03-06 23:49

Message:
Hi Edgar,

thanks for the patch, it looks really good. The only thing that i noticed
is the usage of setjmp/ longjmp for the error handling, which is not really
used in our code base. Perhaps we can find a alternative solution for this,
what do you think?

Henning

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743022&aid=2665569&group_id=139143