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/03/08 16:13:00 UTC

Re: 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/
-----------------------------------------------------------

(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)
-----

  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 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>.
-----------------------------------------------------------
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.


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.

 * 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