in apachesolr_search_execute, there is a $params array. This should really all be encapsulated inside of the query object.

This code is silly:

  // This hook allows modules to modify the query and params objects.
  apachesolr_modify_query($query, $params, $caller);

You can do many things to the query at this point that won't affect the outcome because the only method of $query that gets called in the searching is $query->get_query_basic():

  $response = $solr->search($query->get_query_basic(), $params['start'], $params['rows'], $params);

The API signature should be something more along the lines of $solr->search($query);

Functions like apachesolr_search_basic_params($query) are also misplaced and should be part of a base class. The state that gets set with apachesolr_search_basic_params should just be part of the constructor, or should be passed into the constructor.

Comments

Scott Reynolds’s picture

Scott Reynolds commented

David and I were just debating the same here

#544570: Provide a way to alter the boost, facet and basic parameters

jpmckinney’s picture

jpmckinney commented
Category: bug » task
pwolanin’s picture

pwolanin commented

This should happen in the Drupal 7 port, not Drupal 6 at all.

jpmckinney’s picture

jpmckinney commented
Version: 6.x-2.x-dev » 7.x-1.x-dev
jpmckinney’s picture

jpmckinney commented

Marking #560596: Filter Queries supported? duplicate. See comment #5: #560596-5: Filter Queries supported?.

Scott Reynolds’s picture

Scott Reynolds commented

This should happen in the Drupal 7 port, not Drupal 6 at all.

For the curious, why?

pwolanin’s picture

pwolanin commented

@Scott - the 6.x-2.x branch is far enough to be in production for many people. At DCSF we heard feedback that people would be very put out by API changes there.

@all - I just labeled the HEAD release as 7.x-1.x - James beat me to changing the label here once I made it possible.

pwolanin’s picture

pwolanin commented
Version: 7.x-1.x-dev » 6.x-2.x-dev
Status: Active » Patch (to be ported)

already done for for 7.x.

I'm considering whether it would make sense to make 6.x-2.x a backport of 7.x-1.x in more places. This would totally break APIs, schema, etc.

However, given the lack of contributions to keeping 6.x-2.x going, I would no longer feel much remorse.

robertdouglass’s picture

robertdouglass commented

I think that it is better to have a healthy 6.2 that is forwards compatible than to protect the implementation that currently exist. We have to document the changes well for those who will do the upgrades (6.2 -> 6.2.x) but, yes, the goal should be keeping in step with 7.0.

pwolanin’s picture

pwolanin commented

We will make a 6.x-3.x to be in sync with 7.x-1.x (confusing as though that might be)

pwolanin’s picture

pwolanin commented
Status: Patch (to be ported) » Closed (fixed)

we'll do this in 6.x-3.x