Page MenuHomePhabricator

Implement 'dir' for query properties.
Closed, ResolvedPublic

Description

Currently it's required to save the previous query-continue somewhere or even having to construct it ourselfs to page back from somewhere.

For example, the following query is a "page 3":

http://commons.wikimedia.org/w/api.php?format=jsonfm&action=query&titles=User%3AKrinkle%2FTestcase&tllimit=2&prop=templates&tlcontinue=12796614|10|BUser

How to get (back) to "page 2" ?


Version: 1.18.x
Severity: enhancement

Details

Reference
bz26909

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:22 PM
bzimport set Reference to bz26909.

(In reply to comment #0)

Currently it's required to save the previous query-continue somewhere or even
having to construct it ourselfs to page back from somewhere.

For example, the following query is a "page 3":

http://commons.wikimedia.org/w/api.php?format=jsonfm&action=query&titles=User%3AKrinkle%2FTestcase&tllimit=2&prop=templates&tlcontinue=12796614|10|BUser

How to get (back) to "page 2" ?

By saving the q-continue, like you said, or by reversing the dir parameter and passing the same continue value (no guarantee that this'll actually work).

There's no way to handle the general case efficiently, DB-wise, so WONTFIX.

(In reply to comment #1)

by reversing the dir parameter and passing the same continue value (no guarantee that this'll actually work).

You mentioned this on IRC as well, but I have found no such parameter (tried different things and looked at the documentation).

There's no way to handle the general case efficiently, DB-wise, so WONTFIX.

Fair enough :)

(In reply to comment #2)

(In reply to comment #1)

by reversing the dir parameter and passing the same continue value (no guarantee that this'll actually work).

You mentioned this on IRC as well, but I have found no such parameter (tried
different things and looked at the documentation).

It's not called 'dir', but 'apdir' for allpages, 'rvdir' for revisions, etc.

(In reply to comment #3)

(In reply to comment #2)

(In reply to comment #1)

by reversing the dir parameter and passing the same continue value (no guarantee that this'll actually work).

You mentioned this on IRC as well, but I have found no such parameter (tried
different things and looked at the documentation).

It's not called 'dir', but 'apdir' for allpages, 'rvdir' for revisions, etc.

For lists yes, but not for properties.

For lists there's 'from' and 'dir':
http://www.mediawiki.org/w/api.php?action=query&list=allpages
http://www.mediawiki.org/w/api.php?action=query&list=allpages&apfrom=$wgAllowAnonymousMinor
http://www.mediawiki.org/w/api.php?action=query&list=allpages&apfrom=$wgAllowAnonymousMinor&apdir=descending

But there's no 'dir' for properties

http://www.mediawiki.org/w/api.php?action=query&titles=MediaWiki&prop=links&pllimit=2
http://www.mediawiki.org/w/api.php?action=query&titles=MediaWiki&prop=links&pllimit=2&plcontinue=1|0|Developer%20hub

Re-opened with adjusted summary.

Created attachment 9435
add dir param for prop=links|templates|categories|extlinks|iwlinks|langlinks|images|duplicatefiles

The attached patch adds a dir param to prop=links|templates|categories|extlinks|iwlinks|langlinks|images|duplicatefiles

The other modules have a dir param (revisions) or in my opinion it makes no sense (info|categoriyinfo|pageprops|stashimageinfo|imageinfo).

It also changed the handling of ORDER BY in ApiQueryBase::addWhereRange. This is needed, because the order by can given as array and that was not handled there.

Feel free to modify the patch.

Thanks.

Attached:

(In reply to comment #7)

Created attachment 9435 [details]
add dir param for
prop=links|templates|categories|extlinks|iwlinks|langlinks|images|duplicatefiles

Thanks for the patch. Modified patch committed in r102947.

It also changed the handling of ORDER BY in ApiQueryBase::addWhereRange. This
is needed, because the order by can given as array and that was not handled
there.

Good catch. I cleaned up the ApiQueryBase changes a bit before committing. It looked like it would've generated a notice because the access to $this->options['ORDER BY'] wasn't protected with isset() (there was an isset call nearby, but 1) the array access was done unconditionally anyway and 2) isset was called on a variable that was set unconditionally, which means it was used to check for null, which is evil) and the logic with wrapping a non-array value in an array could be replaced with an (array) cast. I ended up using:

$optionOrderBy = isset( $this->options['ORDER BY'] ) ? (array)$this->options['ORDER BY'] : array();
$optionOrderBy[] = $order;

Feel free to modify the patch.

The only other modification I made was to drop the ApiQueryExternalLinks.php changes. They added an ORDER BY on el_to which is a partially-indexed blob field, so that would filesort. The query isn't very nice as it is, but this would make it worse.

Attached:

Thank you for review and commit.

(In reply to comment #8)

(In reply to comment #7)

Feel free to modify the patch.

The only other modification I made was to drop the ApiQueryExternalLinks.php
changes. They added an ORDER BY on el_to which is a partially-indexed blob
field, so that would filesort. The query isn't very nice as it is, but this
would make it worse.

It there a way to get the dir param also for ApiQueryExternalLinks.php? Should I create a new bug to track this (and which gets maybe LATER or WONTFIX)? Thanks.

(In reply to comment #9)

It there a way to get the dir param also for ApiQueryExternalLinks.php? Should
I create a new bug to track this (and which gets maybe LATER or WONTFIX)?
Thanks.

You should submit a new patch, either on this bug or on a new one (I don't really care) that doesn't add the order by on el_to. I realize this may mean the result set isn't completely reversed, but that's unavoidable; it's either that or no dir param at all.

Created attachment 9730
fix for the dir param in prop=links|templates|categories|iwlinks|langlinks|images|duplicatefiles

Now you cannot continue with dir=descending, because you get the same result. I have create a patch, to fix that problem in each prop module. Thanks.

Attached:

Roan already committed r102947. Is this bug fixed ? Or is this new patch a follow-up that fixes something ?

(In reply to comment #13)

Roan already committed r102947. Is this bug fixed ? Or is this new patch a
follow-up that fixes something ?

It is a follow up, which fix the navigation with 'dir' and 'continue'.

commited the second patch with gerrit 4028

  • Bug 36343 has been marked as a duplicate of this bug. ***