> On Feb. 21, 2017, 9:02 p.m., Vadim Spector wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java, > > line 88 > > <https://reviews.apache.org/r/56481/diff/2/?file=1640442#file1640442line88> > > > > Separate lists of query parts and nested QueryParamBuilder's do not > > preserver the order of the query building. And the order may be important > > for optimization if the query creator code is clever enough. > > > > I'd recommend QueryParamBuilder to subclass QueryPart and be a > > container for a single List<QueryPart> to include both simple query parts > > and nested queries, so they're kept in the order of their addition. > > Alexander Kolbasov wrote: > Why would you think that the original order of the code was any better > than another order? Currently parts are just added in the order that follows > the code. I'd rather keep it simple unless you think there is a good reason > to keep the order - I do not see why ordering is important here.
Example: my code keeps pointers to the query builders parent p1 and its child p2. The way I build query is as follows (in pseudocode): p1 = ...; p1.add(Q1); p2 = p1.addChild(); p2.add(Q2); p1.add(Q3); Sp, basically, I want my query to be "Q1 AND Q2 AND Q3", where Q2 can be some complex subquery (thats why I used the nested builder) - in this specific order. The existing code will produce the query "Q1 AND Q3 AND Q2", because it groups Q1 and Q3 together as belonging to p1. So, it disregards my design of the query. If I wanted this order, I would've easily said: p1 = ...; p1.add(Q1); p1.add(Q3); p2 = p1.addChild(); p2.add(Q2); It's generally known that the order of elements in the query can dramatically affect their performance. E.g. I may know that Q2 tends to produce zero hits really often, so my SELECT will skip the rest of the query (which can be expensive) altogether. In fact, I may choose to put one or more children subqueries upfront. There is no reason why two different code snippets above, each clearly expressing the author's choice of the query, should result in the identical queries, if it's so easy to implement it to honor the author's choice. - Vadim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56481/#review166225 ----------------------------------------------------------- On Feb. 20, 2017, 5:46 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56481/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2017, 5:46 a.m.) > > > Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Sergio Pena, > Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1625 > https://issues.apache.org/jira/browse/SENTRY-1625 > > > Repository: sentry > > > Description > ------- > > SENTRY-1625: PrivilegeOperatePersistence can use QueryParamBuilder > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java > b1180bff1ce8850712e842e59e0e7217273ac59c > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java > 55b61ac07c6865402ffe4d0ff1690afb435deb95 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java > PRE-CREATION > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 7b926a5e546091881d4f7b4f30df4ae82dcd35b0 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 17a4a937221891a72ee44db92976cfa5cab40bc4 > > Diff: https://reviews.apache.org/r/56481/diff/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
