Fix bug when explicit pre-enhanced datasets are used in NcML Aggregations

cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java

@@ -889,7 +889,7 @@ public Set<Enhance> getEnhanceMode() {
     return enhanceMode;
   }
 
-  private void addEnhanceModes(Set<Enhance> addEnhanceModes) {
+  void addEnhanceModes(Set<Enhance> addEnhanceModes) {
     ImmutableSet.Builder<Enhance> result = new ImmutableSet.Builder<>();
     result.addAll(this.enhanceMode);
     result.addAll(addEnhanceModes);
@@ -1064,6 +1064,7 @@ public void empty() {
     coordAxes = new ArrayList<>();
     coordTransforms = new ArrayList<>();
     convUsed = null;
+    this.enhanceMode = Collections.emptySet();
   }
 
   /** @deprecated do not use */

cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java

@@ -7,6 +7,8 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import ucar.ma2.*;
 import ucar.nc2.*;
 import ucar.nc2.constants.CDM;
@@ -35,6 +37,8 @@
  */
 public class VariableDS extends Variable implements VariableEnhanced, EnhanceScaleMissingUnsigned {
 
+  private static final Logger logger = LoggerFactory.getLogger(VariableDS.class);
+
   static final List<EnhancementProvider> ENHANCEMENT_PROVIDERS;
 
   static {
@@ -263,6 +267,53 @@ boolean needConvert() {
         || enhancements.contains(Enhance.ApplyScaleOffset) || enhancements.contains(Enhance.ConvertMissing);
   }
 
+  /**
+   * Will this Variable modify the data values it reads, ie apply one of the data affecting enhancements?
+   * <p>
+   * Unlike {@link #needConvert()} this only considers the enhancements that this Variable applies itself,
+   * not those already applied by a wrapped Variable. Used by the aggregation proxy readers to decide if
+   * they must deliver the data as stored, so that an enhancement is not applied twice.
+   *
+   * @param v the Variable to test, may be null.
+   * @return true if v is a VariableDS that converts the data it reads.
+   */
+  public static boolean appliesDataEnhancements(Variable v) {
+    if (!(v instanceof VariableDS)) {
+      return false;
+    }
+    Set<Enhance> enhancements = ((VariableDS) v).enhanceMode;
+    return enhancements.contains(Enhance.ConvertEnums) || enhancements.contains(Enhance.ConvertUnsigned)
+        || enhancements.contains(Enhance.ApplyScaleOffset) || enhancements.contains(Enhance.ConvertMissing)
+        || enhancements.contains(Enhance.ApplyRuntimeLoadedEnhancements);
+  }
+
+  /**
+   * Find the Variable that an aggregation proxy should read from, so that the data affecting enhancements
+   * are applied exactly once.
+   * <p>
+   * If the aggregate Variable {@code mainV} will convert the data itself, and the member Variable
+   * {@code proxyV} has already applied its own enhancements, read the data as stored instead.
+   *
+   * @param proxyV the Variable found in the member (proxied) dataset.
+   * @param mainV the Variable in the aggregation that the data is being read for.
+   * @return the Variable to actually read from, never null if proxyV is not null.
+   */
+  public static Variable unenhancedProxy(Variable proxyV, Variable mainV) {
+    if (!(proxyV instanceof VariableDS) || !appliesDataEnhancements(mainV)) {
+      return proxyV;
+    }
+    VariableDS proxyDS = (VariableDS) proxyV;
+    Variable orgVar = proxyDS.getOriginalVariable();
+    if (orgVar == null || !appliesDataEnhancements(proxyDS)) {
+      return proxyV;
+    }
+    if (logger.isDebugEnabled() && !((VariableDS) mainV).enhanceMode.containsAll(proxyDS.enhanceMode)) {
+      logger.debug("Aggregation member {} applies enhancements {} not applied by the aggregation variable {}",
+          proxyV.getFullName(), proxyDS.enhanceMode, ((VariableDS) mainV).enhanceMode);
+    }
+    return orgVar;
+  }
+
   Array convert(Array data) {
     return convert(data, enhanceMode);
   }
@@ -503,14 +554,36 @@ protected Array _read(Section section) throws IOException, InvalidRangeException
     return convert(result);
   }
 
+  /**
+   * The data as stored by this VariableDS, ie without applying the enhancements that this Variable would apply.
+   * This is the array that {@link #_read()} passes to {@link #convert(Array)}.
+   */
+  Array readUnenhanced() throws IOException {
+    if (hasCachedData()) {
+      return super._read();
+    }
+    return proxyReader.reallyRead(this, null);
+  }
+
+  /** Section of {@link #readUnenhanced()}. */
+  Array readUnenhanced(Section section) throws IOException, InvalidRangeException {
+    if ((null == section) || section.computeSize() == getSize()) {
+      return readUnenhanced();
+    }
+    if (hasCachedData()) {
+      return super._read(section);
+    }
+    return proxyReader.reallyRead(this, section, null);
+  }
+
   // do not call directly
   @Override
   public Array reallyRead(Variable client, CancelTask cancelTask) throws IOException {
+    if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) {
+      return this.proxyReader.reallyRead(client, cancelTask);
+    }
+
     if (orgVar == null) {
-      // possible aggregation
-      if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) {
-        return this.proxyReader.reallyRead(client, cancelTask);
-      }
       return getMissingDataArray(shape);
     }
 
@@ -527,6 +600,11 @@ public Array reallyRead(Variable client, CancelTask cancelTask) throws IOExcepti
       if (ucar.nc2.ncml.Aggregation.instanceOfDatasetProxyReader(this.proxyReader)) {
         return this.proxyReader.reallyRead(client, cancelTask);
       }
+      if (orgVar instanceof VariableDS) {
+        // orgVar has cached data. The cache holds the data as stored, so hand that to the client
+        // rather than letting orgVar convert it, otherwise enhancements get applied twice.
+        return ((VariableDS) orgVar).readUnenhanced();
+      }
     }
     return orgVar.read();
   }
