HQLPARSER-72 Upgrade to search 5.6.0.Final #40
Conversation
| if ( fieldBridge instanceof NullEncodingTwoWayFieldBridge ) { | ||
| fieldBridge = ( (NullEncodingTwoWayFieldBridge) fieldBridge ).unwrap(); | ||
| if ( fieldBridge instanceof BridgeAdaptor ) { | ||
| fieldBridge = ( (BridgeAdaptor) fieldBridge ).unwrap( NumericFieldBridge.class ); |
There was a problem hiding this comment.
This is a bit dangerous, because if the field bridge is an adaptor over something else than a NumericFieldBridge, the field bridge will be null from there on. It'll work, but it's not obvious why.
Anyway... Inspecting bridges is a bit tricky. Since you seem to have access to metadata here, why don't you simply use org.hibernate.search.engine.metadata.impl.DocumentFieldMetadata.getNumericEncodingType() to retrieve the numeric encoding? It's also accessible through the public metadata API, mind you.
| @@ -111,24 +111,24 @@ public void shouldCreateQueryWithUnqualifiedPropertyReferences() { | |||
| public void shouldCreateNegatedQuery() { | |||
| assertLuceneQuery( | |||
| "from IndexedEntity e where NOT e.name = 'same'", | |||
| "-name:same *:*" ); | |||
| "-name:same #*:*" ); | |||
There was a problem hiding this comment.
This "#" seems weird. Why did you add this? Where did you find documentation about this syntax?
There was a problem hiding this comment.
This is the query generated when I upgrade the library and I don't change anything else. I thought it was due to the upgraded library and didn't think too much of it but it seems nobody knows why it is happening.
There was a problem hiding this comment.
I'll have an in-depth look
There was a problem hiding this comment.
that's the syntax for a "FILTER" clause type of Boolean Query. Looks correct 👍
| indexedEntityBridges.put( "position", NumericFieldBridge.LONG_FIELD_BRIDGE ); | ||
| indexedEntityBridges.put( "code", new NullEncodingTwoWayFieldBridge( NumericFieldBridge.LONG_FIELD_BRIDGE, "_null_" ) ); | ||
|
|
||
| indexedEntityBridges.put( "code", new NullEncodingTwoWayFieldBridge( NumericFieldBridge.LONG_FIELD_BRIDGE, stringNullCodec ) ); |
There was a problem hiding this comment.
I'm not sure about what was going on previously here, but I know that we currently do not support having a non-numeric null marker on an otherwise numeric field. At least the default numeric field bridges will only create numeric null markers. So this kind of setup ("_null_" used as a marker on a numeric field) cannot happen in practice when you're using Hibernate Search.
To be in line with real-world use cases, you should use a LuceneLongNullMarkerCodec here, since code seems to be a numeric (long) field. Also, the null marker should be retrieved from the bridge itself. Something like NumericFieldBridge.LONG_FIELD_BRIDGE.createNullMarker( "-1" ).
The related code in Hibernate Search is in org.hibernate.search.engine.nulls.impl.LuceneMissingValueStrategy.createNullMarkerCodec(Class<?>, DocumentFieldPath, NullMarker) and org.hibernate.search.engine.metadata.impl.AnnotationMetadataProvider.createNullMarker(FieldBridge, String, DocumentFieldPath) (both called by org.hibernate.search.engine.metadata.impl.AnnotationMetadataProvider.determineNullMarkerCodec(FieldBridge, Field, ConfigContext, ParseContext, Class<?>, DocumentFieldPath)).
| @@ -73,10 +73,10 @@ private Object convertToPropertyType(String entityType, List<String> propertyPat | |||
| //Order matters! Some types are subclasses of others | |||
| //TODO expose something in Hibernate Search so that we can avoid this horrible code | |||
| if ( bridge instanceof NullEncodingTwoWayFieldBridge ) { | |||
| return convertToPropertyType( entityType, propertyPath, value, ( (NullEncodingTwoWayFieldBridge) bridge ).unwrap() ); | |||
| return convertToPropertyType( entityType, propertyPath, value, ( (NullEncodingTwoWayFieldBridge) bridge ).unwrap( NullEncodingTwoWayFieldBridge.class ) ); | |||
There was a problem hiding this comment.
This will most likely result in a null unwrapped bridge, since BridgeAdaptor.unwrap doesn't take the adaptor itself into account.
Anyway... here too, can't we simply use the metadata? Add an abstract getNumericEncoding method instead of introspecting field bridges? It would seem simpler and easier to maintain...
The goal is to use the NumericEncodingType when possible instead of messing around with field bridges.
|
@yrodiere I did some refactoring and tried to applied what you suggested. |
|
merged, thanks! |
https://hibernate.atlassian.net/browse/HQLPARSER-72
Can you guys have a quick look at it?
I tried to stick to the existing code although it looks a weird in a couple of places (where we use the unwrap)