ECC-1150: keys 'lowerLimit' and 'upperLimit' cannot be MISSING

definitions/grib2/template.4.probability.def

@@ -1,31 +1,30 @@
 # (C) Copyright 2005- ECMWF.
 
-#  Forecast probability number
+# Forecast probability number
 unsigned[1] forecastProbabilityNumber  : dump;
 
-#  Total number of forecast probabilities
+# Total number of forecast probabilities
 unsigned[1] totalNumberOfForecastProbabilities  : dump;
 
-#  Probability type
+# Probability type
 codetable[1] probabilityType ('4.9.table',masterDir,localDir) : dump;
 meta probabilityTypeName codetable_title(probabilityType): read_only;
 
-
-#  Scale factor of lower limit
+# Scale factor of lower limit
 signed[1] scaleFactorOfLowerLimit : can_be_missing,dump ;
 
-#  Scaled value of lower limit
+# Scaled value of lower limit
 signed[4] scaledValueOfLowerLimit : can_be_missing,dump ;
 
 meta lowerLimit from_scale_factor_scaled_value(
-     scaleFactorOfLowerLimit, scaledValueOfLowerLimit);
+     scaleFactorOfLowerLimit, scaledValueOfLowerLimit): can_be_missing;
 
 
-#  Scale factor of upper limit
+# Scale factor of upper limit
 signed[1] scaleFactorOfUpperLimit : can_be_missing,dump;
 
-#  Scaled value of upper limit
+# Scaled value of upper limit
 signed[4] scaledValueOfUpperLimit : can_be_missing,dump;
 
 meta upperLimit from_scale_factor_scaled_value(
-     scaleFactorOfUpperLimit, scaledValueOfUpperLimit);
+     scaleFactorOfUpperLimit, scaledValueOfUpperLimit): can_be_missing;

src/grib_accessor_class_from_scale_factor_scaled_value.c

@@ -246,16 +246,22 @@ static int unpack_double(grib_accessor* a, double* val, size_t* len)
 
     if ((ret = grib_get_long_internal(hand, self->scaleFactor, &scaleFactor)) != GRIB_SUCCESS)
         return ret;
-    /* ECC-966: If scale factor is missing, print error and treat it as zero (as a fallback) */
-    if (grib_is_missing(hand, self->scaleFactor, &ret) && ret == GRIB_SUCCESS) {
-        grib_context_log(a->context, GRIB_LOG_ERROR,
-                         "unpack_double for %s: %s is missing! Setting it to zero", a->name, self->scaleFactor);
-        scaleFactor = 0;
-    }
-
     if ((ret = grib_get_long_internal(hand, self->scaledValue, &scaledValue)) != GRIB_SUCCESS)
         return ret;
 
