----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50311/#review144628 -----------------------------------------------------------
repository/src/main/scala/org/apache/atlas/query/Expressions.scala (line 332) <https://reviews.apache.org/r/50311/#comment210648> extra whitespace. There are multiple occurrences of this. repository/src/main/scala/org/apache/atlas/query/Expressions.scala (line 806) <https://reviews.apache.org/r/50311/#comment210647> Isn't the data type for a count expression always long? repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala (line 395) <https://reviews.apache.org/r/50311/#comment210658> Is this really needed? Can we add a condition within genFullQuery() instead to determine whether or not "toList()" is needed? We should treat count queries the same as any other gremlin report query with one output column. Like any other query, count queries return a list of rows. It just happens that there is only one row. If we add grouping support, there will be multiple rows. repository/src/main/scala/org/apache/atlas/query/QueryParser.scala (line 169) <https://reviews.apache.org/r/50311/#comment210652> Should "SrcQuery" be changed to querySrc? repository/src/main/scala/org/apache/atlas/query/QueryParser.scala (line 173) <https://reviews.apache.org/r/50311/#comment210649> It looks like count is not allowed for queries that use a loop expression. Is that intentional? repository/src/main/scala/org/apache/atlas/query/Resolver.scala (line 103) <https://reviews.apache.org/r/50311/#comment210650> It looks like we're not resolving the type of the count expression children. I'm wondering if that will cause things to break for more complicated expression. Doesn't that effectively disable the type resolution for the whole query? I think that the child is the expression tree for the things that feed into the count expression. Atlas relies on that type information for query validation, and in some cases it affects how the query gets generated. repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java (line 408) <https://reviews.apache.org/r/50311/#comment210651> If this is not supposed to work, should it be removed? Is there a JIRA for this? - Jeff Hagelberg On July 27, 2016, 10:33 a.m., Neeru Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50311/ > ----------------------------------------------------------- > > (Updated July 27, 2016, 10:33 a.m.) > > > Review request for atlas, David Kantor and Jeff Hagelberg. > > > Bugs: atlas-737 > https://issues.apache.org/jira/browse/atlas-737 > > > Repository: atlas > > > Description > ------- > > Jira 737 Add count queries in dsl querries > > > Diffs > ----- > > repository/src/main/scala/org/apache/atlas/query/Expressions.scala > 297aa2b8f917d5ae25da6a69901af60542082642 > repository/src/main/scala/org/apache/atlas/query/GremlinEvaluator.scala > 7de03c32edf16c1676db94179266e02928e01f35 > repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala > d336f1ec6d7637ed6d05cd781c66ed5407632e88 > repository/src/main/scala/org/apache/atlas/query/QueryParser.scala > 4d2429ebe9e6734a111201db164ec6ce7ab50c32 > repository/src/main/scala/org/apache/atlas/query/Resolver.scala > cff92afe98a6215499a70a9ef00a937804abe73e > > repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java > df3fe8713819d3837dfde2bbf14fb7cacf0bdea4 > repository/src/test/scala/org/apache/atlas/query/ParserTest.scala > 8f277fc160f12f839dd6f2cba4657cd41c68b1c6 > typesystem/.gitignore c7029f81f2e8162e4926aaa8a6cfc153615a0a96 > > Diff: https://reviews.apache.org/r/50311/diff/ > > > Testing > ------- > > Execute all the existing tests successfully. added new tests with count > > > Thanks, > > Neeru Gupta > >
