#204 db_text support for order-by clause, uses qsort

ver devel
closed-fixed
modules (140)
5
2009-03-16
2009-03-05
No

- 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

Discussion

  • Henning Westerholt

    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

     
  • Edgar Holleis

    Edgar Holleis - 2009-03-09

    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?

     
  • Henning Westerholt

    • assigned_to: nobody --> henningw
    • status: open --> closed-fixed
     
  • Henning Westerholt

    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.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks