Solr
  1. Solr
  2. SOLR-612

solrj should (optionally?) use POST for queries

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.3
    • Component/s: clients - java
    • Labels:
      None
    • Environment:

      all

      Description

      Can we make solrj always send post queries (or have it be an init-able option)?

      Jetty has some "problems" (in quotes because I don't know if it's really a problem) with long queries over GET:

      http://www.mail-archive.com/solr-user@lucene.apache.org/msg09457.html
      http://mail-archives.apache.org/mod_mbox/lucene-solr-user/200804.mbox/%3C47F50996.80705@umich.edu%3E

      Tiny patch attached that changes it and doesn't cause an exception on long queries in Jetty w/ solrj.

      1. solrj-post.diff
        0.6 kB
        Brian Whitman
      2. SOLR-612.patch
        2 kB
        Lars Kotthoff

        Issue Links

          Activity

          Hide
          Brian Whitman added a comment -

          patch

          Show
          Brian Whitman added a comment - patch
          Hide
          Lars Kotthoff added a comment -

          Linking this issue to SOLR-443 as changing from GET to POST has some rather nasty implications with regards to charsets.

          Show
          Lars Kotthoff added a comment - Linking this issue to SOLR-443 as changing from GET to POST has some rather nasty implications with regards to charsets.
          Hide
          Yonik Seeley added a comment -

          There are a number of advantages to GET I think... you can see the URLs in the servlet container logs, it's explicitly read-only, etc.
          People can already use POST by setting the method on SolrRequest object.
          Should we really change the default?

          Show
          Yonik Seeley added a comment - There are a number of advantages to GET I think... you can see the URLs in the servlet container logs, it's explicitly read-only, etc. People can already use POST by setting the method on SolrRequest object. Should we really change the default?
          Hide
          Brian Whitman added a comment -

          Well then maybe I just don't understand how to do it right – how do I tell solrj to use post, for this example:

          SolrServer s = new CommonsHttpSolrServer("http://localhost:8983/solr");
          SolrQuery q = new SolrQuery();
          q.setQuery("type:dog");
          QueryResponse qr = s.query(q);

          Where in there would I set SolrRequest to use POST?

          Show
          Brian Whitman added a comment - Well then maybe I just don't understand how to do it right – how do I tell solrj to use post, for this example: SolrServer s = new CommonsHttpSolrServer("http://localhost:8983/solr"); SolrQuery q = new SolrQuery(); q.setQuery("type:dog"); QueryResponse qr = s.query(q); Where in there would I set SolrRequest to use POST?
          Hide
          Yonik Seeley added a comment -

          Hmmm, I guess with .query() you can't. I was looking at .request() which takes a SolrRequest which has a setMethod() on it.

          Perhaps we should add a setter on CommonsHttpSolrServer that can override the "GET" of any SolrRequest and change it to "POST"?

          Show
          Yonik Seeley added a comment - Hmmm, I guess with .query() you can't. I was looking at .request() which takes a SolrRequest which has a setMethod() on it. Perhaps we should add a setter on CommonsHttpSolrServer that can override the "GET" of any SolrRequest and change it to "POST"?
          Hide
          Lars Kotthoff added a comment -

          IMHO the request method is a property of the request, not the server. Unfortunately it's not really feasible to add the method as an attribute to the request, as it's a subclass of SolrParams.

          I'm attaching a patch which adds a second query() method to SolrServer which takes the request method as the second argument. This way the method can be switched per call to query(), without having to change query or server object.

          Show
          Lars Kotthoff added a comment - IMHO the request method is a property of the request, not the server. Unfortunately it's not really feasible to add the method as an attribute to the request, as it's a subclass of SolrParams. I'm attaching a patch which adds a second query() method to SolrServer which takes the request method as the second argument. This way the method can be switched per call to query(), without having to change query or server object.
          Hide
          Yonik Seeley added a comment -

          I'm attaching a patch which adds a second query() method to SolrServer which takes the request method as the second argument.

          Shouldn't that be specific to an HTTP server?

          Show
          Yonik Seeley added a comment - I'm attaching a patch which adds a second query() method to SolrServer which takes the request method as the second argument. Shouldn't that be specific to an HTTP server?
          Hide
          Lars Kotthoff added a comment -

          Ah, good point. Yes, this should probably be moved to the http server implementation. SolrServer just occured to me as the most obvious place since the query() method is defined there.

          Incidentally, would it make sense to keep the second query method there but generalise the request method? Something to specify non-solr parameters with a request in general.

          Show
          Lars Kotthoff added a comment - Ah, good point. Yes, this should probably be moved to the http server implementation. SolrServer just occured to me as the most obvious place since the query() method is defined there. Incidentally, would it make sense to keep the second query method there but generalise the request method? Something to specify non-solr parameters with a request in general.
          Hide
          Lars Kotthoff added a comment -

          After having had a closer look at the code, I don't think that the query method which takes a METHOD as an argument should be specific to the HTTP server implementation – METHOD is defined in SolrRequest, i.e. it's not specific to HTTP requests. It looks like METHOD was really meant to provide a general way of passing non-solr parameters to requests.

          If we decided that the METHOD is really just HTTP specific and shouldn't be in the general request class – it's used nowhere but in the HTTP server – then a significant amount of refactoring will be necessary to implement everything properly. One way to do this would be to introduce a new interface, HttpParameters, and have a subclass for each type of request implement that interface. Then change the HTTP server implementation to only work with requests which implement that interface. This is probably big enough to be handled in a different issue though.

          Alternatively the METHOD could be moved to the HTTP server implementation, making it a property of the server and not of the request. I don't like that option though because different requests have different defaults for the METHOD (e.g. update requests POST, query requests GET) and moving this information requires the server to know how to handle which type of request.

          Show
          Lars Kotthoff added a comment - After having had a closer look at the code, I don't think that the query method which takes a METHOD as an argument should be specific to the HTTP server implementation – METHOD is defined in SolrRequest, i.e. it's not specific to HTTP requests. It looks like METHOD was really meant to provide a general way of passing non-solr parameters to requests. If we decided that the METHOD is really just HTTP specific and shouldn't be in the general request class – it's used nowhere but in the HTTP server – then a significant amount of refactoring will be necessary to implement everything properly. One way to do this would be to introduce a new interface, HttpParameters, and have a subclass for each type of request implement that interface. Then change the HTTP server implementation to only work with requests which implement that interface. This is probably big enough to be handled in a different issue though. Alternatively the METHOD could be moved to the HTTP server implementation, making it a property of the server and not of the request. I don't like that option though because different requests have different defaults for the METHOD (e.g. update requests POST, query requests GET) and moving this information requires the server to know how to handle which type of request.
          Hide
          Yonik Seeley added a comment -

          Lars, I've committed your version.
          Thanks guys!

          Show
          Yonik Seeley added a comment - Lars, I've committed your version. Thanks guys!

            People

            • Assignee:
              Unassigned
              Reporter:
              Brian Whitman
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development