DKAN Datastore API cannot sort by timestamp

[lcshiung]

I am testing the DKAN Datastore API. It is able to sort by all fields in the JSON returned, except the timestamp. When I use the following query:

http://server_ip/api/action/datastore/search.json?resource_id=6bd1a4d7-93c3-4e47-b77e-8dabdb13fcad&sort=timestamp%20desc&limit=5

I get the following error:

["Invalid query provided, double check that the fields and parameters you defined are correct and exist. SQLSTATE[42S22]: Column not found: 1054 Unknown column 't.timestamp_' in 'order clause'"]

After doing some tracing, I have found that this is because the function feeds_flatstore_processor_safe_name in feeds_flatstoreprocessor.module appends '' to timestamp as it is a reserved word:

function feeds_flatstore_processor_safe_name($name) {
  $reserved = feeds_flatstore_reserved_words();
  if (in_array($simple, $reserved)) {
    $simple = $simple . '_';
  }

Is there any way to resolve this?

On a secondary note, the example used in DKAN Datastore API documentation may not be updated when it suggests that it is possible to sort by country using "sort[country]=asc". I realise you have to use "sort=country asc".

[dafeder]

@lcshiung: Does it work when you sort by timestamp_? We could add some code that "unescapes" those reserved words if they're passed in the query. Interested in submitting a PR?

Will definitely check out the documentation and update where necessary.

[lcshiung]

I am new to github, and am not very sure how to submit a PR, would it be ok if I suggest how a simple fix can be done? Let me know if you would prefer that I submit a PR.

It looks like the error only affects the "sort" parameter for the API. This is because when sorting, the datastore API attempts to prevent the use of reserved words in parameters (by adding a "_" suffix). If I understood the code correctly, the original intention of reserved words were to avoid clashes between resource column names and the reserved words, but only at the table creation stage. In other words, cleaning the column names for sorting is unnecessary.

In /profiles/dkan/modules/dkan/dkan_datastore/modules/dkan_datastore_api/dkan_datastore_api.module, comment out the following lines in function dkan_datastore_api_sort_clean($sort):

//  foreach ($sort as $key => $value) {
//    $sort[$key] = feeds_flatstore_processor_safe_name($value);
//  }

The final function looks like this:

function dkan_datastore_api_sort_clean($sort) {
  if (strpos($sort, ' asc') !== FALSE) {
    $type = 'asc';
  }
  elseif (strpos($sort, ' desc') !== FALSE) {
    $type = 'desc';
  }
  else {
    return $sort;
  }
  $sort = explode(" $type", $sort);
//  foreach ($sort as $key => $value) {
//    $sort[$key] = feeds_flatstore_processor_safe_name($value);
//  }
  $sort = implode(" $type", $sort);
  return $sort;
}

I have tested this to work.

Alternatively, since this change would mean the function "dkan_datastore_api_sort_clean($sort)" serves no purpose, we could entirely remove the function, and also a call to this function in the "dkan_datastore_api_datastore_index" function.

//$sort = dkan_datastore_api_sort_clean($sort);

[dafeder]

@lcshiung I'm closing your pull request #1171, as it was created incorrectly. What you would need to do is create a branch of the dkan_datastore repo, make and commit your changes, push your branch and then create a pull request to pull your branch into 7.x-1.x.

In the future we actually plan to stop working on dkan_datastore in a separate repo, and pull it into tbe main DKAN repo, so this will be a little easier. For now, please make PRs to the respective repos though.

[dafeder]

BTW @lcshiung I can create the PR if you like, but you may want to do it yourself to learn and get credit for the commit :)

For a PR this simple you could do the whole thing in the github UI. I forgot one step above, you will need to fork the repo first as you don't have write access to https://github.com/NuCivic/dkan_datastore/. Once it's forked, edit dkan_datastore_api.module, commit your changes to a branch, and create a PR against the main repo.

I'll look at the particulars later, but i think removing the sort_clean function will create other problems.

[fmizzell]

@dafeder I agree with @lcshiung regarding where the clean up should happen (We should clean up when creating the tables). In that case it feels safe for us to remove that clean up in the API. What other possible dangers where you foreseeing?