geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From YehEmily <...@git.apache.org>
Subject [GitHub] geode pull request #591: GEODE-3073: Renamed OrderByComparatorUnmapped to Or...
Date Fri, 23 Jun 2017 22:19:56 GMT
Github user YehEmily commented on a diff in the pull request:

    https://github.com/apache/geode/pull/591#discussion_r123854670
  
    --- Diff: geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
---
    @@ -925,79 +917,89 @@ private SelectResults applyProjectionOnCollection(SelectResults
resultSet,
     
       private SelectResults prepareEmptyResultSet(ExecutionContext context, boolean ignoreOrderBy)
           throws TypeMismatchException, AmbiguousNameException {
    -    // if no projection attributes or '*'as projection attribute
    -    // & more than one/RunTimeIterator then create a StrcutSet.
    -    // If attribute is null or '*' & only one RuntimeIterator then create a
    -    // ResultSet.
    -    // If single attribute is present without alias name , then create
    -    // ResultSet
    -    // Else if more than on attribute or single attribute with alias is
    -    // present then return a StrcutSet
    -    // create StructSet which will contain root objects of all iterators in
    -    // from clause
    +    // If no projection attributes or '*' as projection attribute & more than one/RunTimeIterator
    +    // then create a StructSet.
    +    // If attribute is null or '*' & only one RuntimeIterator then create a ResultSet.
    +    // If single attribute is present without alias name, then create ResultSet.
    +    // Else if more than on attribute or single attribute with alias is present then
return a
    +    // StructSet.
    +    // Create StructSet which will contain root objects of all iterators in from clause.
     
         ObjectType elementType = this.cachedElementTypeForOrderBy != null
             ? this.cachedElementTypeForOrderBy : prepareResultType(context);
         SelectResults results;
    -    if (this.distinct || !this.count) {
    -      if (this.orderByAttrs != null) {
    -        boolean nullValuesAtStart = !((CompiledSortCriterion) orderByAttrs.get(0)).getCriterion();
    -        if (elementType.isStructType()) {
    -          if (ignoreOrderBy) {
    -            results = this.distinct ? new LinkedStructSet((StructTypeImpl) elementType)
    -                : new SortedResultsBag(elementType, nullValuesAtStart);
     
    -          } else {
    -            OrderByComparator comparator = this.hasUnmappedOrderByCols
    -                ? new OrderByComparatorUnmapped(this.orderByAttrs, (StructTypeImpl) elementType,
    -                    context)
    -                : new OrderByComparator(this.orderByAttrs, (StructTypeImpl) elementType,
context);
    -            results = this.distinct ? new SortedStructSet(comparator, (StructTypeImpl)
elementType)
    -                : new SortedStructBag(comparator, (StructType) elementType, nullValuesAtStart);
    +    if (!this.distinct && this.count) {
    +      // Shobhit: If it's a 'COUNT' query and no End processing required Like for 'DISTINCT'
    +      // we can directly keep count in ResultSet and ResultBag is good enough for that.
    +      countStartQueryResult = 0;
    +      return new ResultsBag(new ObjectTypeImpl(Integer.class), 1, context.getCachePerfStats());
    +    }
     
    -          }
    -        } else {
    -          if (ignoreOrderBy) {
    -            results =
    -                this.distinct ? new LinkedResultSet() : new SortedResultsBag(nullValuesAtStart);
    +    // Potential edge-case: Could this be non-null but empty?
    +    boolean nullValuesAtStart = orderByAttrs != null && !orderByAttrs.get(0).getCriterion();
    +    OrderByComparator comparator;
    +    switch (convertToStringCase(elementType, ignoreOrderBy)) {
    +      case "UNORDERED DISTINCT STRUCT":
    +        return new StructSet((StructType) elementType);
    +      case "UNORDERED DISTINCT RESULTS":
    +        return new ResultsSet(elementType);
    +      case "UNORDERED INDISTINCT STRUCT":
    +        return new StructBag((StructType) elementType, context.getCachePerfStats());
    +      case "UNORDERED INDISTINCT RESULTS":
    +        return new ResultsBag(elementType, context.getCachePerfStats());
    +
    +      case "ORDERED DISTINCT STRUCT IGNORED":
    +        return new LinkedStructSet((StructTypeImpl) elementType);
    +      case "ORDERED INDISTINCT STRUCT IGNORED":
    +        return new SortedResultsBag(elementType, nullValuesAtStart);
    +      case "ORDERED DISTINCT STRUCT UNIGNORED":
    --- End diff --
    
    Good suggestion - thanks! @PurelyApplied and I worked together to get rid of the Strings,
and we're now using enums. You should be able to see these changes starting at around line
920 of `CompiledSelect` of the latest commit. We're using a naming convention that I think
follows your suggestion (instead of `UNORDERED_DISTINCT_STRUCT_IGNORED`, for example, we just
use `UNORDERED_DISTINCT_STRUCT`).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message