@@ -539,11 +617,11 @@ public Array reallyRead(Variable client, Section section, CancelTask cancelTask)
     if ((null == section) || section.computeSize() == getSize())
       return reallyRead(client, cancelTask);
 
+    if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) {
+      return this.proxyReader.reallyRead(client, section, cancelTask);
+    }
+
     if (orgVar == null) {
-      // possible aggregation
-      if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) {
-        return this.proxyReader.reallyRead(client, section, cancelTask);
-      }
       return getMissingDataArray(shape);
     }
 
@@ -560,6 +638,11 @@ public Array reallyRead(Variable client, Section section, CancelTask cancelTask)
       if (ucar.nc2.ncml.Aggregation.instanceOfDatasetProxyReader(this.proxyReader)) {
         return this.proxyReader.reallyRead(client, section, cancelTask);
       }
+      if (orgVar instanceof VariableDS) {
+        // orgVar has cached data. The cache holds the data as stored, so hand that to the client
+        // rather than letting orgVar convert it, otherwise enhancements get applied twice.
+        return ((VariableDS) orgVar).readUnenhanced(section);
+      }
     }
     return orgVar.read(section);
   }

cdm/core/src/main/java/ucar/nc2/internal/dataset/DatasetEnhancer.java

@@ -175,7 +175,7 @@ private void enhanceStructure(StructureDS.Builder<?> sdb) {
    * }
    */
 
-  private void enhanceVariable(VariableDS.Builder vb) {
+  private void enhanceVariable(VariableDS.Builder<?> vb) {
     Set<Enhance> varEnhance = EnumSet.copyOf(wantEnhance);
 
     // varEnhance will only contain enhancements not already applied to orgVar.
@@ -185,6 +185,9 @@ private void enhanceVariable(VariableDS.Builder vb) {
       }
     }
 
+    varEnhance.removeAll(vb.enhanceMode);
+    varEnhance.removeAll(dsBuilder.getEnhanceMode());
+
     // enhance() may have been called previously, with a different enhancement set.
     // So, we need to reset to default before we process this new set.
     // if (vb.orgDataType != null) {

cdm/core/src/main/java/ucar/nc2/internal/ncml/AggProxyReader.java

@@ -1,4 +1,8 @@
-/* Copyright Unidata */
+/*
+ * Copyright (c) 2019-2025 John Caron and University Corporation for Atmospheric Research/Unidata
+ * See LICENSE.txt for license information.
+ */
+
 package ucar.nc2.internal.ncml;
 
 import java.io.IOException;
@@ -9,6 +13,7 @@
 import ucar.nc2.NetcdfFile;
 import ucar.nc2.ProxyReader;
 import ucar.nc2.Variable;
+import ucar.nc2.dataset.VariableDS;
 import ucar.nc2.dataset.VariableEnhanced;
 import ucar.nc2.util.CancelTask;
 
@@ -33,7 +38,7 @@ public Array reallyRead(Variable mainV, CancelTask cancelTask) throws IOExceptio
       ncfile = dataset.acquireFile(cancelTask);
       if ((cancelTask != null) && cancelTask.isCancel())
         return null;
-      Variable proxyV = findVariable(ncfile, mainV);
+      Variable proxyV = readTarget(ncfile, mainV);
       return proxyV.read();
     } finally {
       dataset.close(ncfile);
@@ -46,7 +51,7 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask)
     NetcdfFile ncfile = null;
     try {
       ncfile = dataset.acquireFile(cancelTask);
-      Variable proxyV = findVariable(ncfile, mainV);
+      Variable proxyV = readTarget(ncfile, mainV);
       if ((cancelTask != null) && cancelTask.isCancel())
         return null;
       return proxyV.read(section);
@@ -57,6 +62,14 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask)
   }
 
 
