MONDRIAN

       Renaming instance variables and methods to reflect usage. Basically,
       a RolapStar has columns (all of the columns) but associated with
       a query there is only a sub-set of all columns.

[git-p4: depot-paths = "//open/mondrian/": change = 7772]

src/main/mondrian/rolap/FastBatchingCellReader.java

@@ -141,25 +141,27 @@ public String toString() {
                 return star.getFactTable().getTableName()+" "+key.toString();
             }
         };
-        BitKey bitkey = request.getBatchKey();
+        BitKey bitkey = request.getConstrainedColumnsBitKey();
         BatchKey key = new BatchKey(bitkey, request.getMeasure().getStar());
         Batch batch = (Batch) batches.get(key);
         if (batch == null) {
             batch = new Batch(request);
+            batches.put(key, batch);
 
             if (LOGGER.isDebugEnabled()) {
-                LOGGER.debug("FastBatchingCellReader: bitkey=" +
-                        request.getBatchKey());
-                RolapStar star = cube.getStar();
-                if (star != null) {
-                    RolapStar.Column[] columns =
-                            star.lookupColumns((BitKey) request.getBatchKey());
-                    for (int i = 0; i < columns.length; i++) {
-                        LOGGER.debug("  " +columns[i]);
-                    }
+                StringBuffer buf = new StringBuffer(100);
+                buf.append("FastBatchingCellReader: bitkey=");
+                buf.append(request.getConstrainedColumnsBitKey());
+                buf.append(Util.nl);
+
+                RolapStar.Column[] columns = request.getConstrainedColumns();
+                for (int i = 0; i < columns.length; i++) {
+                    buf.append("  ");
+                    buf.append(columns[i]);
+                    buf.append(Util.nl);
                 }
+                LOGGER.debug(buf.toString());
             }
-            batches.put(key, batch);
         }
         batch.add(request);
     }
