You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by xiaojian zhou <zh...@gmail.com> on 2016/05/23 05:34:00 UTC

Review Request 47712: test perfield analyzer

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47712/
-----------------------------------------------------------

Review request for geode and Dan Smith.


Bugs: GEODE-1352
    https://issues.apache.org/jira/browse/GEODE-1352


Repository: geode


Description
-------

designed a lot cases to test query syntax related with analyzer


Diffs
-----

  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java 1e2b63d 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java e589ef4 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java a0319f4 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PdxLuceneSerializer.java c5c55a9 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionLuceneSerializer.java 953f31f 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIntegrationTest.java c302460 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java 15f5747 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java 571049c 

Diff: https://reviews.apache.org/r/47712/diff/


Testing
-------


Thanks,

xiaojian zhou


Re: Review Request 47712: test perfield analyzer

Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47712/#review134621
-----------------------------------------------------------


Fix it, then Ship it!




Looks good, and thank for the fix to the verify query method!

Some of the verifyQuery calls in shouldNotTokenizeWordsWithKeywordAnalyzerseem more like just examples and exploration of how the lucene query parser works, rather than asserts that are testing that geode uses the keyword analyzer. Maybe move them to another method or remove them?


geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java (line 71)
<https://reviews.apache.org/r/47712/#comment199462>

    Remove the commented out code - and the TODO line.



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java (line 140)
<https://reviews.apache.org/r/47712/#comment199463>

    In this test, it seems like you are not really using field1 for anything. Maybe just set that to the empty string and don't include it in the query? Otherwise it's just extra information someone has to wade through to understand the test.


- Dan Smith


On May 23, 2016, 6:22 a.m., xiaojian zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47712/
> -----------------------------------------------------------
> 
> (Updated May 23, 2016, 6:22 a.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: GEODE-1352
>     https://issues.apache.org/jira/browse/GEODE-1352
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> designed a lot cases to test query syntax related with analyzer
> 
> 
> Diffs
> -----
> 
>   geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java 1e2b63d 
>   geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java e589ef4 
>   geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java a0319f4 
>   geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PdxLuceneSerializer.java c5c55a9 
>   geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionLuceneSerializer.java 953f31f 
>   geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIntegrationTest.java c302460 
>   geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java 15f5747 
>   geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java dded69c 
>   geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java 70ec434 
>   geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java 571049c 
> 
> Diff: https://reviews.apache.org/r/47712/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>


Re: Review Request 47712: test perfield analyzer

Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47712/
-----------------------------------------------------------

(Updated May 23, 2016, 6:22 a.m.)


Review request for geode and Dan Smith.


Changes
-------

fixed a few more junit tests


Bugs: GEODE-1352
    https://issues.apache.org/jira/browse/GEODE-1352


Repository: geode


Description
-------

designed a lot cases to test query syntax related with analyzer


Diffs (updated)
-----

  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java 1e2b63d 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java e589ef4 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java a0319f4 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PdxLuceneSerializer.java c5c55a9 
  geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionLuceneSerializer.java 953f31f 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIntegrationTest.java c302460 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java 15f5747 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java dded69c 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java 70ec434 
  geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java 571049c 

Diff: https://reviews.apache.org/r/47712/diff/


Testing
-------


Thanks,

xiaojian zhou