+  /**
+   * Find the Variable to read from the member dataset. If the member dataset was itself enhanced, and the
+   * aggregation variable will apply the same enhancements again, read the data as stored instead.
+   */
+  private Variable readTarget(NetcdfFile ncfile, Variable mainV) {
+    return VariableDS.unenhancedProxy(findVariable(ncfile, mainV), mainV);
+  }
+
   protected Variable findVariable(NetcdfFile ncfile, Variable mainV) {
     Variable v = ncfile.findVariable(mainV.getFullNameEscaped());
     if (v == null) { // might be renamed

cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationExisting.java

@@ -117,6 +117,9 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException {
       VariableDS.Builder vagg = VariableDS.builder().setName(v.getShortName()).setDataType(v.getDataType())
           .setParentGroupBuilder(rootGroup).setDimensionsByName(v.getDimensionsString());
       vagg.setProxyReader(this);
+      if (v instanceof VariableDS) {
+        vagg.setOriginalVariable(v);
+      }
       BuilderHelper.transferAttributes(v, vagg.getAttributeContainer());
 
       rootGroup.replaceVariable(vagg);

cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationNew.java

@@ -115,6 +115,10 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException {
       VariableDS.Builder vagg = VariableDS.builder().setName(aggVar.shortName).setDataType(aggVar.dataType)
           .setParentGroupBuilder(root).setDimensionsByName(dimName + " " + aggVar.makeDimensionsString());
       vagg.setProxyReader(this);
+      if (aggVar instanceof VariableDS.Builder) {
+        VariableDS.Builder<?> vds = (VariableDS.Builder<?>) aggVar;
+        vagg.setOriginalVariable(vds.orgVar);
+      }
       BuilderHelper.transferAttributes(aggVar.getAttributeContainer(), vagg.getAttributeContainer());
 
       // _CoordinateAxes if it exists must be modified

cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java

@@ -554,7 +554,7 @@ private static void setEnhanceMode(NetcdfDataset.Builder builder, Element netcdf
     Set<NetcdfDataset.Enhance> mode = parseEnhanceMode(netcdfElem.getAttributeValue("enhance"));
     if (mode != null) {
       // cant just set enhance mode
-      if (DatasetEnhancer.enhanceNeeded(mode, null)) {
+      if (DatasetEnhancer.enhanceNeeded(mode, builder.getEnhanceMode())) {
         DatasetEnhancer enhancer = new DatasetEnhancer(builder, mode, cancelTask);
         enhancer.enhance();
         builder.setEnhanceMode(mode);

cdm/core/src/main/java/ucar/nc2/ncml/Aggregation.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998-2025 John Caron and University Corporation for Atmospheric Research/Unidata
+ * Copyright (c) 1998-2026 John Caron and University Corporation for Atmospheric Research/Unidata
  * See LICENSE.txt for license information.
  */
 
@@ -22,6 +22,7 @@
 import ucar.nc2.Variable;
 import ucar.nc2.dataset.DatasetUrl;
 import ucar.nc2.dataset.NetcdfDataset;
+import ucar.nc2.dataset.VariableDS;
 import ucar.nc2.dataset.VariableEnhanced;
 import ucar.nc2.units.DateFormatter;
 import ucar.nc2.util.CancelTask;
@@ -823,7 +824,7 @@ public Array reallyRead(Variable mainV, CancelTask cancelTask) throws IOExceptio
         ncfile = dataset.acquireFile(cancelTask);
         if ((cancelTask != null) && cancelTask.isCancel())
           return null;
-        Variable proxyV = findVariable(ncfile, mainV);
+        Variable proxyV = readTarget(ncfile, mainV);
         return proxyV.read();
       } finally {
         dataset.close(ncfile);
@@ -836,7 +837,7 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask)
       NetcdfFile ncfile = null;
       try {
         ncfile = dataset.acquireFile(cancelTask);
-        Variable proxyV = findVariable(ncfile, mainV);
+        Variable proxyV = readTarget(ncfile, mainV);
         if ((cancelTask != null) && cancelTask.isCancel())
           return null;
         return proxyV.read(section);
@@ -847,6 +848,14 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask)
     }
   }
 
+  /**
+   * Find the Variable to read from the member dataset. If the member dataset was itself enhanced, and the
+   * aggregation variable will apply the same enhancements again, read the data as stored instead.
+   */
+  protected Variable readTarget(NetcdfFile ncfile, Variable mainV) {
+    return VariableDS.unenhancedProxy(findVariable(ncfile, mainV), mainV);
+  }
+
   protected Variable findVariable(NetcdfFile ncfile, Variable mainV) {
     Variable v = ncfile.findVariable(mainV.getFullNameEscaped());
     if (v == null) { // might be renamed

cdm/core/src/main/java/ucar/nc2/ncml/AggregationExisting.java

@@ -111,6 +111,9 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException {
       VariableDS vagg = new VariableDS(ncDataset, newGroup, null, v.getShortName(), v.getDataType(),
           v.getDimensionsString(), null, null);
       vagg.setProxyReader(this);
+      if (v instanceof VariableDS) {
+        vagg.setOriginalVariable(v);
+      }
       DatasetConstructor.transferVariableAttributes(v, vagg);
 
       newGroup.removeVariable(v.getShortName());

cdm/core/src/main/java/ucar/nc2/ncml/AggregationNew.java

@@ -106,6 +106,10 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException {
       VariableDS vagg = new VariableDS(ncDataset, newGroup, null, aggVar.getShortName(), aggVar.getDataType(),
           dimName + " " + aggVar.getDimensionsString(), null, null);
       vagg.setProxyReader(this);
+      if (aggVar instanceof VariableDS) {
+        VariableDS vds = (VariableDS) aggVar;
+        vagg.setOriginalVariable(vds.getOriginalVariable() != null ? vds.getOriginalVariable() : vds);
+      }
       DatasetConstructor.transferVariableAttributes(aggVar, vagg);
 
       // _CoordinateAxes if it exists must be modified

cdm/core/src/main/java/ucar/nc2/ncml/AggregationTiled.java

@@ -91,6 +91,9 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException {
         VariableDS vagg = new VariableDS(ncDataset, newGroup, null, v.getShortName(), v.getDataType(),
             v.getDimensionsString(), null, null); // LOOK what about anon dimensions?
         vagg.setProxyReader(this); // do the reading here
+        if (v instanceof VariableDS) {
+          vagg.setOriginalVariable(v);
+        }
         DatasetConstructor.transferVariableAttributes(v, vagg);
 
         newGroup.removeVariable(v.getShortName());

cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java

@@ -554,9 +554,9 @@ private void readNetcdf(String ncmlLocation, NetcdfDataset targetDS, NetcdfFile
 
     // enhance means do scale/offset and/or add CoordSystems
     Set<NetcdfDataset.Enhance> mode = NetcdfDataset.parseEnhanceMode(netcdfElem.getAttributeValue("enhance"));
-    // if (mode == null)
-    // mode = NetcdfDataset.getEnhanceDefault();
-    targetDS.enhance(mode);
+    if (mode != null) {
+      targetDS.enhance(mode);
+    }
 
     // optionally add record structure to netcdf-3
     String addRecords = netcdfElem.getAttributeValue("addRecords");

cdm/core/src/test/data/ncml/enhance/aggMemberEnhanced.ncml

@@ -0,0 +1,7 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<netcdf xmlns='http://www.unidata.ucar.edu/namespaces/netcdf/ncml-2.2'>
+  <aggregation dimName='time' type='joinExisting'>
+    <netcdf location="agg/scaledAgg1.nc" enhance='true' />
+    <netcdf location="agg/scaledAgg2.nc" enhance='true' />
+  </aggregation>
+</netcdf>

cdm/core/src/test/data/ncml/enhance/aggNoMemberEnhanced.ncml

@@ -0,0 +1,7 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<netcdf xmlns='http://www.unidata.ucar.edu/namespaces/netcdf/ncml-2.2'>
+  <aggregation dimName='time' type='joinExisting'>
+    <netcdf location="agg/scaledAgg1.nc" />
+    <netcdf location="agg/scaledAgg2.nc" />
+  </aggregation>
+</netcdf>