Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9146

Parallel SQL engine should support >, >=, <, <=, <>, != syntax

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.3
    • Component/s: Parallel SQL
    • Labels:
      None

      Description

      this gives expected result:

           SELECT title_s, COUNT(*) as cnt
              FROM movielens
           WHERE genre_ss='action' AND rating_i='[4 TO 5]'
      GROUP BY title_s
      ORDER BY cnt desc
               LIMIT 5
      

      but using >= 4 doesn't give same results (my ratings are 1-5):

            SELECT title_s, COUNT(*) as cnt
               FROM movielens
            WHERE genre_ss='action' AND rating_i >= 4
      GROUP BY title_s
      ORDER BY cnt desc
                LIMIT 5
      

      on the Solr side, I see queries forumlated as:

      2016-05-21 14:53:43.096 INFO  (qtp1435804085-1419) [c:movielens
      s:shard1 r:core_node1 x:movielens_shard1_replica1] o.a.s.c.S.Request
      [movielens_shard1_replica1]  webapp=/solr path=/export
      params={q=((genre_ss:"action")+AND+(rating_i:"4"))&distrib=false&fl=title_s&sort=title_s+desc&wt=json&version=2.2}
      hits=2044 status=0 QTime=0
      

      which is obviously wrong ...

      In general, rather than crafting an incorrect query that gives the
      wrong results, we should throw an exception stating that the syntax is
      not supported.

      Also, the ref guide should be updated to contain a known limitations section so users don't have to guess at what SQL features are supported by Solr.

      1. SOLR-9146.patch
        12 kB
        Kevin Risden
      2. SOLR-9146.patch
        7 kB
        Kevin Risden

        Issue Links

          Activity

          Hide
          risdenk Kevin Risden added a comment -

          Also, the ref guide should be updated to contain a known limitations section so users don't have to guess at what SQL features are supported by Solr.

          I absolutely agree with this. Currently it would be easier to start with a what is supported section by taking the tests (TestSQLHandler and JDBCTest) as a place to start that list.

          Show
          risdenk Kevin Risden added a comment - Also, the ref guide should be updated to contain a known limitations section so users don't have to guess at what SQL features are supported by Solr. I absolutely agree with this. Currently it would be easier to start with a what is supported section by taking the tests (TestSQLHandler and JDBCTest) as a place to start that list.
          Hide
          risdenk Kevin Risden added a comment -

          I had forgotten about this and finally had time to look at this closer. I am currently testing a patch that will at least make this start to work. The better fix would be to get the integration of Calcite in (SOLR-8593) so we can rely on that for parsing things we don't natively support.

          Show
          risdenk Kevin Risden added a comment - I had forgotten about this and finally had time to look at this closer. I am currently testing a patch that will at least make this start to work. The better fix would be to get the integration of Calcite in ( SOLR-8593 ) so we can rely on that for parsing things we don't natively support.
          Hide
          risdenk Kevin Risden added a comment -

          Attaching a patch with some added tests. This at least makes rudimentary >, >=, <, <=, <>, != syntax work. I didn't go much further than checking strings and numbers work.

          Show
          risdenk Kevin Risden added a comment - Attaching a patch with some added tests. This at least makes rudimentary >, >=, <, <=, <>, != syntax work. I didn't go much further than checking strings and numbers work.
          Hide
          risdenk Kevin Risden added a comment -

          Timothy Potter / Joel Bernstein - Can you take a peek?

          Show
          risdenk Kevin Risden added a comment - Timothy Potter / Joel Bernstein - Can you take a peek?
          Hide
          joel.bernstein Joel Bernstein added a comment -

          Patch looks good!

          Maybe just a few tests running some queries with the new syntax.

          Show
          joel.bernstein Joel Bernstein added a comment - Patch looks good! Maybe just a few tests running some queries with the new syntax.
          Hide
          risdenk Kevin Risden added a comment -

          Thanks I'll add some more tests that are actual queries instead of just the parsing piece.

          Show
          risdenk Kevin Risden added a comment - Thanks I'll add some more tests that are actual queries instead of just the parsing piece.
          Hide
          risdenk Kevin Risden added a comment -

          Added some tests with actual query requests checking.

          I noticed something interesting about pure negative queries. If using /select handler (with limit) then pure negative queries work (https://wiki.apache.org/solr/NegativeQueryProblems). However the /export handler doesn't seem to have the same optimization for pure negative queries. Does this sound plausible Joel Bernstein?

          Show
          risdenk Kevin Risden added a comment - Added some tests with actual query requests checking. I noticed something interesting about pure negative queries. If using /select handler (with limit) then pure negative queries work ( https://wiki.apache.org/solr/NegativeQueryProblems ). However the /export handler doesn't seem to have the same optimization for pure negative queries. Does this sound plausible Joel Bernstein ?
          Hide
          joel.bernstein Joel Bernstein added a comment -

          New patch looks great.

          +1 to commit.

          Not sure about the negative queries and the /export handler. The /export hander still uses the QueryComponent so in theory it would evaluate negative queries the same way.

          Show
          joel.bernstein Joel Bernstein added a comment - New patch looks great. +1 to commit. Not sure about the negative queries and the /export handler. The /export hander still uses the QueryComponent so in theory it would evaluate negative queries the same way.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 87bad0956079b3d6b5b634fa17c0ee057cb42161 in lucene-solr's branch refs/heads/master from Kevin Risden
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=87bad09 ]

          SOLR-9146: Parallel SQL engine should support >, >=, <, <=, <>, != syntax

          Show
          jira-bot ASF subversion and git services added a comment - Commit 87bad0956079b3d6b5b634fa17c0ee057cb42161 in lucene-solr's branch refs/heads/master from Kevin Risden [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=87bad09 ] SOLR-9146 : Parallel SQL engine should support >, >=, <, <=, <>, != syntax
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b1a5c5f92e41f7180fdc0af5811bd4346a42af28 in lucene-solr's branch refs/heads/branch_6x from Kevin Risden
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b1a5c5f ]

          SOLR-9146: Parallel SQL engine should support >, >=, <, <=, <>, != syntax

          Show
          jira-bot ASF subversion and git services added a comment - Commit b1a5c5f92e41f7180fdc0af5811bd4346a42af28 in lucene-solr's branch refs/heads/branch_6x from Kevin Risden [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b1a5c5f ] SOLR-9146 : Parallel SQL engine should support >, >=, <, <=, <>, != syntax
          Hide
          thelabdude Timothy Potter added a comment -

          +1 ... nice addition!

          Show
          thelabdude Timothy Potter added a comment - +1 ... nice addition!
          Hide
          ctargett Cassandra Targett added a comment -

          Kevin Risden: I added a section to the Solr Ref Guide about supported WHERE operators at https://cwiki.apache.org/confluence/display/solr/Parallel+SQL+Interface#ParallelSQLInterface-SupportedWHEREOperators. However, I did not include '!=' because I didn't see it in the TestSQLHandler.java test. Let me know if it is supported even if it is not tested. If I should add other operators that are not supported, please let me know - I used a general list of operators and some feedback from the page comments.

          Due to time constraints, I did not try to cover every scenario demonstrated in the test, just wanted to add a section based on this specific issue.

          Show
          ctargett Cassandra Targett added a comment - Kevin Risden : I added a section to the Solr Ref Guide about supported WHERE operators at https://cwiki.apache.org/confluence/display/solr/Parallel+SQL+Interface#ParallelSQLInterface-SupportedWHEREOperators . However, I did not include '!=' because I didn't see it in the TestSQLHandler.java test. Let me know if it is supported even if it is not tested. If I should add other operators that are not supported, please let me know - I used a general list of operators and some feedback from the page comments. Due to time constraints, I did not try to cover every scenario demonstrated in the test, just wanted to add a section based on this specific issue.
          Hide
          risdenk Kevin Risden added a comment -

          Thanks Cassandra Targett! I'll double check the ref guide page. There are some caveats right now to the WHERE support (like one side needs to be a field.) Both sides being constants (5 < 10) or being fields (fielda > fieldb) are not supported. This hopefully gets addressed with SOLR-8593. I can add this to the ref guide page. I'm pretty sure '!=' works (even though it is not standard sql like <>) I'll double check.

          Show
          risdenk Kevin Risden added a comment - Thanks Cassandra Targett ! I'll double check the ref guide page. There are some caveats right now to the WHERE support (like one side needs to be a field.) Both sides being constants (5 < 10) or being fields (fielda > fieldb) are not supported. This hopefully gets addressed with SOLR-8593 . I can add this to the ref guide page. I'm pretty sure '!=' works (even though it is not standard sql like <>) I'll double check.
          Hide
          risdenk Kevin Risden added a comment -

          Expanded the supported where operators to include some examples. Also added a note about the where clause requiring a field on one side of the predicate.

          Show
          risdenk Kevin Risden added a comment - Expanded the supported where operators to include some examples. Also added a note about the where clause requiring a field on one side of the predicate.
          Hide
          ctargett Cassandra Targett added a comment -

          Thanks so much for your help.

          Show
          ctargett Cassandra Targett added a comment - Thanks so much for your help.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Closing after 6.3.0 release.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.

            People

            • Assignee:
              risdenk Kevin Risden
              Reporter:
              thelabdude Timothy Potter
            • Votes:
              1 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development