+    if (grib_is_missing(hand, self->scaledValue, &ret) && ret == GRIB_SUCCESS) {
+        *val = GRIB_MISSING_DOUBLE;
+        *len = 1;
+        return GRIB_SUCCESS;
+    } else {
+        /* ECC-966: If scale factor is missing, print error and treat it as zero (as a fallback) */
+        if (grib_is_missing(hand, self->scaleFactor, &ret) && ret == GRIB_SUCCESS) {
+            grib_context_log(a->context, GRIB_LOG_ERROR,
+                    "unpack_double for %s: %s is missing! Using zero instead", a->name, self->scaleFactor);
+            scaleFactor = 0;
+        }
+    }
+
     *val = scaledValue;
 
     /* The formula is:
@@ -289,5 +295,5 @@ static int is_missing(grib_accessor* a)
     if ((ret = grib_get_long_internal(grib_handle_of_accessor(a), self->scaledValue, &scaledValue)) != GRIB_SUCCESS)
         return ret;
 
-    return ((scaleFactor == GRIB_MISSING_LONG) | (scaledValue == GRIB_MISSING_LONG));
+    return ((scaleFactor == GRIB_MISSING_LONG) || (scaledValue == GRIB_MISSING_LONG));
 }

src/grib_accessor_class_gen.c

@@ -275,7 +275,10 @@ static int unpack_long(grib_accessor* a, long* v, size_t* len)
         double val = 0.0;
         size_t l   = 1;
         grib_unpack_double(a, &val, &l);
-        *v = (long)val;
+        if (val == GRIB_MISSING_DOUBLE)
+            *v = GRIB_MISSING_LONG;
+        else
+            *v = (long)val;
         grib_context_log(a->context, GRIB_LOG_DEBUG, " Casting double %s to long", a->name);
         return GRIB_SUCCESS;
     }

tests/CMakeLists.txt

@@ -63,9 +63,11 @@ if( HAVE_BUILD_TOOLS )
         grib_uerra
         grib_2nd_order_numValues
         grib_ecc-136
+        grib_ecc-966
         grib_ecc-967
-        grib_ecc-1065
+        grib_ecc-1150
         grib_ecc-1053
+        grib_ecc-1065
         bufr_json_samples
         bufr_ecc-359
         bufr_ecc-517

tests/grib_ecc-1150.sh

@@ -0,0 +1,41 @@
+#!/bin/sh
+# (C) Copyright 2005- ECMWF.
+#
+# This software is licensed under the terms of the Apache Licence Version 2.0
+# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
+# virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
+#
+
+. ./include.sh
+set -u
+# ---------------------------------------------------------
+# This is the test for the JIRA issue ECC-1150
+# ECC-1150: keys 'lowerLimit' and 'upperLimit' cannot be MISSING
+# ---------------------------------------------------------
+label="grib_ecc-1150-test"
+tempGrib=temp.${label}.grib
+tempFilt=temp.${label}.filt
+
+in=$ECCODES_SAMPLES_PATH/GRIB2.tmpl
+${tools_dir}/grib_set -s \
+   productDefinitionTemplateNumber=5,scaleFactorOfLowerLimit=missing,scaledValueOfLowerLimit=missing \
+   $in $tempGrib
+
+grib_check_key_equals $tempGrib lowerLimit MISSING
+grib_check_key_equals $tempGrib upperLimit 0
+
+cat > $tempFilt <<EOF
+ print "lower=[lowerLimit], upper=[upperLimit]";
+ transient llim_missing = missing(lowerLimit);
+ transient ulim_missing = missing(upperLimit);
+
+ assert( llim_missing == 1 );
+ assert( ulim_missing == 0 );
+EOF
+${tools_dir}/grib_filter $tempFilt $tempGrib
+
+
+# Clean up
+rm -f $tempGrib $tempFilt

tests/grib_ecc-966.sh

@@ -0,0 +1,28 @@
+#!/bin/sh
+# (C) Copyright 2005- ECMWF.
+#
+# This software is licensed under the terms of the Apache Licence Version 2.0
+# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
+# virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
+#
+
+. ./include.sh
+set -u
+# ---------------------------------------------------------
+# This is the test for the JIRA issue ECC-966.
+# Crash: setting shapeOfTheEarth=1 but scale factor missing
+# ---------------------------------------------------------
+label="grib_ecc-966-test"
+tempGrib=temp.${label}.grib
+tempErrs=temp.${label}.errs
+
+in=$ECCODES_SAMPLES_PATH/GRIB2.tmpl
+${tools_dir}/grib_set -s shapeOfTheEarth=1,scaledValueOfRadiusOfSphericalEarth=6367999 $in $tempGrib
+grib_check_key_equals $tempGrib scaleFactorOfRadiusOfSphericalEarth MISSING
+grib_check_key_equals $tempGrib 'radius:i' 6367999 2>$tempErrs
+grep -q "ECCODES ERROR   :  unpack_double for radius" $tempErrs
+
+# Clean up
+rm -f $tempGrib $tempErrs