Comments

vladimir.dolgopolov’s picture

StatusFileSize
new3.03 KB
new1.85 KB
new1.02 KB

Here is the patch and tests fixing it.

gábor hojtsy’s picture

Unfortunately *.test files are not viewable on drupal.org right now. Logged a bug report on that here: http://drupal.org/node/348164

Until that is fixed, it would be a good idea if you could repost these in a patch form or as .txt files.

pwolanin’s picture

Status: Active » Needs review
StatusFileSize
new1012 bytes

Yes, sorry about that - obviosuly a total bug here:$fq = $this->fq[] = implode(" {$operator} ", $subfq);

I think the patch is not quite right, however - we need to implode with the operator.

pwolanin’s picture

committed the patch in #3

pwolanin’s picture

Title: Subqueries handling works not so well » Fix nodeacess and Subquery handling, and fix tests
StatusFileSize
new8.31 KB

Seems like we need some further fixes here as well - per testing and discussion w/ Jacob.

vladimir.dolgopolov’s picture

StatusFileSize
new2.93 KB
new1.69 KB

Here is the tests from #1 renamed as txt.

vladimir.dolgopolov’s picture

@Peter #3

I think the code:

$query1 = apachesolr_drupal_query('', 'uid:1 tid:5');
$query2 = apachesolr_drupal_query('', 'uid:10 tid:50');
$query2->add_subquery($query1, 'AND');
echo ($query2->get_url_querystring());

should produces (result of the patch #1):

filters=uid:10 tid:50 AND (uid:1 tid:5)

instead of (result of the patch #3):

filters=uid:10 tid:50 uid:1 AND tid:5

I think so because the code (notice I enter similar $keys instead of $filters param in apachesolr_drupal_query()):

$query1 = apachesolr_drupal_query('uid_1 tid_5');
$query2 = apachesolr_drupal_query('uid_10 tid_50');
$query2->add_subquery($query1, '', 'AND');
echo ($query2->get_query_basic());

produces:

uid_10 tid_50 AND (uid_1 tid_5)

P.S.
I'm thinking we should parenthesize the parent query as well:

(uid_10 tid_50) AND (uid_1 tid_5)
JacobSingh’s picture

StatusFileSize
new6.56 KB

Hey Guys,

The nodeaccess patch above looks good, and it looks like Peter already committed it.

I made one change to use parenthesis as Vlad suggests. Attached is a patch in progress of the subquery tests being handled a little better now. Since filter queries are always AND some of the previous tests no longer make sense AFAICT.

pwolanin’s picture

@Vladamir - I was not actually thinking about subqueries being in the URL, rather as being added later - I'm not sure that the parsing code correctly handles them from the URL.

pwolanin’s picture

Jacob - did you commit this patch? It looks like it from the CVS log.

pwolanin’s picture

StatusFileSize
new2.82 KB

patch to fix nodeaccess handling for mlt block queries.

JacobSingh’s picture

Status: Needs review » Patch (to be ported)
StatusFileSize
new3.18 KB

The handler can be determined form $params if needed, otherwise, committed the above. (patch attached).

vladimir.dolgopolov’s picture

There are fixed tests.

Notes:

1. solr_base_query.test
It passes if you apply the patch Pure negative queries don't work

2. solr_base_subquery.test
It includes (commented) testStaticFactory() method intended to test apachesolr_current_query() static cache. But there is a problem.

$query1 = apachesolr_current_query('foo', '', '', TRUE); // TRUE to resert. $query1 is 'foo'
$query2 = apachesolr_current_query('bar', 'title:baz'); // $query2 is 'bar?filters=title:baz'

$query2->add_subquery($query1); // $query2 should be 'bar AND (foo)?filters=title:baz' or whatever

// What should be as a result of the next line?
// a) 'bar?filters=title:baz' (not altered $query2)
// b) 'bar AND (foo)?filters=title:baz' (altered $query2)
$query3 = apachesolr_current_query('bar', 'title:baz'); // $query3 IS 'bar?filters=title:baz'

The original test assumes option (b). Here I see option (a).
We shoud decide which approach to follow.

3. solr_index_and_search.test
The original one is based on search.module test so it cannot be used for testing dismax functionality.
It uses a query type 'standard'.
The dismax test will be created soon.

pwolanin’s picture

Status: Patch (to be ported) » Needs review
JacobSingh’s picture

Status: Needs review » Closed (won't fix)

This is pretty ancient.