Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle null 'type' parameter in search #559

Closed
d13r opened this issue Mar 30, 2017 · 2 comments
Closed

Handle null 'type' parameter in search #559

d13r opened this issue Mar 30, 2017 · 2 comments

Comments

@d13r
Copy link

d13r commented Mar 30, 2017

Summary of problem or feature request

If you pass 'type' => null to $client->search(), instead of (1) returning all types or (2) showing a useful error message (e.g. Invalid type), it returns this error:

UnexpectedValueException in AbstractEndpoint.php line 221:
"type" is not a valid parameter. Allowed parameters are "_source", "_source_exclude", "_source_include", "allow_no_indices", "analyze_wildcard", "analyzer", "client", "custom", "default_operator", "df", "docvalue_fields", "expand_wildcards", "explain", "fielddata_fields", "filter_path", "filter_path", "from", "human", "ignore_unavailable", "indices_boost", "lenient", "lowercase_expanded_terms", "preference", "q", "query_cache", "request_cache", "routing", "scroll", "search_type", "size", "sort", "source", "stats", "suggest_field", "suggest_mode", "suggest_size", "suggest_text", "terminate_after", "timeout", "version"

I believe this is because search() calls:

$type = $this->extractArgument($params, 'type');

And extractArgument() checks for the parameter using isset():

if (isset($params[$arg]) === true) {
    $val = $params[$arg];
    unset($params[$arg]);
    return $val;
} else {
    return null;
}

Which returns false when used on null values. If it used array_key_exists($arg, $params) instead of isset($params[$arg]) I think it would work as expected - remove the key from $params, return null and therefore return all types.

My reason for wanting to be able to pass null to get all types is simply so I can do this:

$response = $client->search([
    'index' => 'myapp',
    'type' => $_GET['type'] ?? null,
    'body'  => /*...*/
]);

Instead of this:

$params = [
    'index' => 'myapp',
    'body'  => /*...*/
];

if (isset($_GET['type'])) {
    $params['type'] = $_GET['type'];
}

$response = $client->search($params);

But if that can't / won't be supported for any reason, a better error message would be nice.

Thanks!

Code snippet of problem

$response = $client->search([
    'index' => 'myapp',
    'type' => null,
    'body'  => [
        'query' => [
            'match' => [
                '_all' => 'test',
            ],
        ],
    ],
]);

System details

  • Operating System: Ubuntu 16.04
  • PHP Version: 7.1.2
  • ES-PHP client version: 5.1.3
  • Elasticsearch version: 5.3.0
@d13r
Copy link
Author

d13r commented Mar 30, 2017

FYI I've worked out I can achieve my desired outcome with 'type' => '' (empty string):

$response = $client->search([
    'index' => 'myapp',
    'type' => $_GET['type'] ?? '',
    'body'  => /*...*/
]);

Still, better handling of null would be nice to have. 😃

@polyfractal
Copy link
Contributor

Agreed! Your suggested fix looks good, I need to patch up a few more (entirely unrelated) issues then I'll push a fix. :)

polyfractal added a commit that referenced this issue Apr 4, 2017
p365labs pushed a commit to p365labs/elasticsearch-php that referenced this issue Sep 10, 2017
p365labs pushed a commit to p365labs/elasticsearch-php that referenced this issue Sep 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants