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 2017/03/07 22:03:29 UTC

Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

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

(Updated March 7, 2017, 10:03 p.m.)


Review request for geode, Barry Oglesby and Dan Smith.


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


Repository: geode


Description
-------

If customer call a lucene query in a function, they need to define their own Serializable class to collect results. This has been complained by field engineer. 

It will not hurt to let LuceneResultStruct to extend Serializable


Diffs
-----

  geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneResultStruct.java b922185 


Diff: https://reviews.apache.org/r/57206/diff/1/


Testing
-------


Thanks,

xiaojian zhou


Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

Posted by Dan Smith <ds...@pivotal.io>.
On Tue, Mar 7, 2017 at 2:30 PM, Xiaojian Zhou <gz...@pivotal.io> wrote:

> If so, we have to implement fromData() and toData() in
> LuceneResultStructImpl.java for K and V. But we don't know their type yet.
>
> You don't need to know the type, you just call DataSerializer.writeObject.

-Dan

Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

Posted by Xiaojian Zhou <gz...@pivotal.io>.
If so, we have to implement fromData() and toData() in
LuceneResultStructImpl.java for K and V. But we don't know their type yet.

On Tue, Mar 7, 2017 at 2:25 PM, Dan Smith <ds...@pivotal.io> wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57206/#review168203
> -----------------------------------------------------------
>
>
>
> It looks good - my one question is if this should be
> DataSerializableFixedID or something like that, so that it can be
> serialized more efficiently and we can manage backwards compatibility?
>
> - Dan Smith
>
>
> On March 7, 2017, 10:03 p.m., xiaojian zhou wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/57206/
> > -----------------------------------------------------------
> >
> > (Updated March 7, 2017, 10:03 p.m.)
> >
> >
> > Review request for geode, Barry Oglesby and Dan Smith.
> >
> >
> > Bugs: GEODE-2617
> >     https://issues.apache.org/jira/browse/GEODE-2617
> >
> >
> > Repository: geode
> >
> >
> > Description
> > -------
> >
> > If customer call a lucene query in a function, they need to define their
> own Serializable class to collect results. This has been complained by
> field engineer.
> >
> > It will not hurt to let LuceneResultStruct to extend Serializable
> >
> >
> > Diffs
> > -----
> >
> >   geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneResultStruct.java
> b922185
> >
> >
> > Diff: https://reviews.apache.org/r/57206/diff/1/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > xiaojian zhou
> >
> >
>
>

Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

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



It looks good - my one question is if this should be DataSerializableFixedID or something like that, so that it can be serialized more efficiently and we can manage backwards compatibility?

- Dan Smith


On March 7, 2017, 10:03 p.m., xiaojian zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57206/
> -----------------------------------------------------------
> 
> (Updated March 7, 2017, 10:03 p.m.)
> 
> 
> Review request for geode, Barry Oglesby and Dan Smith.
> 
> 
> Bugs: GEODE-2617
>     https://issues.apache.org/jira/browse/GEODE-2617
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> If customer call a lucene query in a function, they need to define their own Serializable class to collect results. This has been complained by field engineer. 
> 
> It will not hurt to let LuceneResultStruct to extend Serializable
> 
> 
> Diffs
> -----
> 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneResultStruct.java b922185 
> 
> 
> Diff: https://reviews.apache.org/r/57206/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>


Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

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

(Updated March 9, 2017, 1:20 a.m.)


Review request for geode, Barry Oglesby and Dan Smith.


Changes
-------

another option to fix


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


Repository: geode


Description
-------

If customer call a lucene query in a function, they need to define their own Serializable class to collect results. This has been complained by field engineer. 

It will not hurt to let LuceneResultStruct to extend Serializable


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java 63a95a5 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneResultStruct.java 5d2184e 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneResultStructImpl.java 6c31025 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java 5c908f5 


Diff: https://reviews.apache.org/r/57206/diff/3/

Changes: https://reviews.apache.org/r/57206/diff/2-3/


Testing
-------


Thanks,

xiaojian zhou


Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

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



Did this work? I think you need to register the DSFID. See LuceneServiceImpl.registerDataSerializables.

- Dan Smith


On March 8, 2017, 5:49 a.m., xiaojian zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57206/
> -----------------------------------------------------------
> 
> (Updated March 8, 2017, 5:49 a.m.)
> 
> 
> Review request for geode, Barry Oglesby and Dan Smith.
> 
> 
> Bugs: GEODE-2617
>     https://issues.apache.org/jira/browse/GEODE-2617
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> If customer call a lucene query in a function, they need to define their own Serializable class to collect results. This has been complained by field engineer. 
> 
> It will not hurt to let LuceneResultStruct to extend Serializable
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java 63a95a5 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneResultStruct.java 5d2184e 
>   geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneResultStructImpl.java 6c31025 
> 
> 
> Diff: https://reviews.apache.org/r/57206/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>


Re: Review Request 57206: let LuceneResultStruct to be Serializable, then customer does not have to defined their Serializable class to hold result

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

(Updated March 8, 2017, 5:49 a.m.)


Review request for geode, Barry Oglesby and Dan Smith.


Changes
-------

let LuceneResultStructImpl to implements DataSerializableFixedID


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


Repository: geode


Description
-------

If customer call a lucene query in a function, they need to define their own Serializable class to collect results. This has been complained by field engineer. 

It will not hurt to let LuceneResultStruct to extend Serializable


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java 63a95a5 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneResultStruct.java 5d2184e 
  geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneResultStructImpl.java 6c31025 


Diff: https://reviews.apache.org/r/57206/diff/2/

Changes: https://reviews.apache.org/r/57206/diff/1-2/


Testing
-------


Thanks,

xiaojian zhou