[hibernate-issues] [Hibernate-JIRA] Commented: (HSEARCH-923) Add support for geospatial queries

[Emmanuel Bernard (JIRA)]

    [ http://opensource.atlassian.com/projects/hibernate/browse/HSEARCH-923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43644#comment-43644 ] 

Emmanuel Bernard commented on HSEARCH-923:
------------------------------------------

Yo, I've reviewed the code on Novacodex's repo, this looks good and simple. I was afraid I would not understand it ;P 

Here are the remarks I had on the fly.

h3. Feedback on Hibernate Search Geospatial module

h4. General

- Isolated project, Isolated module or integrated in HSearch?
- Is keep Git History important?
- package name?
- Hibernate Search 3.4 => Hibernate Search 4, that's the likely target.
- Eventually add some more genereral purpose comment or JavaDocs on main classes ( I know I don't do it often ;P )

In particular HSearch 4 has a few specificities:

- uses JBoss Logging (i18n logs and exceptions)
- splits API, SPI and implementation details in separate packages ( org.hibernate.foo, org.hibernate.foo.spi and org.hibernate.foo.impl)

h4. specifics

h5. FieldUtils

{code}    HSSI_Latitude_%s => %s_HSSI_Latitude ?{code}

ie. the field being selective.

h5. Point and Rectangle

Open question: is there a standard for such class or should we push our own version?

h5. SpatialFieldBridge

- Spaces at every line ;)
- MIN_GRID_LEVEL, MAX_GRID_LEVEL: parameters instead of constants? (see {{ParameterizedBridge}})
- use {{LuceneOptions}}

{code:java}
    //from
    document.add( new Field( FieldUtils.formatFieldName( i, name ), cellIds.get( i ), Field.Store.YES, Field.Index.NOT_ANALYZED_NO_NORMS ) );
    //to
    luceneOptions.addFieldToDocument( FieldUtils.formatFieldName( i, name ), cellIds.get( i ), document); //reuse store and analyzer options
{code}
Or is there a reason to store the data?

Same for numeric fields {{luceneOptions.addNumericFieldToDocument}}

h5. SpatialQueryBuilder

That's for a bit later but if integrated in the HSearch query DSL, we can make it even easier for users than this helper class

When to use GridFilter, when to use GridQuery?

build vs create prefix?

h5. GridManager

We tend to name these kind of static method holder classes Helper classes in HSearch, but nothing too serious.

h5. Tests

If integrated back into HSearch, we have some test harness that will help initialize Hibernate (Search).

For IndexingTest it's probably better to use an in-memory index to avoid the directory cleaning routine.

It's probably better to let the raw exception go through in the test to see the raw stacktrace when the test fails.

I have pushed some changes to the {{improvetests}} branch on novacodex repo to make IndexingTest more resilient.

h5. Benchmark

nanoTime is much more precise but I'm tempted to make the start / stop measurement outsite the loop (moving the session and transaction creation outside the loop).


> Add support for geospatial queries
> ----------------------------------
>
>                 Key: HSEARCH-923
>                 URL: http://opensource.atlassian.com/projects/hibernate/browse/HSEARCH-923
>             Project: Hibernate Search
>          Issue Type: New Feature
>          Components: query
>            Reporter: Emmanuel Bernard
>            Assignee: Nicolas Helleringer
>


--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira