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 2015/02/15 22:22:30 UTC

Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

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

Review request for MetaModel.

Bugs: METAMODEL-79
https://issues.apache.org/jira/browse/METAMODEL-79

Repository: metamodel

Description
-------

Partial fix for METAMODEL-79 - see description

Diffs
-----

elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java 06353f1
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDropTableBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchInsertIntoBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUpdateCallback.java PRE-CREATION
elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java 449490b

Diff: https://reviews.apache.org/r/31066/diff/

Testing
-------

This is my initial/partial fix for METAMODEL-79. I want to share it because 1) there's more to come but I want to confirm that I'm on the right way and 2) I have questions for experts on E.S. :-)

This patch adds support for the MetaModel ElasticSearch to do INSERT INTO, CREATE TABLE and DROP TABLE statements.

It does not (yet) have support for UPDATE or DELETE FROM statements. I wanted to validate my initial work first.

And I have a few questions regarding types and mappings.

* Please check the ElasticSearchCreateTableBuilder.getType(Column) method. Here I've attempted to convert ColumnTypes to ElasticSearch types. Are these correct? I am not sure about generalizations such as the NUMERIC -> "double" mapping etc.
* As a last resort I have used the type "object". But when I tried it out I ran into the problem that "object" is not polymorphic like in Java, it is actually the opposite of a "value type". So that means you cannot define an "object" field and then insert a single value into it. This makes it a bad fit for a "fallback" type. Is there a better way? Should we then simply NOT define the field in the mapping maybe?

Thanks,

Kasper Sørensen

Re: Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

Posted by Dennis Krøger <De...@humaninference.com>.

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


Looks good, except the noted issues. But it's a big change, so it probably needs more (and more MM experienced) eyes.


elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java
<https://reviews.apache.org/r/31066/#comment122903>

    Reject mapping instead, _object_ is not useable for this purpose



elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java
<https://reviews.apache.org/r/31066/#comment122904>

    Reject mapping instead, _object_ is not useable as a generic type.



elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDeleteBuilder.java
<https://reviews.apache.org/r/31066/#comment122900>

    createQueryBuilderForSimpleWhere already does this for you. No need to do it again.


- Dennis Krøger


On March 8, 2015, 3:12 p.m., Kasper Sørensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31066/
> -----------------------------------------------------------
> 
> (Updated March 8, 2015, 3:12 p.m.)
> 
> 
> Review request for MetaModel.
> 
> 
> Bugs: METAMODEL-79
>     https://issues.apache.org/jira/browse/METAMODEL-79
> 
> 
> Repository: metamodel
> 
> 
> Description
> -------
> 
> Partial fix for METAMODEL-79 - see description
> 
> 
> Diffs
> -----
> 
>   CHANGES.md 5c2e2eb 
>   core/src/main/java/org/apache/metamodel/MetaModelHelper.java 4ee9798 
>   core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java 0eba921 
>   core/src/main/java/org/apache/metamodel/query/FilterClause.java fe981a1 
>   core/src/test/java/org/apache/metamodel/QueryPostprocessDataContextTest.java 3fdb711 
>   core/src/test/java/org/apache/metamodel/query/FilterItemTest.java 2fedfb9 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java 06353f1 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java e4f1054 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDeleteBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDropTableBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchInsertBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUpdateCallback.java PRE-CREATION 
>   elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java 449490b 
> 
> Diff: https://reviews.apache.org/r/31066/diff/
> 
> 
> Testing
> -------
> 
> This is my initial/partial fix for METAMODEL-79. I want to share it because 1) there's more to come but I want to confirm that I'm on the right way and 2) I have questions for experts on E.S. :-)
> 
> This patch adds support for the MetaModel ElasticSearch to do INSERT INTO, CREATE TABLE and DROP TABLE statements.
> 
> It does not (yet) have support for UPDATE or DELETE FROM statements. I wanted to validate my initial work first.
> 
> And I have a few questions regarding types and mappings.
> 
>  * Please check the ElasticSearchCreateTableBuilder.getType(Column) method. Here I've attempted to convert ColumnTypes to ElasticSearch types. Are these correct? I am not sure about generalizations such as the NUMERIC -> "double" mapping etc.
>  * As a last resort I have used the type "object". But when I tried it out I ran into the problem that "object" is not polymorphic like in Java, it is actually the opposite of a "value type". So that means you cannot define an "object" field and then insert a single value into it. This makes it a bad fit for a "fallback" type. Is there a better way? Should we then simply NOT define the field in the mapping maybe?
> 
> 
> Thanks,
> 
> Kasper Sørensen
> 
>

Re: Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

Posted by Kasper Sørensen <i....@gmail.com>.

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

Ship it!


Ship It!

- Kasper Sørensen