@@ -220,13 +222,16 @@ void setDirty(boolean dirty) {
 
     private static final Logger BATCH_LOGGER = Logger.getLogger(Batch.class);
     class Batch {
+        // these are the CellRequest's constrained columns
         final RolapStar.Column[] columns;
+        // this is the CellRequest's constrained column BitKey
         final BitKey bitKey;
         final List measuresList = new ArrayList();
         final Set[] valueSets;
+
         public Batch(CellRequest request) {
-            columns = request.getColumns();
-            bitKey = request.getBatchKey();
+            columns = request.getConstrainedColumns();
+            bitKey = request.getConstrainedColumnsBitKey();
             valueSets = new HashSet[columns.length];
             for (int i = 0; i < valueSets.length; i++) {
                 valueSets[i] = new HashSet();
@@ -244,18 +249,25 @@ public void add(CellRequest request) {
                         (measure.getStar() ==
                         ((RolapStar.Measure) measuresList.get(0)).getStar()):
                         "Measure must belong to same star as other measures";
-/*
-System.out.println("Batch.add: size=" +measuresList.size());
-System.out.println(" measure.getName()=" +measure.getName());
-System.out.println(" measure.getTable().getTableName()=" +measure.getTable().getTableName());
-*/
                 measuresList.add(measure);
             }
         }
+
+        /** 
+         * This can only be called after the add method has been called. 
+         * 
+         * @return the RolapStar associated with the Batch's first Measure.
+         */
+        private RolapStar getStar() {
+            RolapStar.Measure measure = (RolapStar.Measure) measuresList.get(0);
+            RolapStar star = measure.getStar();
+            return star;
+        }
 
         void loadAggregation() {
             if (generateAggregateSql) {
-                if (FastBatchingCellReader.this.cube.isVirtual()) {
+                RolapCube cube = FastBatchingCellReader.this.cube;
+                if (cube == null || cube.isVirtual()) {
                     StringBuffer buf = new StringBuffer(64);
                     buf.append("AggGen: Sorry, can not create SQL for virtual Cube \"");
                     buf.append(FastBatchingCellReader.this.cube.getName());
@@ -301,28 +313,26 @@ void loadAggregation() {
 
                 constraintses[j] = constraints;
             }
-            // todo: optimize key sets; drop a constraint if more than x% of
+            // TODO: optimize key sets; drop a constraint if more than x% of
             // the members are requested; whether we should get just the cells
             // requested or expand to a n-cube
 
             // If the database cannot execute "count(distinct ...)", split the
             // distinct aggregations out.
-            RolapStar.Measure measure =
-                (RolapStar.Measure) measuresList.get(0);
-            if (!measure.getStar().getSqlQueryDialect().allowsCountDistinct()) {
+            if (!getStar().getSqlQueryDialect().allowsCountDistinct()) {
                 while (true) {
                     // Scan for a measure based upon a distinct aggregation.
                     RolapStar.Measure distinctMeasure =
                         getFirstDistinctMeasure(measuresList);
                     if (distinctMeasure == null) {
                         break;
                     }
-                    final String expr =
-                        distinctMeasure.getExpression().
+                    final String expr = distinctMeasure.getExpression().
                             getGenericExpression();
                     final List distinctMeasuresList = new ArrayList();
                     for (int i = 0; i < measuresList.size();) {
-                        measure = (RolapStar.Measure) measuresList.get(i);
+                        RolapStar.Measure measure =
+                                (RolapStar.Measure) measuresList.get(i);
                         if (measure.getAggregator().isDistinct() &&
                             measure.getExpression().getGenericExpression().
                                 equals(expr))
@@ -340,13 +350,13 @@ void loadAggregation() {
                             constraintses, pinnedSegments);
                 }
             }
+
             final int measureCount = measuresList.size();
-            if (measureCount > 0) {
-                RolapStar.Measure[] measures = (RolapStar.Measure[])
+            RolapStar.Measure[] measures = (RolapStar.Measure[])
                     measuresList.toArray(new RolapStar.Measure[measureCount]);
-                aggmgr.loadAggregation(measures, columns, bitKey,
+            aggmgr.loadAggregation(measures, columns, bitKey,
                     constraintses, pinnedSegments);
-            }
+
             if (BATCH_LOGGER.isDebugEnabled()) {
                 long t2 = System.currentTimeMillis();
                 BATCH_LOGGER.debug("Batch.loadAggregation (millis) " + (t2 - t1));

src/main/mondrian/rolap/RolapStar.java

@@ -442,7 +442,7 @@ Object getCell(CellRequest request) {
 
     private Object getCell(CellRequest request, Connection jdbcConnection) {
         Measure measure = request.getMeasure();
-        Column[] columns = request.getColumns();
+        Column[] columns = request.getConstrainedColumns();
         Object[] values = request.getSingleValues();
         Util.assertTrue(columns.length == values.length);
         SqlQuery sqlQuery = getSqlQuery();

src/main/mondrian/rolap/SqlMemberSource.java

@@ -585,7 +585,10 @@ private static AggStar chooseAggStar(MemberChildrenConstraint constraint, RolapM
             // One or more calculated members. Cannot use agg table.
             return null;
         }
-        RolapStar.Column[] columns = request.getColumns();
+        // TODO: RME why is this using the array of constrained columns
+        // from the CellRequest rather than just the constrained columns
+        // BitKey (method getConstrainedColumnsBitKey)?
+        RolapStar.Column[] columns = request.getConstrainedColumns();
         for (int i = 0; i < columns.length; i++) {
             levelBitKey.set(columns[i].getBitPosition());
         }

src/main/mondrian/rolap/agg/Aggregation.java

@@ -60,6 +60,10 @@ public class Aggregation {
 
     private final RolapStar star;
 
+    /** 
+     * This is a BitKey for ALL columns (Measures and Levels) involved in the 
+     * query. 
+     */
     private final BitKey bitKey;
 
     /**

src/main/mondrian/rolap/agg/AggregationManager.java

@@ -57,8 +57,8 @@ public Logger getLogger() {
      * RolapStar creates an Aggregation if needed.
      * 
      * @param measures 
-     * @param columns 
-     * @param bitKey 
+     * @param columns this is the CellRequest's constrained columns
+     * @param bitKey this is the CellRequest's constrained column BitKey
      * @param constraintses 
      * @param pinnedSegments 
      */
@@ -87,7 +87,7 @@ public void loadAggregation(
     public Object getCellFromCache(CellRequest request) {
         RolapStar.Measure measure = request.getMeasure();
         Aggregation aggregation = measure.getStar().lookupAggregation(
-            request.getBatchKey());
+            request.getConstrainedColumnsBitKey());
 
         if (aggregation == null) {
             // cell is not in any aggregation
@@ -106,7 +106,7 @@ public Object getCellFromCache(CellRequest request, Set pinSet) {
 
         RolapStar.Measure measure = request.getMeasure();
         Aggregation aggregation = measure.getStar().lookupAggregation(
-            request.getBatchKey());
+            request.getConstrainedColumnsBitKey());
 
         if (aggregation == null) {
             // cell is not in any aggregation
@@ -236,15 +236,18 @@ public AggStar findAgg(
         // of the combined measure BitKey and foreign-key/level BitKey.
         //
         // On the other hand, if there is at least one distinct count
-        // measure, isDistinct == true, then want is wanted is an AggStar
+        // measure, isDistinct == true, then what is wanted is an AggStar
         // whose measure BitKey is a superset of the measure BitKey,
         // whose level BitKey is an exact match and the aggregate table
         // can NOT have any foreign keys.
         assert rollup != null;
         BitKey fullBitKey = levelBitKey.or(measureBitKey);
-        // superset match
+
+        // The AggStars are already ordered from smallest to largest so
+        // we need only find the first one and return it.
         for (Iterator it = star.getAggStars().iterator(); it.hasNext(); ) {
             AggStar aggStar = (AggStar) it.next();
+            // superset match
             if (!aggStar.superSetMatch(fullBitKey)) {
                 continue;
             }

src/main/mondrian/rolap/agg/CellRequest.java

@@ -38,14 +38,26 @@ public class CellRequest {
      * that 2 requests for the same columns on measures in the same star get
      * put into the same batch.
      */
-    private final List columnList = new ArrayList();
+    private final List constrainedColumnList = new ArrayList();
     private final List valueList = new ArrayList();
-    private RolapStar.Column[] columns = null;
+
+    /** 
+     * After all of the columns are loaded into the constrainedColumnList instance
+     * variable, this columnsCache is created the first time the getColumns
+     * method is called. 
+     * <p>
+     * It is assumed that the call to all additional columns, addConstrainedColumn,
+     * will not be called after the first call to getColumns method.
+     */
+    private RolapStar.Column[] columnsCache = null;
     /**
      * A bit is set for each column in the column list. Allows us to rapidly
      * figure out whether two requests are for the same column set.
+     * These are all of the columns that are involved with a query, that is, all 
+     * required to be present in an aggregate table for the table be used to
+     * fulfill the query.
      */
-    private final BitKey bitKey;
+    private final BitKey constrainedColumnsBitKey;
     /**
      * Whether the request is impossible to satisfy. This is set to 'true' if
      * contradictory constraints are applied to the same column. For example,
@@ -65,8 +77,8 @@ public CellRequest(
         this.measure = measure;
         this.extendedContext = extendedContext;
         this.drillThrough = drillThrough;
-        this.columnList.add(measure.getStar());
-        this.bitKey =
+        this.constrainedColumnList.add(measure.getStar());
+        this.constrainedColumnsBitKey =
             BitKey.Factory.makeBitKey(measure.getStar().getColumnCount());
     }
 
@@ -80,14 +92,18 @@ public CellRequest(
     public void addConstrainedColumn(
             RolapStar.Column column,
             ColumnConstraint constraint) {
+
+        assert columnsCache == null;
+
         final int bitPosition = column.getBitPosition();
-        if (this.bitKey.get(bitPosition)) {
+        if (this.constrainedColumnsBitKey.get(bitPosition)) {
             // This column is already constrained. Unless the value is the same,
             // or this value or the previous value is null (meaning
             // unconstrained) the request will never return any results.
-            int index = columnList.indexOf(column);
+            int index = constrainedColumnList.indexOf(column);
             Util.assertTrue(index >= 0);
-            --index; // column list has dummy first element
+            // column list has RolapStar as its first element
+            --index; 
             final ColumnConstraint prevValue =
                     (ColumnConstraint) valueList.get(index);
             if (prevValue == null) {
@@ -104,40 +120,40 @@ public void addConstrainedColumn(
                 unsatisfiable = true;
             }
             valueList.set(index, constraint);
-            return;
-        }
 
-        this.columnList.add(column);
-        this.bitKey.set(bitPosition);
-        this.valueList.add(constraint);
+        } else {
+            this.constrainedColumnList.add(column);
+            this.constrainedColumnsBitKey.set(bitPosition);
+            this.valueList.add(constraint);
+        }
     }
 
     public RolapStar.Measure getMeasure() {
         return measure;
     }
 
-    public RolapStar.Column[] getColumns() {
-        if (this.columns == null) {
+    public RolapStar.Column[] getConstrainedColumns() {
+        if (this.columnsCache == null) {
             // This is called more than once so caching the value makes
             // sense.
-            makeColumns();
+            makeColumnsCache();
         }
-        return this.columns;
+        return this.columnsCache;
     }
 
-    private void makeColumns() {
-        // ignore the star, the 0th element of columnList
-        this.columns = new RolapStar.Column[columnList.size() - 1];
-        for (int i = 0; i < this.columns.length; i++) {
-            columns[i] = (RolapStar.Column) columnList.get(i + 1);
+    private void makeColumnsCache() {
+        // ignore the star, the 0th element of constrainedColumnList
+        this.columnsCache = new RolapStar.Column[constrainedColumnList.size() - 1];
+        for (int i = 0; i < this.columnsCache.length; i++) {
+            columnsCache[i] = (RolapStar.Column) constrainedColumnList.get(i + 1);
         }
     }
 
     /**
      * Returns the BitKey for the list of columns.
      */
-    public BitKey getBatchKey() {
-        return bitKey;
+    public BitKey getConstrainedColumnsBitKey() {
+        return constrainedColumnsBitKey;
     }
 
     public List getValueList() {

src/main/mondrian/rolap/agg/DrillThroughQuerySpec.java

@@ -86,7 +86,7 @@ public String getMeasureAlias(final int i) {
     }
 
     public RolapStar.Column[] getColumns() {
-        return request.getColumns();
+        return request.getConstrainedColumns();
     }
 
     public String getColumnAlias(final int i) {

src/main/mondrian/rolap/aggmatcher/AggStar.java

@@ -175,9 +175,27 @@ private static void collectLevels(
 
     private final RolapStar star;
     private final AggStar.FactTable aggTable;
+
+    /** 
+     * This BitKey is for all of the columns in the AggStar (levels and
+     * measures). 
+     */
     private final BitKey bitKey;
+
+    /** 
+     * BitKey of the levels (levels and foreign keys) of this AggStar. 
+     */
     private final BitKey levelBitKey;
+
+    /** 
+     * BitKey of the measures of this AggStar. 
+     */
     private final BitKey measureBitKey;
+
+    /** 
+     * BitKey of those measures of this AggStar that are distinct count
+     * aggregates. 
+     */
     private final BitKey distinctMeasureBitKey;
     private final AggStar.Table.Column[] columns;