Return-Path: X-Original-To: apmail-metamodel-dev-archive@minotaur.apache.org Delivered-To: apmail-metamodel-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id BCB571138C for ; Sun, 21 Sep 2014 20:29:08 +0000 (UTC) Received: (qmail 5549 invoked by uid 500); 21 Sep 2014 20:29:08 -0000 Delivered-To: apmail-metamodel-dev-archive@metamodel.apache.org Received: (qmail 5514 invoked by uid 500); 21 Sep 2014 20:29:08 -0000 Mailing-List: contact dev-help@metamodel.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@metamodel.incubator.apache.org Delivered-To: mailing list dev@metamodel.incubator.apache.org Received: (qmail 5502 invoked by uid 99); 21 Sep 2014 20:29:08 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 21 Sep 2014 20:29:08 +0000 X-ASF-Spam-Status: No, hits=2.5 required=5.0 tests=FREEMAIL_REPLY,HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of i.am.kasper.sorensen@gmail.com designates 74.125.82.170 as permitted sender) Received: from [74.125.82.170] (HELO mail-we0-f170.google.com) (74.125.82.170) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 21 Sep 2014 20:28:42 +0000 Received: by mail-we0-f170.google.com with SMTP id x48so535420wes.1 for ; Sun, 21 Sep 2014 13:28:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=HOHhTM1o9QZghytxQc7bQ6mj5y61nn/5XgCDGw0nM/M=; b=ARD+ZqvX2Pz0hAvHmS6CwdtGMbnmefKx30Guz6uORJeRQ+MK4iy0ihShZMmY/Z/taN dOkV8wni62nuQOfhI81bzRLRqgUKRgtZpgbgapRhaJAioNd96y0PBFa4vmz/9uRDgjLw aRNIVIk0h/nFceFbTk/bVLrpNGYD+vK5RO6nq0tHxrX8sRw0APnLbC1SiT4mQ72jC021 qIaIhyAyUCkXDEIGPW4Adn1Fsql39wsXxLtayTyVTAjtZ1txeekzHQhIX8aPX2V+9GST g7YuaofQtBOmf8FOO2nVgs+i+hqwNM0QBhlCuqgNPcP0J7Y/zcQ6/dKSUSR8i6S/8PGL G51w== MIME-Version: 1.0 X-Received: by 10.180.75.210 with SMTP id e18mr11173506wiw.6.1411331319272; Sun, 21 Sep 2014 13:28:39 -0700 (PDT) Received: by 10.194.190.232 with HTTP; Sun, 21 Sep 2014 13:28:39 -0700 (PDT) In-Reply-To: References: Date: Sun, 21 Sep 2014 22:28:39 +0200 Message-ID: Subject: Re: Elastic Search module - Progress update From: =?UTF-8?Q?Kasper_S=C3=B8rensen?= To: "dev@metamodel.incubator.apache.org" Content-Type: multipart/alternative; boundary=f46d04389067f21a500503992da8 X-Virus-Checked: Checked by ClamAV on apache.org --f46d04389067f21a500503992da8 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 : > 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=C3=B8rensen" > escribi=C3=B3: > > > 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" : > > > > > 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 : > > > > > > > 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=C3=B8rensen < > > > 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=C3=ADguez > > > > > > > > > > > > > > > > > > > > > > > > Avenida de Europa, 26. =C3=81tica 5. 3=C2=AA Planta > > > > > > 28224 Pozuelo de Alarc=C3=B3n, Madrid > > > > > > Tel: +34 91 352 59 42 // *@stratiobd < > > https://twitter.com/StratioBD > > > >* > > > > > > > > > > > > 2014-09-17 9:45 GMT+02:00 Kasper S=C3=B8rensen < > > > > > 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 wan= t > 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=C3=ADguez > > > > > > > > > > > > > > > > > > 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 > > > > > > > > >> wrote: > > > > > > > > >> > Hi Kasper, > > > > > > > > >> > > > > > > > > > >> > thank you very much for your feedback! > > > > > > > > >> > > > > > > > > > >> > I have just modified the getMainSchemaName method to g= et > > 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=C3=ADguez > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > Avenida de Europa, 26. =C3=81tica 5. 3=C2=AA Planta > > > > > > > > >> > 28224 Pozuelo de Alarc=C3=B3n, Madrid > > > > > > > > >> > Tel: +34 91 352 59 42 // *@stratiobd < > > > > > > https://twitter.com/StratioBD > > > > > > > >* > > > > > > > > >> > > > > > > > > > >> > 2014-09-16 10:45 GMT+02:00 Kasper S=C3=B8rensen < > > > > > > > > >> 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 i= f > 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 mo= re > > > > > "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 modu= le > > > > > > > > >> >> > < > https://github.com/albertostratio/incubator-metamodel > > > > > > > > > > > >> >> > > > > > > > > > >> >> > I have also been informed by the ASF that my ICLA h= as > > > been > > > > > > filed > > > > > > > in > > > > > > > > >> their > > > > > > > > >> >> > records. > > > > > > > > >> >> > > > > > > > > > >> >> > Kind regards, > > > > > > > > >> >> > > > > > > > > > >> >> > > > > > > > > > >> >> > Alberto Rodr=C3=ADguez > > > > > > > > >> >> > > > > > > > > > >> >> > > > > > > > > > >> >> > > > > > > > > > >> >> > Avenida de Europa, 26. =C3=81tica 5. 3=C2=AA Planta > > > > > > > > >> >> > 28224 Pozuelo de Alarc=C3=B3n, Madrid > > > > > > > > >> >> > Tel: +34 91 352 59 42 // *@stratiobd < > > > > > > > > https://twitter.com/StratioBD>* > > > > > > > > >> >> > > > > > > > > > >> >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --f46d04389067f21a500503992da8--