On marts 9, 2015, 7:51 p.m., Kasper Sørensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31066/
> -----------------------------------------------------------
> 
> (Updated marts 9, 2015, 7:51 p.m.)
> 
> 
> Review request for MetaModel.
> 
> 
> Bugs: METAMODEL-79
>     https://issues.apache.org/jira/browse/METAMODEL-79
> 
> 
> Repository: metamodel
> 
> 
> Description
> -------
> 
> Partial fix for METAMODEL-79 - see description
> 
> 
> Diffs
> -----
> 
>   CHANGES.md 5c2e2eb 
>   core/src/main/java/org/apache/metamodel/MetaModelHelper.java 4ee9798 
>   core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java 0eba921 
>   core/src/main/java/org/apache/metamodel/query/FilterClause.java fe981a1 
>   core/src/test/java/org/apache/metamodel/QueryPostprocessDataContextTest.java 3fdb711 
>   core/src/test/java/org/apache/metamodel/query/FilterItemTest.java 2fedfb9 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java 06353f1 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java e4f1054 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDeleteBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDropTableBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchInsertBuilder.java PRE-CREATION 
>   elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUpdateCallback.java PRE-CREATION 
>   elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java 449490b 
> 
> Diff: https://reviews.apache.org/r/31066/diff/
> 
> 
> Testing
> -------
> 
> This is my initial/partial fix for METAMODEL-79. I want to share it because 1) there's more to come but I want to confirm that I'm on the right way and 2) I have questions for experts on E.S. :-)
> 
> This patch adds support for the MetaModel ElasticSearch to do INSERT INTO, CREATE TABLE and DROP TABLE statements.
> 
> It does not (yet) have support for UPDATE or DELETE FROM statements. I wanted to validate my initial work first.
> 
> And I have a few questions regarding types and mappings.
> 
>  * Please check the ElasticSearchCreateTableBuilder.getType(Column) method. Here I've attempted to convert ColumnTypes to ElasticSearch types. Are these correct? I am not sure about generalizations such as the NUMERIC -> "double" mapping etc.
>  * As a last resort I have used the type "object". But when I tried it out I ran into the problem that "object" is not polymorphic like in Java, it is actually the opposite of a "value type". So that means you cannot define an "object" field and then insert a single value into it. This makes it a bad fit for a "fallback" type. Is there a better way? Should we then simply NOT define the field in the mapping maybe?
> 
> 
> Thanks,
> 
> Kasper Sørensen
> 
>

Re: Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

Posted by Kasper Sørensen <i....@gmail.com>.

(Updated marts 9, 2015, 7:51 p.m.)

Review request for MetaModel.

Changes
-------

Fixes the issues raised by Dennis Du Krøger (thanks for the review btw)

Bugs: METAMODEL-79
https://issues.apache.org/jira/browse/METAMODEL-79

Repository: metamodel

Description
-------

Partial fix for METAMODEL-79 - see description

Diffs (updated)
-----

CHANGES.md 5c2e2eb
core/src/main/java/org/apache/metamodel/MetaModelHelper.java 4ee9798
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java 0eba921
core/src/main/java/org/apache/metamodel/query/FilterClause.java fe981a1
core/src/test/java/org/apache/metamodel/QueryPostprocessDataContextTest.java 3fdb711
core/src/test/java/org/apache/metamodel/query/FilterItemTest.java 2fedfb9
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java 06353f1
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataSet.java e4f1054
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDeleteBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDropTableBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchInsertBuilder.java PRE-CREATION
elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUpdateCallback.java PRE-CREATION
elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java 449490b

Diff: https://reviews.apache.org/r/31066/diff/

Testing
-------

This is my initial/partial fix for METAMODEL-79. I want to share it because 1) there's more to come but I want to confirm that I'm on the right way and 2) I have questions for experts on E.S. :-)

This patch adds support for the MetaModel ElasticSearch to do INSERT INTO, CREATE TABLE and DROP TABLE statements.

It does not (yet) have support for UPDATE or DELETE FROM statements. I wanted to validate my initial work first.

And I have a few questions regarding types and mappings.

Thanks,

Kasper Sørensen

Re: Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

Posted by Kasper Sørensen <i....@gmail.com>.

(Updated marts 8, 2015, 3:12 p.m.)

Review request for MetaModel.

Changes
-------

Updated the patch to be complete. Now has support for DELETE as well (under most conditions).

Some additional notes:

I also updated some stuff in the core module. This was to make it easier for the subclass to implement a query method when a few simple WHERE clauses are there. I believe this can later also be used to simplify e.g. the MongoDB and CouchDB modules (and maybe more). This was needed first for the DELETE part, but ...

FURTHERMORE I made an improvement to the query capability of the ElasticSearch module so that simple where clauses are evaluated for queries.

Bugs: METAMODEL-79
https://issues.apache.org/jira/browse/METAMODEL-79

Repository: metamodel

Description
-------

Partial fix for METAMODEL-79 - see description

Diffs (updated)
-----

Diff: https://reviews.apache.org/r/31066/diff/

Testing
-------

This is my initial/partial fix for METAMODEL-79. I want to share it because 1) there's more to come but I want to confirm that I'm on the right way and 2) I have questions for experts on E.S. :-)

This patch adds support for the MetaModel ElasticSearch to do INSERT INTO, CREATE TABLE and DROP TABLE statements.

It does not (yet) have support for UPDATE or DELETE FROM statements. I wanted to validate my initial work first.

