On Special:Drilldown, a page which matches n selected values on a filter field is displayed n times (instead of just once)
Open, Needs TriagePublicBUG REPORT
Actions

Assigned To

None

Authored By

	Jonathan3
	Sep 8 2022, 12:46 PM

Description

For example, here's something I found on a public wiki at https://www.funtoo.org/Special:Drilldown/video_cards?_single&platforms%5B0%5D=DRI&platforms%5B1%5D=Vulkan&platforms%5B2%5D=X

https://www.funtoo.org/Make.conf/VIDEO_CARDS/nvidia appears twice because platforms=Vulkan, X
https://www.funtoo.org/Make.conf/VIDEO_CARDS/Osmesa appears twice because platforms=DRI, X

In both cases, the page should only appear once.

If it were, say, platforms=A,B,C,D,E,F then the page would appear 6 times if all 6 values were selected.

See:

Event Timeline

Jonathan3 created this task.Sep 8 2022, 12:46 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 8 2022, 12:46 PM

I think the query used on Special:Drilldown is (paraphrasing):

SELECT _pageName 
FROM cargo__video_cards
LEFT OUTER JOIN cargo__video_cards__platforms ON cargo__video_cards._ID = cargo__video_cards__platforms._rowID

As cargo__video_cards__platforms will often have multiple rows per _rowID (it holds a Cargo list) this means that there are often multiple rows in the query result with the same _pageName.

GROUP BY _pageName could be added to the query to remove the duplicate results.

On drilldown page tabs other than list, more than just the _pageName field is selected, but the same principle would apply.

Various queries with GROUP BY statements had the desired effect when I tested them, but I don't know whether or how they could be added to the Cargo code.

Is this the code to be looking at?

https://github.com/wikimedia/mediawiki-extensions-Cargo/blob/master/drilldown/CargoDrilldownPage.php#L1821

		// @TODO - fix the handling of "group by". It may be useful to
		// add all the queried field names to "group by" to prevent
		// duplicate listings, but it also sometimes causes the query to
		// not work.
		if ( !$this->formatByFieldIsList ) {
			if ( is_array( $queryOptions['GROUP BY'] ) && count( $queryOptions['GROUP BY'] ) > 0 ) {
				// $queryOptions['GROUP BY'] =
				//	array_merge( $queryOptions['GROUP BY'], array_values( $aliasedFieldNames ) );
			} else {
				// $queryOptions['GROUP BY'] = array_values( $aliasedFieldNames );
				$queryOptions['GROUP BY'] = null;
			}
		} else {
			$queryOptions['GROUP BY'] = null;
		}

"but it also sometimes causes the query to not work"

What makes it sometimes not work seems just to be that the helper tables/fields aren't added by the (commented out) code above.

e.g. if there's a date field there'll be SELECT ... Date, Date__precision but only GROUP BY ... Date.

Does that sounds right?

This makes it work for my Drilldown pages, including one with Date fields. The "timeline" tabs on Special:Drilldown/_pageData and Special:Drilldown/_fileData don't like it (popup says "Caught exception: SyntaxError: Unexpected number"). It's a bad first draft but I hope is a step in the right direction.

diff --git a/drilldown/CargoDrilldownPage.php b/drilldown/CargoDrilldownPage.php
index 751281c..1c05ea0 100644
--- a/drilldown/CargoDrilldownPage.php
+++ b/drilldown/CargoDrilldownPage.php
@@ -1824,15 +1824,21 @@ END;
                // not work.
                if ( !$this->formatByFieldIsList ) {
                        if ( is_array( $queryOptions['GROUP BY'] ) && count( $queryOptions['GROUP BY'] ) > 0 ) {
-                               // $queryOptions['GROUP BY'] =
-                               //      array_merge( $queryOptions['GROUP BY'], array_values( $aliasedFieldNames ) );
+                               $queryOptions['GROUP BY'] =
+                                       array_merge( $queryOptions['GROUP BY'], array_values( $aliasedFieldNames ) );
                        } else {
-                               // $queryOptions['GROUP BY'] = array_values( $aliasedFieldNames );
-                               $queryOptions['GROUP BY'] = null;
+                               $queryOptions['GROUP BY'] = array_values( $aliasedFieldNames );
+                               // $queryOptions['GROUP BY'] = null;
                        }
                } else {
                        $queryOptions['GROUP BY'] = null;
                }
+               // Add __precision field for each date field
+               // This adds it to dates that don't need it (e.g. _creationDate)
+               // so instead maybe should use code from CargoSQLQuery's handleDateFields function
+               foreach ($this->dateFields as $dfield) {
+                       $queryOptions['GROUP BY'][] = $dfield . '__precision';
+               }

                $tablesStr = '';
                $i = 0;

From email correspondence ages ago, I think Yaron was not in favour of this as there might be times when someone would want duplicate results on the Drilldown page.

Currently the page shows a plain list, with multiple duplicate values. I believe that this is no good. It is not explained and, to me, just looks like a bug.

But I can see that it would make sense for someone to use a drilldown tab with a template to display the page name plus relevant additional field values (e.g. the filter values that caused the page to match multiple times).

So how about this:

There should be a _drilldownTabs option to determine whether to include the duplicate page names.
The default would be "yes" so as not to break any existing wikis which are doing something like the above.

On Special:Drilldown, a page which matches n selected values on a filter field is displayed n times (instead of just once)Open, Needs TriagePublicBUG REPORTActions

Description

Event Timeline

On Special:Drilldown, a page which matches n selected values on a filter field is displayed n times (instead of just once)
Open, Needs TriagePublicBUG REPORT
Actions