> On July 20, 2015, 9:39 a.m., Rajat Khandelwal wrote: > > lens-driver-es/src/main/java/org/apache/lens/driver/es/ESDriver.java, line > > 307 > > <https://reviews.apache.org/r/36434/diff/5/?file=1016016#file1016016line307> > > > > Can we rename it to something more revealing? `QueryExecuteCallable` or > > something?
I think I ll name it ESQueryExecuteCallable ? > On July 20, 2015, 9:39 a.m., Rajat Khandelwal wrote: > > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/ESClient.java, > > lines 47-57 > > <https://reviews.apache.org/r/36434/diff/5/?file=1016019#file1016019line47> > > > > Let's make it an interface instead of an abstract class which does > > nothing but set an instance level variable in constructor. The > > implementations should be free to use whatever is passed in constructor the > > way it wants. Some implementations might not need the parameter. The > > implementations should not be given the instance level field without them > > being aware of its availability. > > > > Put another way, `super(query)` and calling `this.esQuery = query` both > > take same number of lines and the latter is cleaner for the code readers. But here any implementation of an execution mode has to be compulsarily be aware of the query that its gonna run. So abstracting it out kinda naturally fits the context here right? > On July 20, 2015, 9:39 a.m., Rajat Khandelwal wrote: > > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/jest/JestClientImpl.java, > > lines 45-49 > > <https://reviews.apache.org/r/36434/diff/5/?file=1016021#file1016021line45> > > > > Can these be moved to `ESDriverConfig`? These are constants that are specific to the JestClient, ESDriverConfig has the only the driver level configs > On July 20, 2015, 9:39 a.m., Rajat Khandelwal wrote: > > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/jest/JestResultSetTransformer.java, > > lines 168-172 > > <https://reviews.apache.org/r/36434/diff/5/?file=1016022#file1016022line168> > > > > If these are the only usages of the inner static Transformer classes, > > then I don't see the need to have these as subclasses of one common class, > > much less as subclssses of a class that wraps them. But I need to follow some kind of strategy pattern to choose the transformation method. Tomorrow I might have to have private transient state in my transformers which kind of becomes difficult if I dont have an object oriented approach around it? - Amruth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36434/#review92234 ----------------------------------------------------------- On July 19, 2015, 10:29 a.m., Amruth Sampath wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36434/ > ----------------------------------------------------------- > > (Updated July 19, 2015, 10:29 a.m.) > > > Review request for lens, Amareshwari Sriramadasu, Jaideep dhok, and sharad > agarwal. > > > Bugs: LENS-252 > https://issues.apache.org/jira/browse/LENS-252 > > > Repository: lens > > > Description > ------- > > Elastic search driver for lens > ~~~~~~~~~~~~~~~~~~~~~~ > Elastic search accepts a nested json as a query and returns a json result. > The json result is nested for group by queries and simple for simple selects. > HQL -> ES json query > ~~~~~~~~~~~~~~~~~ > -> I have written a traversal (ASTTraverserForES)(specific for noSQL stores). > The traversal could be used for any purpose like query building/validation > etc. > -> The traversal will take in a query visitor (ASTVisitor.java) (for building > the query) and a criteria visitor (for building the where clause). I have > checked in concrete visitors for ElasticSearch that can build the elastic > search json query. > Elasticsearch client > ~~~~~~~~~~~~~~~ > -> There are multiple choices of elasticsearch client available. I've made > the client pluggable. > -> I've added one default HTTPClient implementation (Jest library - apache > 2). The choice of HTTP client over transport client was made because of the > version consistency requirement between the transport client and the ES > server. > -> Every client has to implement an execute method that takes in the query > and returns a LensResultSet. Hence the transformation of resultset must also > be done by the client implementation. > Elasticsearch Jest json response -> LensResultSet > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > The result set obtained could be a simple hit result (document response) or > facets response. > -> Facets response has a tree structure. Finding all the paths in the tree > will give us all the rows. Look at JestResultSetTransformer > (AggregateTransformer) > -> Hits response is straightforward to decode (TermTransformer) > Known issues/shortcommings > ~~~~~~~~~~~~~~~~~~~~~~~~ > -> Scrolling responses for aggregate queries (by design ES always returns the > complete bucket in a single json - there is no scroll facility) > -> Order by in aggregate queries. Fully functional order by queries can get > complex as the ordering by measure can happen only in the immediate parent > group by. 'Limit' is also blocked as it could be misleading to have limit > without order by. (Please note that order by and limit will still work in > queries without group bys) > -> *, count is not available as of now. > -> support for other UDFs. Right now common UDAFs like sum, min, max are > supported. We need a way to seamlessly translate a new UDF to elastic search > without code change > -> Query estimation > -> Session level config injection for properties like fetch size and group by > cardinality size? (Right now these configs are at driver level) > Have added a esdriver-default.xml for looking up default properties > > > Diffs > ----- > > lens-driver-es/pom.xml PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/ASTTraverserForES.java > PRE-CREATION > lens-driver-es/src/main/java/org/apache/lens/driver/es/ESDriver.java > PRE-CREATION > lens-driver-es/src/main/java/org/apache/lens/driver/es/ESDriverConfig.java > PRE-CREATION > lens-driver-es/src/main/java/org/apache/lens/driver/es/ESQuery.java > PRE-CREATION > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/ESClient.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/ESResultSet.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/jest/JestClientImpl.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/client/jest/JestResultSetTransformer.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/exceptions/ESClientException.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/exceptions/InvalidQueryException.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/ASTCriteriaVisitor.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/ASTVisitor.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/CriteriaVisitorFactory.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/ESVisitor.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/impl/ESAggregateVisitor.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/impl/ESCriteriaVisitor.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/impl/ESCriteriaVisitorFactory.java > PRE-CREATION > > lens-driver-es/src/main/java/org/apache/lens/driver/es/translator/impl/ESTermVisitor.java > PRE-CREATION > lens-driver-es/src/main/resources/esdriver-default.xml PRE-CREATION > lens-driver-es/src/test/java/org/apache/lens/driver/es/ESDriverTest.java > PRE-CREATION > lens-driver-es/src/test/java/org/apache/lens/driver/es/MockClientES.java > PRE-CREATION > > lens-driver-es/src/test/java/org/apache/lens/driver/es/QueryTranslationTest.java > PRE-CREATION > > lens-driver-es/src/test/java/org/apache/lens/driver/es/ResultSetTransformationTest.java > PRE-CREATION > > lens-driver-es/src/test/java/org/apache/lens/driver/es/ScrollingQueryTest.java > PRE-CREATION > lens-server/pom.xml b85292c > pom.xml 4b049f0 > > Diff: https://reviews.apache.org/r/36434/diff/ > > > Testing > ------- > > Added unit test cases for testing > - query translation > - result set translation > - Scrolling > > > Thanks, > > Amruth Sampath > >
