metamodel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kasper Sørensen <i.am.kasper.soren...@gmail.com>
Subject Re: Elastic Search module - Progress update
Date Sun, 21 Sep 2014 20:55:06 GMT
Here's my suggested changes to make it single-index based. I also took the
liberty to clean up the code style a little to be in line with all the
other MM code. Hope that's OK.

https://reviews.apache.org/r/25883/

2014-09-21 22:28 GMT+02:00 Kasper Sørensen <i.am.kasper.sorensen@gmail.com>:

> Yeah, let's make it single-schema oriented as a start. We might later add
> multi-schema support to it, if we can also get around that in the
> superclass.
>
> The JdbcDataContext does not inherit from QueryPostprocessDataContext at
> all - it's a completely separate implementation that has all the schema
> model handling and query execution based on JDBC and encoding the queries
> into SQL. That's of course by far more effecient, but also a lot more work
> than just using the QueryPostprocessDataContext's query handling mechanism.
>
> 2014-09-21 19:55 GMT+02:00 Alberto Rodriguez <ardlema@gmail.com>:
>
>> Yes, we might force the user to pass in the index name on the DataContext
>> creation. This way, if the user wants to point to another index he should
>> create another DataContext which is not so bad IMHO.
>>
>> I am out of home this weekend and I can't have a look at MM repo...how do
>> you handle this for JDBC?
>>
>> Kind Regards,
>>
>> Alberto
>> El 21/09/2014 15:18, "Kasper Sørensen" <i.am.kasper.sorensen@gmail.com>
>> escribió:
>>
>> > Hmm I imagine that we would not be happy with that solution in the long
>> run
>> > and then would have to change it in the future anyway. And that kind of
>> > change is a bit painful because it's only a runtime backwards
>> > incompatibility so people will find out too late.
>> >
>> > We could also consider to make the implementation index specific,  ie.
>> only
>> > for a single index. That would make a lot of things easy to model too.
>> > Den 21/09/2014 12.29 skrev "Alberto Rodriguez" <ardlema@gmail.com>:
>> >
>> > > Hi Kasper,
>> > >
>> > > That's great news!! Thank you for merging the code!
>> > >
>> > > Regarding the issue you found out, as a simple workaround, what if we
>> > > prepend the index name to the "table" name,something like:
>> > > indexname.indextable
>> > >
>> > > Kind regards,
>> > >
>> > > Alberto
>> > > Hi Alberto,
>> > >
>> > > As you've probably seen, the code has now been merged with the main
>> > > MetaModel repo - congratulations and thank you a lot!
>> > >
>> > > After merging it I took it for another spin to really dive deep into
>> some
>> > > of the design choices made. I had an observation (and just committed a
>> > > javadoc TODO comment about it) - the approach of mapping indexes and
>> > > document types is potentially destructable in the sense that if you
>> have
>> > > multiple indexes with the same document types, those table names are
>> > going
>> > > to be in conflict. Everything is right now put under the same schema,
>> > while
>> > > I think actually the ideal approach would be to produce multiple
>> schemas,
>> > > with their document types represented as tables. That would reflect
>> > better
>> > > on the hierarchy that ES has.
>> > >
>> > > Obviously that's harder to do ... And unfortunately right now the
>> > > QueryPostprocessDataContext class dictates that you have to produce a
>> > > single "main" schema. Which is kinda stupid and circumstantial. So I
>> > > propose we see if we can fix that, and then make the ES module
>> > multi-schema
>> > > oriented.
>> > >
>> > > 2014-09-17 23:51 GMT+02:00 Alberto Rodriguez <ardlema@gmail.com>:
>> > >
>> > > > Hi Kasper,
>> > > >
>> > > > all your suggestions/changes have been done.
>> > > >
>> > > > I am also happy to be involved in such an interesting project.
>> > > >
>> > > > Kind regards,
>> > > >
>> > > > Alberto
>> > > >
>> > > > 2014-09-17 21:21 GMT+02:00 Kasper Sørensen <
>> > > i.am.kasper.sorensen@gmail.com
>> > > > >:
>> > > >
>> > > > > Looks like good changes :-) Happy to co-develop some of this.
I
>> made
>> > a
>> > > > few
>> > > > > further remarks, but so far looking great!
>> > > > >
>> > > > > 2014-09-17 10:56 GMT+02:00 Alberto Rodriguez <
>> arodriguez@stratio.com
>> > >:
>> > > > >
>> > > > > > Hi Kasper,
>> > > > > >
>> > > > > > please see my notes on your PR comments, I've also pushed
new
>> > changes
>> > > > to
>> > > > > > the repo.
>> > > > > >
>> > > > > > 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-09-17 9:45 GMT+02:00 Kasper Sørensen <
>> > > > > i.am.kasper.sorensen@gmail.com
>> > > > > > >:
>> > > > > >
>> > > > > > > Hi Alberto,
>> > > > > > >
>> > > > > > > I added a bunch of remarks to the PR in github. None
of them
>> > > > absolutely
>> > > > > > > critical to do before we merge it, but nice if you
anyway
>> want to
>> > > > spend
>> > > > > > > more time on it.
>> > > > > > >
>> > > > > > > Best regards
>> > > > > > > Kasper
>> > > > > > > Den 16/09/2014 23.01 skrev "Henry Saputra" <
>> > > henry.saputra@gmail.com
>> > > > >:
>> > > > > > >
>> > > > > > > > Thanks Alberto, this should help review the patch
>> > > > > > > >
>> > > > > > > > - Henry
>> > > > > > > >
>> > > > > > > > On Tue, Sep 16, 2014 at 1:44 PM, Alberto Rodriguez
<
>> > > > > ardlema@gmail.com>
>> > > > > > > > wrote:
>> > > > > > > > > Hi Henry,
>> > > > > > > > >
>> > > > > > > > > I have just submitted a PR against the MM
github mirror.
>> > > > > > > > >
>> > > > > > > > > Kind regards,
>> > > > > > > > >
>> > > > > > > > > Alberto Rodríguez
>> > > > > > > > >
>> > > > > > > > > 2014-09-16 19:01 GMT+02:00 Henry Saputra
<
>> > > > henry.saputra@gmail.com
>> > > > > >:
>> > > > > > > > >
>> > > > > > > > >> Alberto,
>> > > > > > > > >>
>> > > > > > > > >> Thanks for the ES module patch.
>> > > > > > > > >>
>> > > > > > > > >> Could you submit PR against Apache MetaModel
github
>> mirror?
>> > > > > > > > >> We may need to manually merge your fixes
to ASF Git repo
>> > later
>> > > > but
>> > > > > > > > >> Github PR will help with reviews.
>> > > > > > > > >>
>> > > > > > > > >> - Henry
>> > > > > > > > >>
>> > > > > > > > >> On Tue, Sep 16, 2014 at 2:45 AM, Alberto
Rodriguez
>> > > > > > > > >> <arodriguez@stratio.com> wrote:
>> > > > > > > > >> > Hi Kasper,
>> > > > > > > > >> >
>> > > > > > > > >> > thank you very much for your feedback!
>> > > > > > > > >> >
>> > > > > > > > >> > I have just modified the getMainSchemaName
method to
>> get
>> > the
>> > > > > > cluster
>> > > > > > > > name
>> > > > > > > > >> > from ES. Regarding the executeQuery
method I commented
>> it
>> > > out
>> > > > > > > because
>> > > > > > > > >> some
>> > > > > > > > >> > tests were failing, anyway I will
try to make this
>> method
>> > > work
>> > > > > and
>> > > > > > > get
>> > > > > > > > >> all
>> > > > > > > > >> > the benefits from the awesome elasticsearch
java API.
>> > > > > > > > >> >
>> > > > > > > > >> > My apache username is: albertostratio
>> > > > > > > > >> >
>> > > > > > > > >> > 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-09-16 10:45 GMT+02:00 Kasper
Sørensen <
>> > > > > > > > >> i.am.kasper.sorensen@gmail.com>:
>> > > > > > > > >> >
>> > > > > > > > >> >> Hi Alberto,
>> > > > > > > > >> >>
>> > > > > > > > >> >> Once again, thank you very much
for this great work. I
>> > > think
>> > > > > this
>> > > > > > > > >> module is
>> > > > > > > > >> >> looking 99% ready to be introduced
into the main
>> > MetaModel
>> > > > > > > codebase.
>> > > > > > > > >> >>
>> > > > > > > > >> >> Here are my very minor remarks:
>> > > > > > > > >> >>
>> > > > > > > > >> >>
>> > > > > > > > >> >>    - The main schema name "ElasticSearchSchema"
is
>> > > hardcoded
>> > > > > and
>> > > > > > > not
>> > > > > > > > >> >>    necesarily what users would
want. I was thinking
>> if we
>> > > > > should
>> > > > > > > > either:
>> > > > > > > > >> >>    - Make it parameterizable
in the constructor
>> (default
>> > > > value
>> > > > > > > would
>> > > > > > > > IMO
>> > > > > > > > >> >>       better be just "ElasticSearch"
which is in line
>> > with
>> > > > > > > "MongoDB",
>> > > > > > > > >> >> "CouchDB"
>> > > > > > > > >> >>       etc.)
>> > > > > > > > >> >>       - Use the ES cluster name.
This would give a
>> more
>> > > > > "native"
>> > > > > > > > >> feeling -
>> > > > > > > > >> >>       then we've mapped it to
something that is
>> > > recognizable
>> > > > > from
>> > > > > > > > the ES
>> > > > > > > > >> >>       connection.
>> > > > > > > > >> >>       - Or a combination of
the above - make a
>> > constructor
>> > > > name
>> > > > > > > > >> available,
>> > > > > > > > >> >>       but if null we use the
cluster name.
>> > > > > > > > >> >>    - You've now commented out
your whole
>> > > executeQuery(Query)
>> > > > > > > method.
>> > > > > > > > >> Might
>> > > > > > > > >> >>    be on purpose, but I guess
you had some nice
>> > > optimizations
>> > > > > for
>> > > > > > > > >> special
>> > > > > > > > >> >>    cases in there. I hope we
can restore that.
>> > > > > > > > >> >>
>> > > > > > > > >> >> What is your apache username?
When we have that, I
>> > propose
>> > > > that
>> > > > > > we
>> > > > > > > > take
>> > > > > > > > >> >> your contribution for a VOTE
and then hopefully
>> integrate
>> > > it
>> > > > > into
>> > > > > > > the
>> > > > > > > > >> main
>> > > > > > > > >> >> codebase of MM. The corrections
above can be made
>> either
>> > > > before
>> > > > > > or
>> > > > > > > > after
>> > > > > > > > >> >> IMO.
>> > > > > > > > >> >>
>> > > > > > > > >> >> Best regards,
>> > > > > > > > >> >> Kasper
>> > > > > > > > >> >>
>> > > > > > > > >> >>
>> > > > > > > > >> >> 2014-09-16 9:28 GMT+02:00 Alberto
Rodriguez <
>> > > > > > > arodriguez@stratio.com
>> > > > > > > > >:
>> > > > > > > > >> >>
>> > > > > > > > >> >> > Hi all,
>> > > > > > > > >> >> >
>> > > > > > > > >> >> > I have been working on
the ES module. As Kasper
>> > suggested
>> > > I
>> > > > > > have
>> > > > > > > > >> >> overriden
>> > > > > > > > >> >> > the materializeMainSchemaTable
to get a first
>> > functional
>> > > > > > > version. I
>> > > > > > > > >> have
>> > > > > > > > >> >> > added quite a few tests
to check that the module is
>> > > working
>> > > > > > fine
>> > > > > > > > for
>> > > > > > > > >> >> > *SELECT* operations. I
have also implemented the
>> > > > > > > executeCountQuery
>> > > > > > > > >> >> method.
>> > > > > > > > >> >> >
>> > > > > > > > >> >> > I forked the project from
the *master branch, *you
>> can
>> > > > check
>> > > > > > out
>> > > > > > > > the
>> > > > > > > > >> >> > changes here: incubator-metamodel
fork with ES
>> module
>> > > > > > > > >> >> > <
>> https://github.com/albertostratio/incubator-metamodel
>> > >
>> > > > > > > > >> >> >
>> > > > > > > > >> >> > I have also been informed
by the ASF that my ICLA
>> has
>> > > been
>> > > > > > filed
>> > > > > > > in
>> > > > > > > > >> their
>> > > > > > > > >> >> > records.
>> > > > > > > > >> >> >
>> > > > > > > > >> >> > 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>*
>> > > > > > > > >> >> >
>> > > > > > > > >> >>
>> > > > > > > > >>
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message