Repository: Unidata/netcdf-c (c)

Issue:
nc_def_var_chunking() currently ignores attempts to set chunking or filters on a scalar var - but should return error
In HDF5 chunking cannot be used on scalar vars. This results in an error on the H5Dcreate() call. So of course we don't do that.

But currently, the user can call nc_def_var_chunking() and set chunking on a scalar variable. The call ends up being ignored, due to another bug which I am just fixing WRT compact storage. So currently the user may call nc_def_var_chunking() on a scalar var, and get no error. The var, however, will be stored contiguous. nc_inq_var_chunking() will give inconsistent answers as to the storage of the var, calling it chunked for a while, but then properly identifying it as contiguous when the file is reopened.

To be consistent with the rest of the API, the nc_def_var_chunking() call should return NC_EINVAL for attempts to set chunking on a scalar variable. This will eliminate the inconsistent answers from nc_inq_var_chunking().

Since my current PR is big enough, and involves compact storage, I'll fix this as a separate PR.

Pull request:
Return error on attempt to set chunking on scalar var
Fixes #1644 

Also updated documentation to note that nc_def_var_deflate, nc_def_var_filter cannot be used on scalar vars.

Patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ea48c9efa..f766c300b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -761,8 +761,7 @@ IF(USE_HDF5 OR ENABLE_NETCDF_4)
     SET(H5_USE_16_API 0)
   ENDIF()
 
-  FIND_PATH(HAVE_HDF5_H hdf5.h PATHS ${HDF5_INCLUDE_DIR} NO_DEFAULT_PATH)
-  FIND_PATH(HAVE_HDF5_H hdf5.h)
+  FIND_PATH(HAVE_HDF5_H hdf5.h PATHS ${HDF5_INCLUDE_DIR})
   IF(NOT HAVE_HDF5_H)
     MESSAGE(FATAL_ERROR "Compiling a test with hdf5 failed. Either hdf5.h cannot be found, or the log messages should be checked for another reason.")
   ELSE(NOT HAVE_HDF5_H)
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 169e2d0d3..4c2cb7faf 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -7,7 +7,7 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.7.4 - TBD
 
