> On June 14, 2016, 4:55 p.m., Dan Smith wrote: > > Hi Jason, > > > > It looks good, but I think the LuceneQueryProvider should not be passed a > > default field. The APIs on LuceneQueryFactory should be: > > > > create(String index, String region, String query, String defaultField) > > create(String index, String region, LuceneQueryProvider) /*NO DEFAULT FIELD > > HERE*/ > > > > The reason for that is that most of the use cases for the > > LuceneQueryProvider don't really have a concept of a default field. A > > default field is really specific to this StandardQueryParser, but the user > > may be constructing their query programatically and have no need for one. > > If they do need a default field, they can embed it in their > > LuceneQueryProvider. > > Jason Huynh wrote: > I think that makes sense. I added it due to the way we are > reusing/passing through that method. Let me take a look and see what I can > do...
What I meant to say is, I think what you are suggesting makes sense ;-) - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48695/#review137537 ----------------------------------------------------------- On June 14, 2016, 4:01 p.m., Jason Huynh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48695/ > ----------------------------------------------------------- > > (Updated June 14, 2016, 4:01 p.m.) > > > Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and > xiaojian zhou. > > > Repository: geode > > > Description > ------- > > This diff includes changes from changing the MultiFieldQueryParser to the > StandardQueryParser > > Propogates defaultField through the LuceneFunction > > Currently does not handle null, we can check for null defaultFields at the > query search level and use and internal default field? > > Was not sure if we should put default field into the provider itself, then it > would save having to pass the provider and the default field everywhere > > Currently most of our tests do not rely on the default field. The queries > ended up specifying the field and not using the default field. > > If anyone has preferences in parameter order for methods or any other > suggestions, please let me know. > > > Diffs > ----- > > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryFactory.java > 198961a > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryProvider.java > ef60158 > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImpl.java > 385b226 > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImpl.java > a876b40 > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java > 62cb65c > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java > 9567305 > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContext.java > b0b2c60 > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java > 26426ca > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java > af8c51f > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java > 92d8e8b > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java > c26997d > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java > 4bb67d2 > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImplJUnitTest.java > 975b92f > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImplIntegrationTest.java > 262efaa > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java > cfd8c32 > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContextJUnitTest.java > 39a4dde > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java > c1a64ae > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesFunctionCollectorJUnitTest.java > b7709bc > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java > 15ef449 > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java > 0cf8953 > > Diff: https://reviews.apache.org/r/48695/diff/ > > > Testing > ------- > > > Thanks, > > Jason Huynh > >
