You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by Kasper Sørensen <i....@gmail.com> on 2014/10/19 12:25:49 UTC
Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26909/
-----------------------------------------------------------
Review request for MetaModel.
Bugs: METAMODEL-84
https://issues.apache.org/jira/browse/METAMODEL-84
Repository: metamodel
Description
-------
My proposed fix for METAMODEL-84
Diffs
-----
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java a7b00c9
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java c7e5fe9
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java 04991fe
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java PRE-CREATION
elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java d1794fc
elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java d7e62e6
Diff: https://reviews.apache.org/r/26909/diff/
Testing
-------
Added unittests.
Also accidentally encountered a (unrelated) issue with date values in ElasticSearch. Added a reproduction unittest with @Ignore (so that it would not block this work).
Thanks,
Kasper Sørensen
Re: Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
Posted by Alberto Rodriguez <ar...@stratio.com>.
Thank you for replying Kasper!
Sounds good to me then.
Kind regards,
Alberto Rodríguez
<http://www.stratio.com/>
Avenida de Europa, 26. Ática 5. 3ª Planta
28224 Pozuelo de Alarcón, Madrid
Tel: +34 91 352 59 42 // *@stratiobd <https://twitter.com/StratioBD>*
2014-10-21 19:05 GMT+02:00 Kasper Sørensen <i....@gmail.com>:
> I'd say it's pretty consistent from a outside view, since the other
> datastores that resemble ElasticSearch (MongoDB, CouchDB and HBase) all
> have such an "artificial" id column. But in MongoDb specifically it is
> implemented adhering to the native metadata API, since it delivers the id
> column type as an ObjectId, which is something we can react to
> specifically. In HBase, CouchDb and now ElasticSearch, we explicitly add an
> "_id" column.
>
> 2014-10-21 16:34 GMT+02:00 Alberto Rodriguez <ar...@stratio.com>:
>
> > Hi Kasper,
> >
> > Your changes look fine to me, just one question: you are now adding the
> > FIELD_ID within the metadata... is that consistent with the other
> modules?
> > For instance, for the Mongo connector are we also adding the objectid
> > within the metadata?
> >
> > Kind regards,
> >
> >
> > Alberto Rodríguez
> >
> >
> > <http://www.stratio.com/>
> > Avenida de Europa, 26. Ática 5. 3ª Planta
> > 28224 Pozuelo de Alarcón, Madrid
> > Tel: +34 91 352 59 42 // *@stratiobd <https://twitter.com/StratioBD>*
> >
> > 2014-10-21 13:48 GMT+02:00 Kasper Sørensen <
> i.am.kasper.sorensen@gmail.com
> > >:
> >
> > > Anyone wants to give a review for this? Or else I will push it as per
> > lazy
> > > concensus.
> > >
> > > 2014-10-19 12:30 GMT+02:00 Kasper Sørensen <
> > i.am.kasper.sorensen@gmail.com
> > > >:
> > >
> > > > This is an automatically generated e-mail. To reply, visit:
> > > > https://reviews.apache.org/r/26909/
> > > > Review request for MetaModel.
> > > > By Kasper Sørensen.
> > > >
> > > > *Updated okt. 19, 2014, 10:30 a.m.*
> > > > *Bugs: * METAMODEL-84
> > > > <https://issues.apache.org/jira/browse/METAMODEL-84>
> > > > *Repository: * metamodel
> > > > Description
> > > >
> > > > My proposed fix for METAMODEL-84
> > > >
> > > > Testing (updated)
> > > >
> > > > Added unittests.
> > > >
> > > > Also accidentally encountered a (unrelated) issue with date values in
> > > ElasticSearch. Added a reproduction unittest with @Ignore (so that it
> > would
> > > not block this work). Reported this other issue here:
> > > https://issues.apache.org/jira/browse/METAMODEL-87
> > > >
> > > > Diffs
> > > >
> > > > -
> > >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> > > > (a7b00c9)
> > > > -
> > >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java
> > > > (c7e5fe9)
> > > > -
> > >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java
> > > > (04991fe)
> > > > -
> > >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java
> > > > (PRE-CREATION)
> > > > -
> > >
> >
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> > > > (d1794fc)
> > > > -
> > >
> >
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java
> > > > (d7e62e6)
> > > >
> > > > View Diff <https://reviews.apache.org/r/26909/diff/>
> > > >
> > >
> >
>
Re: Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
Posted by Kasper Sørensen <i....@gmail.com>.
I'd say it's pretty consistent from a outside view, since the other
datastores that resemble ElasticSearch (MongoDB, CouchDB and HBase) all
have such an "artificial" id column. But in MongoDb specifically it is
implemented adhering to the native metadata API, since it delivers the id
column type as an ObjectId, which is something we can react to
specifically. In HBase, CouchDb and now ElasticSearch, we explicitly add an
"_id" column.
2014-10-21 16:34 GMT+02:00 Alberto Rodriguez <ar...@stratio.com>:
> Hi Kasper,
>
> Your changes look fine to me, just one question: you are now adding the
> FIELD_ID within the metadata... is that consistent with the other modules?
> For instance, for the Mongo connector are we also adding the objectid
> within the metadata?
>
> Kind regards,
>
>
> Alberto Rodríguez
>
>
> <http://www.stratio.com/>
> Avenida de Europa, 26. Ática 5. 3ª Planta
> 28224 Pozuelo de Alarcón, Madrid
> Tel: +34 91 352 59 42 // *@stratiobd <https://twitter.com/StratioBD>*
>
> 2014-10-21 13:48 GMT+02:00 Kasper Sørensen <i.am.kasper.sorensen@gmail.com
> >:
>
> > Anyone wants to give a review for this? Or else I will push it as per
> lazy
> > concensus.
> >
> > 2014-10-19 12:30 GMT+02:00 Kasper Sørensen <
> i.am.kasper.sorensen@gmail.com
> > >:
> >
> > > This is an automatically generated e-mail. To reply, visit:
> > > https://reviews.apache.org/r/26909/
> > > Review request for MetaModel.
> > > By Kasper Sørensen.
> > >
> > > *Updated okt. 19, 2014, 10:30 a.m.*
> > > *Bugs: * METAMODEL-84
> > > <https://issues.apache.org/jira/browse/METAMODEL-84>
> > > *Repository: * metamodel
> > > Description
> > >
> > > My proposed fix for METAMODEL-84
> > >
> > > Testing (updated)
> > >
> > > Added unittests.
> > >
> > > Also accidentally encountered a (unrelated) issue with date values in
> > ElasticSearch. Added a reproduction unittest with @Ignore (so that it
> would
> > not block this work). Reported this other issue here:
> > https://issues.apache.org/jira/browse/METAMODEL-87
> > >
> > > Diffs
> > >
> > > -
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> > > (a7b00c9)
> > > -
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java
> > > (c7e5fe9)
> > > -
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java
> > > (04991fe)
> > > -
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java
> > > (PRE-CREATION)
> > > -
> >
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> > > (d1794fc)
> > > -
> >
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java
> > > (d7e62e6)
> > >
> > > View Diff <https://reviews.apache.org/r/26909/diff/>
> > >
> >
>
Re: Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
Posted by Alberto Rodriguez <ar...@stratio.com>.
Hi Kasper,
Your changes look fine to me, just one question: you are now adding the
FIELD_ID within the metadata... is that consistent with the other modules?
For instance, for the Mongo connector are we also adding the objectid
within the metadata?
Kind regards,
Alberto Rodríguez
<http://www.stratio.com/>
Avenida de Europa, 26. Ática 5. 3ª Planta
28224 Pozuelo de Alarcón, Madrid
Tel: +34 91 352 59 42 // *@stratiobd <https://twitter.com/StratioBD>*
2014-10-21 13:48 GMT+02:00 Kasper Sørensen <i....@gmail.com>:
> Anyone wants to give a review for this? Or else I will push it as per lazy
> concensus.
>
> 2014-10-19 12:30 GMT+02:00 Kasper Sørensen <i.am.kasper.sorensen@gmail.com
> >:
>
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/26909/
> > Review request for MetaModel.
> > By Kasper Sørensen.
> >
> > *Updated okt. 19, 2014, 10:30 a.m.*
> > *Bugs: * METAMODEL-84
> > <https://issues.apache.org/jira/browse/METAMODEL-84>
> > *Repository: * metamodel
> > Description
> >
> > My proposed fix for METAMODEL-84
> >
> > Testing (updated)
> >
> > Added unittests.
> >
> > Also accidentally encountered a (unrelated) issue with date values in
> ElasticSearch. Added a reproduction unittest with @Ignore (so that it would
> not block this work). Reported this other issue here:
> https://issues.apache.org/jira/browse/METAMODEL-87
> >
> > Diffs
> >
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> > (a7b00c9)
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java
> > (c7e5fe9)
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java
> > (04991fe)
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java
> > (PRE-CREATION)
> > -
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> > (d1794fc)
> > -
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java
> > (d7e62e6)
> >
> > View Diff <https://reviews.apache.org/r/26909/diff/>
> >
>
Re: Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
Posted by Alberto Rodriguez <ar...@stratio.com>.
Hi Kasper,
I will have a look, I've been quite busy the last days but I will try
review it this evening.
Kind regards,
Alberto Rodríguez
<http://www.stratio.com/>
Avenida de Europa, 26. Ática 5. 3ª Planta
28224 Pozuelo de Alarcón, Madrid
Tel: +34 91 352 59 42 // *@stratiobd <https://twitter.com/StratioBD>*
2014-10-21 13:48 GMT+02:00 Kasper Sørensen <i....@gmail.com>:
> Anyone wants to give a review for this? Or else I will push it as per lazy
> concensus.
>
> 2014-10-19 12:30 GMT+02:00 Kasper Sørensen <i.am.kasper.sorensen@gmail.com
> >:
>
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/26909/
> > Review request for MetaModel.
> > By Kasper Sørensen.
> >
> > *Updated okt. 19, 2014, 10:30 a.m.*
> > *Bugs: * METAMODEL-84
> > <https://issues.apache.org/jira/browse/METAMODEL-84>
> > *Repository: * metamodel
> > Description
> >
> > My proposed fix for METAMODEL-84
> >
> > Testing (updated)
> >
> > Added unittests.
> >
> > Also accidentally encountered a (unrelated) issue with date values in
> ElasticSearch. Added a reproduction unittest with @Ignore (so that it would
> not block this work). Reported this other issue here:
> https://issues.apache.org/jira/browse/METAMODEL-87
> >
> > Diffs
> >
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> > (a7b00c9)
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java
> > (c7e5fe9)
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java
> > (04991fe)
> > -
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java
> > (PRE-CREATION)
> > -
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> > (d1794fc)
> > -
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java
> > (d7e62e6)
> >
> > View Diff <https://reviews.apache.org/r/26909/diff/>
> >
>
Re: Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
Posted by Kasper Sørensen <i....@gmail.com>.
Anyone wants to give a review for this? Or else I will push it as per lazy
concensus.
2014-10-19 12:30 GMT+02:00 Kasper Sørensen <i....@gmail.com>:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26909/
> Review request for MetaModel.
> By Kasper Sørensen.
>
> *Updated okt. 19, 2014, 10:30 a.m.*
> *Bugs: * METAMODEL-84
> <https://issues.apache.org/jira/browse/METAMODEL-84>
> *Repository: * metamodel
> Description
>
> My proposed fix for METAMODEL-84
>
> Testing (updated)
>
> Added unittests.
>
> Also accidentally encountered a (unrelated) issue with date values in ElasticSearch. Added a reproduction unittest with @Ignore (so that it would not block this work). Reported this other issue here: https://issues.apache.org/jira/browse/METAMODEL-87
>
> Diffs
>
> - elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> (a7b00c9)
> - elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java
> (c7e5fe9)
> - elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java
> (04991fe)
> - elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java
> (PRE-CREATION)
> - elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> (d1794fc)
> - elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java
> (d7e62e6)
>
> View Diff <https://reviews.apache.org/r/26909/diff/>
>
Re: Review Request 26909: METAMODEL-84: Use ElasticSearch doc id as
Primary Key
Posted by Kasper Sørensen <i....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26909/
-----------------------------------------------------------
(Updated okt. 19, 2014, 10:30 a.m.)
Review request for MetaModel.
Bugs: METAMODEL-84
https://issues.apache.org/jira/browse/METAMODEL-84
Repository: metamodel
Description
-------
My proposed fix for METAMODEL-84
Diffs
-----
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java a7b00c9
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java c7e5fe9
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParser.java 04991fe
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUtils.java PRE-CREATION
elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java d1794fc
elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchMetaDataParserTest.java d7e62e6
Diff: https://reviews.apache.org/r/26909/diff/
Testing (updated)
-------
Added unittests.
Also accidentally encountered a (unrelated) issue with date values in ElasticSearch. Added a reproduction unittest with @Ignore (so that it would not block this work). Reported this other issue here: https://issues.apache.org/jira/browse/METAMODEL-87
Thanks,
Kasper Sørensen