Open jeckyhl opened 11 years ago
1° your proposition makes sense to me, however I believe it would be better to leave the definition of the order clause in the Source_Process_Filters function, and change the return value as appropriate instead -- something like (not tested)
diff --git a/Source/Source.FilterAPI.php b/Source/Source.FilterAPI.php
index aceef05..de9f75b 100644
--- a/Source/Source.FilterAPI.php
+++ b/Source/Source.FilterAPI.php
@@ -65,10 +65,10 @@ class SourceFilter {
function find( $p_page=1, $p_limit=25 ) {
list( $t_filters, $t_filter_params ) = Source_Twomap( 'Source_Process_FilterOption', $this->
- list ( $t_query_tail, $t_params ) = Source_Process_Filters( $t_filters, $t_filter_params );
+ list( $t_query_tail, $t_order, $t_params ) = Source_Process_Filters( $t_filters, $t_filter_p
$t_count_query = "SELECT COUNT(c.id) $t_query_tail";
- $t_full_query = "SELECT DISTINCT( c.id ), c.* $t_query_tail";
+ $t_full_query = "SELECT DISTINCT( c.id ), c.* $t_query_tail $t_order";
$t_count = db_result( db_query_bound( $t_count_query, $t_params ) );
@@ -139,7 +139,7 @@ function Source_Process_Filters( $p_filters, $p_filter_params ) {
$t_order = 'ORDER BY c.timestamp DESC';
- return array( "$t_join $t_where $t_order", $t_params );
+ return array( "$t_join $t_where", $t_order, $t_params );
}
function Source_Process_FilterOption( $key, $option ) {
Regarding 2°, considering that @jreese specifically added the DISTINCT clause in commit faf9ca0196eee7dbe8757041ceeb633b43d35cb4, I would assume he had a valid reason for doing so, therefore we should be careful for any potential regression before removing it. John, any comment ?
1°) I think your approach is indeed cleaner. Works fine for me (tested with MantisBT 1.2.12/SQL Server 2008)
Reopened as point 2° has not yet been addressed
Assuming that you are adding some form of correction in the code for duplicate result rows, then I would imagine the DISTINCT is not necessary. But I don't even remember the context of when/why I added that, so your guess is as good as mine.
This bug report follows bug #36 ; I found some others incomptabilities with SQL Server
config : MantisBT 1.2.12 with SQL Server 2008 plugin-version : master (changeset 2a0e94beba)
1° Error when searching issues (Repositories > Search ; search for all changesets) saying that
c.timestamp
field is not in a aggregate function query :This is caused by
ORDER BY
clause at the end of query2° Again when searching issues --> error cannot compare TEXT fields query :
This is caused by
DISTINCT
clause.My workarounds / solutions : 1° move
$t_order
variable from Source_Process_Filters(..) to find(..) function. The find(..) function becomes :2° Simply delete
DISTINCT
keyword (seem not useful here) ini Source.FilterAPI.php, function find