And I have a few questions regarding types and mappings.

Thanks,

Kasper Sørensen

Re: Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

Posted by Alberto Rodriguez <ar...@gmail.com>.

Hi Kasper,

I'd like to have a look but quite busy at work :(

Kind Regards,

2015-02-17 9:19 GMT+01:00 Kasper Sørensen <i....@gmail.com>:

> Any comments or remarks on this patch?
>
> 2015-02-15 22:22 GMT+01: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/31066/
> > -----------------------------------------------------------
> >
> > Review request for MetaModel.
> >
> >
> > Bugs: METAMODEL-79
> >     https://issues.apache.org/jira/browse/METAMODEL-79
> >
> >
> > Repository: metamodel
> >
> >
> > Description
> > -------
> >
> > Partial fix for METAMODEL-79 - see description
> >
> >
> > Diffs
> > -----
> >
> >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java
> > PRE-CREATION
> >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> > 06353f1
> >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDropTableBuilder.java
> > PRE-CREATION
> >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchInsertIntoBuilder.java
> > PRE-CREATION
> >
> >
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUpdateCallback.java
> > PRE-CREATION
> >
> >
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> > 449490b
> >
> > Diff: https://reviews.apache.org/r/31066/diff/
> >
> >
> > Testing
> > -------
> >
> > This is my initial/partial fix for METAMODEL-79. I want to share it
> > because 1) there's more to come but I want to confirm that I'm on the
> right
> > way and 2) I have questions for experts on E.S. :-)
> >
> > This patch adds support for the MetaModel ElasticSearch to do INSERT
> INTO,
> > CREATE TABLE and DROP TABLE statements.
> >
> > It does not (yet) have support for UPDATE or DELETE FROM statements. I
> > wanted to validate my initial work first.
> >
> > And I have a few questions regarding types and mappings.
> >
> >  * Please check the ElasticSearchCreateTableBuilder.getType(Column)
> > method. Here I've attempted to convert ColumnTypes to ElasticSearch
> types.
> > Are these correct? I am not sure about generalizations such as the
> NUMERIC
> > -> "double" mapping etc.
> >  * As a last resort I have used the type "object". But when I tried it
> out
> > I ran into the problem that "object" is not polymorphic like in Java, it
> is
> > actually the opposite of a "value type". So that means you cannot define
> an
> > "object" field and then insert a single value into it. This makes it a
> bad
> > fit for a "fallback" type. Is there a better way? Should we then simply
> NOT
> > define the field in the mapping maybe?
> >
> >
> > Thanks,
> >
> > Kasper Sørensen
> >
> >
>

Re: Review Request 31066: METAMODEL-79: Support ElasticSearch operations: insert, create table and drop table

Posted by Kasper Sørensen <i....@gmail.com>.

Any comments or remarks on this patch?

2015-02-15 22:22 GMT+01:00 Kasper Sørensen <i....@gmail.com>:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31066/
> -----------------------------------------------------------
>
> Review request for MetaModel.
>
>
> Bugs: METAMODEL-79
>     https://issues.apache.org/jira/browse/METAMODEL-79
>
>
> Repository: metamodel
>
>
> Description
> -------
>
> Partial fix for METAMODEL-79 - see description
>
>
> Diffs
> -----
>
>
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchCreateTableBuilder.java
> PRE-CREATION
>
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContext.java
> 06353f1
>
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchDropTableBuilder.java
> PRE-CREATION
>
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchInsertIntoBuilder.java
> PRE-CREATION
>
> elasticsearch/src/main/java/org/apache/metamodel/elasticsearch/ElasticSearchUpdateCallback.java
> PRE-CREATION
>
> elasticsearch/src/test/java/org/apache/metamodel/elasticsearch/ElasticSearchDataContextTest.java
> 449490b
>
> Diff: https://reviews.apache.org/r/31066/diff/
>
>
> Testing
> -------
>
> This is my initial/partial fix for METAMODEL-79. I want to share it
> because 1) there's more to come but I want to confirm that I'm on the right
> way and 2) I have questions for experts on E.S. :-)
>
> This patch adds support for the MetaModel ElasticSearch to do INSERT INTO,
> CREATE TABLE and DROP TABLE statements.
>
> It does not (yet) have support for UPDATE or DELETE FROM statements. I
> wanted to validate my initial work first.
>
> And I have a few questions regarding types and mappings.
>
>  * Please check the ElasticSearchCreateTableBuilder.getType(Column)
> method. Here I've attempted to convert ColumnTypes to ElasticSearch types.
> Are these correct? I am not sure about generalizations such as the NUMERIC
> -> "double" mapping etc.
>  * As a last resort I have used the type "object". But when I tried it out
> I ran into the problem that "object" is not polymorphic like in Java, it is
> actually the opposite of a "value type". So that means you cannot define an
> "object" field and then insert a single value into it. This makes it a bad
> fit for a "fallback" type. Is there a better way? Should we then simply NOT
> define the field in the mapping maybe?
>
>
> Thanks,
>
> Kasper Sørensen
>
>