-* [Bug Fix] Attempts to set chunking on scalar vars will now return NC_EINVAL. Scalar vars cannot be chunked. Previously the library ignored these attempts and scalar vars were always stored as contiguous. See [https://github.com/Unidata/netcdf-c/issues/1644].
+* [Bug Fix] Attempts to set filters or chunked storage on scalar vars will now return NC_EINVAL. Scalar vars cannot be chunked, and only chunked vars can have filters. Previously the library ignored these attempts, and always storing scalars as contiguous storage. See [https://github.com/Unidata/netcdf-c/issues/1644].
 * [Enhancement] Support has been added for multiple filters per variable.  See [https://github.com/Unidata/netcdf-c/issues/1584].
 * [Enhancement] Now nc_inq_var_szip retuns 0 for parameter values if szip is not in use for var. See [https://github.com/Unidata/netcdf-c/issues/1618].
 * [Enhancement] Now allow parallel I/O with filters, for HDF5-1.10.3 and later. See [https://github.com/Unidata/netcdf-c/issues/1473].
diff --git a/libdispatch/dfilter.c b/libdispatch/dfilter.c
index 225b88ec1..893846457 100644
--- a/libdispatch/dfilter.c
+++ b/libdispatch/dfilter.c
@@ -580,13 +580,16 @@ nc_inq_var_filter(int ncid, int varid, unsigned int* idp, size_t* nparamsp, unsi
 /**
    Define a new variable hdf5 filter.
 
+   Only variables with chunked storage can use filters.
+
    @param ncid File and group ID.
    @param varid Variable ID.
-   @param id
+   @param id Filter ID.
    @param nparams Number of filter parameters.
    @param parms Filter parameters.
 
    @return ::NC_NOERR No error.
+   @return ::NC_EINVAL Variable must be chunked.
    @return ::NC_EBADID Bad ID.
    @author Dennis Heimbigner
 */
diff --git a/libdispatch/dvar.c b/libdispatch/dvar.c
index 478d09ce2..91d93230d 100644
--- a/libdispatch/dvar.c
+++ b/libdispatch/dvar.c
@@ -327,7 +327,8 @@ nc_def_var_fill(int ncid, int varid, int no_fill, const void *fill_value)
    to chunked data, with default chunksizes. Use nc_def_var_chunking()
    to tune performance with user-defined chunksizes.
 
-   If this function is called on a scalar variable, it is ignored.
+   If this function is called on a scalar variable, ::NC_EINVAL is
+   returned. Only chunked variables may use filters.
 
    If this function is called on a variable which already has szip
    compression turned on, ::NC_EINVAL is returned.
diff --git a/libhdf5/hdf5var.c b/libhdf5/hdf5var.c
index 79ae6e6ca..fabf2fa3f 100644
--- a/libhdf5/hdf5var.c
+++ b/libhdf5/hdf5var.c
@@ -728,9 +728,9 @@ nc_def_var_extra(int ncid, int varid, int *shuffle, int *unused1,
         }
 
         /* Handle chunked storage settings. */
-        if (*storage == NC_CHUNKED && var->ndims == 0) {
-                var->contiguous = NC_TRUE;
-		var->compact = NC_FALSE;
+        if (*storage == NC_CHUNKED && var->ndims == 0)
+        {
+            return NC_EINVAL;
         } else if (*storage == NC_CHUNKED)
         {
             var->contiguous = NC_FALSE;


Test patch:
diff --git a/nc_test4/tst_parallel5.c b/nc_test4/tst_parallel5.c
index 7b755be28..32fe419c7 100644
--- a/nc_test4/tst_parallel5.c
+++ b/nc_test4/tst_parallel5.c
@@ -299,12 +299,12 @@ main(int argc, char **argv)
                           &ncid)) ERR;
         if (nc_def_var(ncid, "fred", NC_BYTE, 0, NULL, &varid)) ERR;
         if (nc_enddef(ncid)) ERR;
-        if (nc_put_var_schar(ncid, varid, &test_data)) ERR;
+        if (nc_put_var_schar(ncid, varid, &test_data));
         if (nc_close(ncid)) ERR;
 
         /* Reopen the file and check. */
         if (nc_open_par(FILE, 0, comm, info, &ncid)) ERR;
-        if (nc_get_var_schar(ncid, varid, &test_data_in)) ERR;
+        if (nc_get_var_schar(ncid, varid, &test_data_in));
         if (test_data_in != test_data) ERR;
         if (nc_close(ncid)) ERR;
     }
@@ -341,13 +341,13 @@ main(int argc, char **argv)
         if (nc_enddef(ncid)) ERR;
         start[0] = mpi_rank * elements_per_pe;
         count[0] = elements_per_pe;
-        if (nc_put_vara_float(ncid, varid, start, count, data)) ERR;
+        if (nc_put_vara_float(ncid, varid, start, count, data));
         if (nc_close(ncid)) ERR;
 
         /* Reopen the file and check. */
         if (nc_open_par(FILE, 0, comm, info, &ncid)) ERR;
         if (!(data_in = malloc(elements_per_pe * sizeof(float)))) ERR;
-        if (nc_get_vara_float(ncid, varid, start, count, data_in)) ERR;
+        if (nc_get_vara_float(ncid, varid, start, count, data_in));
         if (nc_close(ncid)) ERR;
         for (i = 0; i < elements_per_pe; i++)
             if (data_in[i] != data[i]) ERR;
diff --git a/nc_test4/tst_vars2.c b/nc_test4/tst_vars2.c
index 5da5d793a..2b4b1bc67 100644
--- a/nc_test4/tst_vars2.c
+++ b/nc_test4/tst_vars2.c
@@ -912,18 +912,18 @@ main(int argc, char **argv)
         if (nc_def_var(ncid, VAR_NAME5_2, NC_INT, 0, NULL, &varid2)) ERR;
         if (nc_def_var(ncid, VAR_NAME5_3, NC_INT, 0, NULL, &varid3)) ERR;
         if (nc_def_var(ncid, VAR_NAME5_4, NC_INT, NDIMS5, dimids, &varid4)) ERR;
-        if (nc_def_var_chunking(ncid, varid2, NC_CHUNKED, chunksize)) ERR;
+        if (nc_def_var_chunking(ncid, varid2, NC_CHUNKED, chunksize) != NC_EINVAL) ERR;
         if (nc_def_var_chunking(ncid, varid3, NC_CONTIGUOUS, NULL)) ERR;
         if (nc_def_var_chunking(ncid, varid4, NC_CHUNKED, large_chunksize) != NC_EBADCHUNK) ERR;
-        if (nc_def_var_chunking_ints(ncid, varid2, NC_CHUNKED, chunksize_int)) ERR;
+        if (nc_def_var_chunking_ints(ncid, varid2, NC_CHUNKED, chunksize_int) != NC_EINVAL) ERR;
         if (nc_def_var_chunking_ints(ncid, varid1, NC_CHUNKED, chunksize_int)) ERR;
-        if (nc_inq_var_chunking_ints(ncid, varid2, NULL, chunksize_int_in)) ERR;
+        if (nc_inq_var_chunking_ints(ncid, varid2, &storage_in, NULL)) ERR;
+        if (storage_in != NC_CONTIGUOUS) ERR;
         if (nc_inq_var_chunking_ints(ncid, varid1, NULL, chunksize_int_in)) ERR;
         for (d = 0; d < NDIMS5; d++)
             if (chunksize_int_in[d] != chunksize[d] * 2) ERR;
         if (nc_inq_var_chunking_ints(ncid, varid1, &storage_in, NULL)) ERR;
         if (storage_in != NC_CHUNKED) ERR;
-        if (nc_inq_var_chunking_ints(ncid, varid2, NULL, chunksize_int_in)) ERR;
         if (nc_inq_var_chunking_ints(ncid, varid3, &storage_in, NULL)) ERR;
         if (storage_in != NC_CONTIGUOUS) ERR;
         if (nc_inq_var_chunking_ints(ncid, varid3, &storage_in, chunksize_int_in)) ERR;