OK, so I'll push that change to the master soon. I guess the next step is to start making JIRAs for stuff we want to improve. That could be to support insert/update/delete (ie. implement UpdateableDataContext interface) or it could be other improvements (for instance I noticed that the DataSet doesn't look to be streaming, so large datasets would cause out of memory exceptions (I think, didn't look very closely into it yet)).
You are more than welcome to start putting in those things into JIRA. Then we have a nice process going where issues can be dealt with individually and we can track if there's more on our wish-list. Best regards, Kasper 2014-09-24 9:27 GMT+02:00 Alberto Rodriguez <[email protected]>: > Hi Kasper, > > I've been having a look at the changes and they all look fine with me. I am > not really used to this kind of revisions...what should be the next steps? > > By the way, I did not have time to work on the insert, delete and update > operations, do you think we should implement them? > > Kind regards, > > Alberto > > 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-21 22:55 GMT+02:00 Kasper Sørensen <[email protected] > >: > > > 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 < > [email protected] > > >: > > > > > 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 <[email protected]>: > > > > > >> 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" < > [email protected]> > > >> 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" <[email protected]>: > > >> > > > >> > > 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 <[email protected]>: > > >> > > > > >> > > > 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 < > > >> > > [email protected] > > >> > > > >: > > >> > > > > > >> > > > > 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 < > > >> [email protected] > > >> > >: > > >> > > > > > > >> > > > > > 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 < > > >> > > > > [email protected] > > >> > > > > > >: > > >> > > > > > > > >> > > > > > > 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" < > > >> > > [email protected] > > >> > > > >: > > >> > > > > > > > > >> > > > > > > > Thanks Alberto, this should help review the patch > > >> > > > > > > > > > >> > > > > > > > - Henry > > >> > > > > > > > > > >> > > > > > > > On Tue, Sep 16, 2014 at 1:44 PM, Alberto Rodriguez < > > >> > > > > [email protected]> > > >> > > > > > > > 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 < > > >> > > > [email protected] > > >> > > > > >: > > >> > > > > > > > > > > >> > > > > > > > >> 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 > > >> > > > > > > > >> <[email protected]> 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 < > > >> > > > > > > > >> [email protected]>: > > >> > > > > > > > >> > > > >> > > > > > > > >> >> 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 < > > >> > > > > > > [email protected] > > >> > > > > > > > >: > > >> > > > > > > > >> >> > > >> > > > > > > > >> >> > 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>* > > >> > > > > > > > >> >> > > > >> > > > > > > > >> >> > > >> > > > > > > > >> > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > > > > >
