Merge pull request #2066 from DennisHeimbigner/zarrfixes.dmh

Fix a number of bugs in the nczarr code.
Unidata · Aug 12, 2021 · 1279681 · 1279681
2 parents a193b44 + 8be7d29
commit 1279681
Show file tree

Hide file tree

Showing 15 changed files with 292 additions and 58 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -7,11 +7,12 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.8.1 - TBD
 
-* [Enhancement] Support windows network paths (e.g. \\svc\...). See [Github #2065](https://github.com/Unidata/netcdf-c/issues/2065).
-* [Enhancement] Convert to a new representation of the NCZarr meta-data extensions: version 2. Read-only backward compatibility is provided. See [Github #2032](https://github.com/Unidata/netcdf-c/issues/2032).
-* [Bug Fix] Fix dimension_separator bug in libnczarr. See [Github #2035](https://github.com/Unidata/netcdf-c/issues/2035).
-* [Bug Fix] Fix bugs in libdap4. See [Github #2005](https://github.com/Unidata/netcdf-c/issues/2005).
-* [Bug Fix] Store NCZarr fillvalue as a singleton instead of a 1-element array. See [Github #2017](https://github.com/Unidata/netcdf-c/issues/2017).  
+* [Bug Fix] Fix multiple bugs in libnczarr. See [Github #2066](https://github.com/Unidata/netcdf-c/pull/2066).
+* [Enhancement] Support windows network paths (e.g. \\svc\...). See [Github #2065](https://github.com/Unidata/netcdf-c/pull/2065).
+* [Enhancement] Convert to a new representation of the NCZarr meta-data extensions: version 2. Read-only backward compatibility is provided. See [Github #2032](https://github.com/Unidata/netcdf-c/pull/2032).
+* [Bug Fix] Fix dimension_separator bug in libnczarr. See [Github #2035](https://github.com/Unidata/netcdf-c/pull/2035).
+* [Bug Fix] Fix bugs in libdap4. See [Github #2005](https://github.com/Unidata/netcdf-c/pull/2005).
+* [Bug Fix] Store NCZarr fillvalue as a singleton instead of a 1-element array. See [Github #2017](https://github.com/Unidata/netcdf-c/pull/2017).  
 * [Bug Fixes] The netcdf-c library was incorrectly determining the scope of dimension; similar to the type scope problem. See [Github #2012](https://github.com/Unidata/netcdf-c/pull/2012) for more information.
 * [Bug Fix] Re-enable DAP2 authorization testing. See [Github #2011](https://github.com/Unidata/netcdf-c/issues/2011).  
 * [Bug Fix] Fix bug with windows version of mkstemp that causes failure to create more than 26 temp files. See [Github #1998](https://github.com/Unidata/netcdf-c/pull/1998).  

diff --git a/libnczarr/zs3sdk.cpp b/libnczarr/zs3sdk.cpp
@@ -78,7 +78,9 @@ NCZ_s3sdkcreateconfig(const char* host, const char* region, void** configp)
     if(region) config->region = region;
     if(host) config->endpointOverride = host;
     config->enableEndpointDiscovery = true;
-    config->followRedirects = true;
+#if 0
+    config->followRedirects = Aws::Client::FollowRedirectsPolicy::ALWAYS;
+#endif
     if(configp) * configp = config;
     return ZUNTRACE(stat);
 }

diff --git a/libnczarr/zsync.c b/libnczarr/zsync.c
@@ -116,6 +116,7 @@ ncz_sync_grp(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp)
     int i,stat = NC_NOERR;
     NCZ_FILE_INFO_T* zinfo = NULL;
     char version[1024];
+    int purezarr = 0;
     NCZMAP* map = NULL;
     char* fullpath = NULL;
     char* key = NULL;
@@ -132,47 +133,50 @@ ncz_sync_grp(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp)
     zinfo = file->format_file_info;
     map = zinfo->map;
 
+    purezarr = (zinfo->controls.flags & FLAG_PUREZARR)?1:0;
+
     /* Construct grp key */
     if((stat = NCZ_grpkey(grp,&fullpath)))
 	goto done;
 
-    /* Create dimensions dict */
-    if((stat = ncz_collect_dims(file,grp,&jdims))) goto done;
+    if(!purezarr) {
+        /* Create dimensions dict */
+        if((stat = ncz_collect_dims(file,grp,&jdims))) goto done;
 
-    /* Create vars list */
-    if((stat = NCJnew(NCJ_ARRAY,&jvars)))
-	goto done;
-    for(i=0; i<ncindexsize(grp->vars); i++) {
-	NC_VAR_INFO_T* var = (NC_VAR_INFO_T*)ncindexith(grp->vars,i);
-	if((stat = NCJaddstring(jvars,NCJ_STRING,var->hdr.name))) goto done;
-    }
+        /* Create vars list */
+        if((stat = NCJnew(NCJ_ARRAY,&jvars)))
+	    goto done;
+        for(i=0; i<ncindexsize(grp->vars); i++) {
+	    NC_VAR_INFO_T* var = (NC_VAR_INFO_T*)ncindexith(grp->vars,i);
+	    if((stat = NCJaddstring(jvars,NCJ_STRING,var->hdr.name))) goto done;
+        }
 
-    /* Create subgroups list */
-    if((stat = NCJnew(NCJ_ARRAY,&jsubgrps)))
-	goto done;
-    for(i=0; i<ncindexsize(grp->children); i++) {
-	NC_GRP_INFO_T* g = (NC_GRP_INFO_T*)ncindexith(grp->children,i);
-	if((stat = NCJaddstring(jsubgrps,NCJ_STRING,g->hdr.name))) goto done;
+        /* Create subgroups list */
+        if((stat = NCJnew(NCJ_ARRAY,&jsubgrps)))
+    	    goto done;
+        for(i=0; i<ncindexsize(grp->children); i++) {
+	    NC_GRP_INFO_T* g = (NC_GRP_INFO_T*)ncindexith(grp->children,i);
+	    if((stat = NCJaddstring(jsubgrps,NCJ_STRING,g->hdr.name))) goto done;
+        }
+        /* Create the "_NCZARR_GROUP" dict */
+        if((stat = NCJnew(NCJ_DICT,&json)))
+	    goto done;
+        /* Insert the various dicts and arrays */
+        if((stat = NCJinsert(json,"dims",jdims))) goto done;
+        jdims = NULL; /* avoid memory problems */
+        if((stat = NCJinsert(json,"vars",jvars))) goto done;
+        jvars = NULL; /* avoid memory problems */
+        if((stat = NCJinsert(json,"groups",jsubgrps))) goto done;
+        jsubgrps = NULL; /* avoid memory problems */
     }
 
-    /* Create the "_NCZARR_GROUP" dict */
-    if((stat = NCJnew(NCJ_DICT,&json)))
-	goto done;
-    /* Insert the various dicts and arrays */
-    if((stat = NCJinsert(json,"dims",jdims))) goto done;
-    jdims = NULL; /* avoid memory problems */
-    if((stat = NCJinsert(json,"vars",jvars))) goto done;
-    jvars = NULL; /* avoid memory problems */
-    if((stat = NCJinsert(json,"groups",jsubgrps))) goto done;
-    jsubgrps = NULL; /* avoid memory problems */
-
     /* build ZGROUP contents */
     if((stat = NCJnew(NCJ_DICT,&jgroup)))
 	goto done;
     snprintf(version,sizeof(version),"%d",zinfo->zarr.zarr_version);
     if((stat = NCJaddstring(jgroup,NCJ_STRING,"zarr_format"))) goto done;
     if((stat = NCJaddstring(jgroup,NCJ_INT,version))) goto done;
-    if(grp->parent == NULL) { /* Root group */
+    if(!purezarr && grp->parent == NULL) { /* Root group */
         snprintf(version,sizeof(version),"%lu.%lu.%lu",
 		 zinfo->zarr.nczarr_version.major,
 		 zinfo->zarr.nczarr_version.minor,
@@ -185,9 +189,11 @@ ncz_sync_grp(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp)
 	jsuper = NULL;
     }
 
-    /* Insert the "_NCZARR_GROUP" dict */    
-    if((stat = NCJinsert(jgroup,NCZ_V2_GROUP,json))) goto done;
-    json = NULL;
+    if(!purezarr) {
+        /* Insert the "_NCZARR_GROUP" dict */    
+        if((stat = NCJinsert(jgroup,NCZ_V2_GROUP,json))) goto done;
+        json = NULL;
+    }
 
     /* build ZGROUP path */
     if((stat = nczm_concat(fullpath,ZGROUP,&key)))
@@ -329,7 +335,9 @@ ncz_sync_var(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var)
     jtmp = NULL;
 
     /* fill_value key */
-    if(!var->no_fill) {
+    if(var->no_fill) {
+	if((stat=NCJnew(NCJ_NULL,&jfill))) goto done;
+    } else {/*!var->no_fill*/
 	int fillsort;
 	int atomictype = var->type_info->hdr.id;
 	/* A scalar value providing the default value to use for uninitialized
@@ -354,9 +362,9 @@ ncz_sync_var(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var)
 	    jfill = jtmp;
 	    jtmp = NULL;
         }
-        if((stat = NCJinsert(jvar,"fill_value",jfill))) goto done;
-        jfill = NULL;
     }
+    if((stat = NCJinsert(jvar,"fill_value",jfill))) goto done;
+    jfill = NULL;
 
     /* order key */
     if((stat = NCJaddstring(jvar,NCJ_STRING,"order"))) goto done;
@@ -577,6 +585,7 @@ ncz_sync_atts(NC_FILE_INFO_T* file, NC_OBJ* container, NCindex* attlist)
     char* content = NULL;
     char* dimpath = NULL;
     int isxarray = 0;
+    int isrootgroup = 0;
 
     LOG((3, "%s", __func__));
 
@@ -585,6 +594,12 @@ ncz_sync_atts(NC_FILE_INFO_T* file, NC_OBJ* container, NCindex* attlist)
 
     if(zinfo->controls.flags & FLAG_XARRAYDIMS) isxarray = 1;
 
+    if(container->sort == NCVAR) {
+	NC_VAR_INFO_T* var = (NC_VAR_INFO_T*)container;
+	if(var->container && var->container->parent == NULL)
+	    isrootgroup = 1;
+    }
+
     if(!isxarray && ncindexsize(attlist) == 0) 
 	goto done; /* do nothing */
 
@@ -611,7 +626,7 @@ ncz_sync_atts(NC_FILE_INFO_T* file, NC_OBJ* container, NCindex* attlist)
     }
 
     /* Construct container path */
-    if(NCJsort(container) == NCGRP)
+    if(container->sort == NCGRP)
 	stat = NCZ_grpkey((NC_GRP_INFO_T*)container,&fullpath);
     else
 	stat = NCZ_varkey((NC_VAR_INFO_T*)container,&fullpath);
@@ -622,8 +637,8 @@ ncz_sync_atts(NC_FILE_INFO_T* file, NC_OBJ* container, NCindex* attlist)
     if((stat = ncz_jsonize_atts(attlist,&jatts)))
 	goto done;
 
-    if(NCJsort(container) == NCVAR) {
-        if(isxarray) {
+    if(container->sort == NCVAR) {
+        if(isrootgroup && isxarray) {
 	    NC_VAR_INFO_T* var = (NC_VAR_INFO_T*)container;
 	    /* Insert the XARRAY _ARRAY_ATTRIBUTE attribute */
 	    if((stat = NCJnew(NCJ_ARRAY,&jdimrefs)))
@@ -740,7 +755,7 @@ load_jatts(NCZMAP* map, NC_OBJ* container, int nczarrv1, NCjson** jattrsp, NClis
     /* alway return (possibly empty) list of types */
     atypes = nclistnew();
 
-    if(NCJsort(container) == NCGRP) {
+    if(container->sort == NCGRP) {
 	NC_GRP_INFO_T* grp = (NC_GRP_INFO_T*)container;
 	/* Get grp's fullpath name */
 	if((stat = NCZ_grpkey(grp,&fullpath)))
@@ -1184,7 +1199,7 @@ ncz_read_atts(NC_FILE_INFO_T* file, NC_OBJ* container)
     zinfo = file->format_file_info;
     map = zinfo->map;
 
-    if(NCJsort(container) == NCGRP)
+    if(container->sort == NCGRP)
 	attlist = ((NC_GRP_INFO_T*)container)->att;
     else
 	attlist = ((NC_VAR_INFO_T*)container)->att;
@@ -1210,7 +1225,7 @@ ncz_read_atts(NC_FILE_INFO_T* file, NC_OBJ* container)
 	    if(ra != NULL) {
 		/* case 1: name = _NCProperties, grp=root, varid==NC_GLOBAL, flags & READONLYFLAG */
 		if(strcmp(NCJstring(key),NCPROPS)==0
-		   && NCJsort(container) == NCGRP
+		   && container->sort == NCGRP
 		   && file->root_grp == (NC_GRP_INFO_T*)container) {
 		    /* Setup provenance */
 		    if(NCJsort(value) != NCJ_STRING)
@@ -1220,7 +1235,7 @@ ncz_read_atts(NC_FILE_INFO_T* file, NC_OBJ* container)
 		}
 		/* case 2: name = _ARRAY_DIMENSIONS, sort==NCVAR, flags & HIDDENATTRFLAG */
 		if(strcmp(NCJstring(key),NC_XARRAY_DIMS)==0
-		   && NCJsort(container) == NCVAR
+		   && container->sort == NCVAR
 		   && (ra->flags & HIDDENATTRFLAG)) {
 		       /* store for later */
 		    NCZ_VAR_INFO_T* zvar = (NCZ_VAR_INFO_T*)((NC_VAR_INFO_T*)container)->format_var_info;
@@ -1249,13 +1264,13 @@ ncz_read_atts(NC_FILE_INFO_T* file, NC_OBJ* container)
 	}
     }
     /* If we have not read a _FillValue, then go ahead and create it */
-    if(fillvalueatt == NULL && NCJsort(container) == NCVAR) {
+    if(fillvalueatt == NULL && container->sort == NCVAR) {
 	if((stat = ncz_create_fillvalue((NC_VAR_INFO_T*)container)))
 	    goto done;
     }
 
     /* Remember that we have read the atts for this var or group. */
-    if(NCJsort(container) == NCVAR)
+    if(container->sort == NCVAR)
 	((NC_VAR_INFO_T*)container)->atts_read = 1;
     else
 	((NC_GRP_INFO_T*)container)->atts_read = 1;
@@ -1399,6 +1414,8 @@ define_vars(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp, NClist* varnames)
 		if((stat = ncz_gettype(file,grp,vtype,&var->type_info)))
 		    goto done;
 	    } else {stat = NC_EBADTYPE; goto done;}
+	    if(endianness == NC_ENDIAN_NATIVE)
+		endianness = zinfo->native_endianness;
 	    if(endianness == NC_ENDIAN_LITTLE || endianness == NC_ENDIAN_BIG) {
 		var->endianness = endianness;
 	    } else {stat = NC_EBADTYPE; goto done;}
@@ -1466,7 +1483,7 @@ define_vars(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp, NClist* varnames)
 	/* fill_value */
 	{
 	    if((stat = NCJdictget(jvar,"fill_value",&jvalue))) goto done;
-	    if(jvalue == NULL)
+	    if(jvalue == NULL || NCJsort(jvalue) == NCJ_NULL)
 		var->no_fill = 1;
 	    else {
 		size_t fvlen;
@@ -1511,6 +1528,7 @@ define_vars(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp, NClist* varnames)
 		 /* Extract the NCZ_V2_ARRAY dict */
 	        if((stat = NCJdictget(jvar,NCZ_V2_ARRAY,&jncvar))) goto done;
 	    }
+	    if(jncvar == NULL) {stat = NC_ENCZARR; goto done;}
    	    assert((NCJsort(jncvar) == NCJ_DICT));
 	    /* Extract storage flag */
 	    if((stat = NCJdictget(jncvar,"storage",&jvalue)))

diff --git a/libnczarr/zutil.c b/libnczarr/zutil.c
@@ -514,6 +514,7 @@ ncz_dtype2typeinfo(const char* dtype, nc_type* nctypep, int* endianp)
     switch (dtype[0]) {
     case '<': endianness = NC_ENDIAN_LITTLE; break;
     case '>': endianness = NC_ENDIAN_BIG; break;
+    case '|': endianness = NC_ENDIAN_NATIVE; break;
     default: goto zerr;
     }
     /* Decode the type length */

diff --git a/nczarr_test/CMakeLists.txt b/nczarr_test/CMakeLists.txt
@@ -78,7 +78,7 @@ IF(ENABLE_TESTS)
     add_sh_test(nczarr_test run_ut_misc)
     add_sh_test(nczarr_test run_ut_chunk)
     IF(USE_HDF5)
-    add_sh_test(nczarr_test run_nccopyz)
+#    add_sh_test(nczarr_test run_nccopyz)
     add_sh_test(nczarr_test run_fillonlyz)
     ENDIF()
     add_sh_test(nczarr_test run_ncgen4)
@@ -90,6 +90,7 @@ IF(ENABLE_TESTS)
     add_sh_test(nczarr_test run_purezarr)
     add_sh_test(nczarr_test run_interop)
     add_sh_test(nczarr_test run_misc)
+    add_sh_test(nczarr_test run_nczarr_fill)
 
     if(ENABLE_NCZARR_S3)
 	add_sh_test(nczarr_test run_s3_cleanup)

diff --git a/nczarr_test/Makefile.am b/nczarr_test/Makefile.am
@@ -47,7 +47,7 @@ if BUILD_UTILITIES
 TESTS += run_ncgen4.sh
 
 if USE_HDF5
-TESTS += run_nccopyz.sh
+#TESTS += run_nccopyz.sh
 TESTS += run_fillonlyz.sh
 endif
 
@@ -58,6 +58,7 @@ TESTS += run_chunkcases.sh
 TESTS += run_purezarr.sh
 TESTS += run_interop.sh
 TESTS += run_misc.sh
+TESTS += run_nczarr_fill.sh
 
 endif
 
@@ -108,7 +109,7 @@ ncdumpchunks_SOURCES = ncdumpchunks.c
 EXTRA_DIST = CMakeLists.txt \
 run_ut_map.sh run_ut_mapapi.sh run_ut_misc.sh run_ut_chunk.sh run_ncgen4.sh \
 run_nccopyz.sh run_fillonlyz.sh run_chunkcases.sh test_nczarr.sh run_perf_chunks1.sh run_s3_cleanup.sh \
-run_purezarr.sh run_interop.sh run_misc.sh run_newformat.sh
+run_purezarr.sh run_interop.sh run_misc.sh run_newformat.sh run_nczarr_fill.sh
 
 EXTRA_DIST += \
 ref_ut_map_create.cdl ref_ut_map_writedata.cdl ref_ut_map_writemeta2.cdl ref_ut_map_writemeta.cdl \
@@ -125,7 +126,9 @@ ref_misc1.cdl ref_misc1.dmp ref_misc2.cdl \
 ref_avail1.cdl ref_avail1.dmp ref_avail1.txt \
 ref_xarray.cdl ref_purezarr.cdl ref_purezarr_base.cdl ref_nczarr2zarr.cdl \
 ref_power_901_constants.zip ref_power_901_constants.cdl ref_quotes.zip ref_quotes.cdl \
-ref_oldformat.cdl ref_oldformat.zip ref_newformatpure.cdl
+ref_oldformat.cdl ref_oldformat.zip ref_newformatpure.cdl \
+ref_groups.h5 ref_byte.zarr.zip ref_byte_fill_value_null.zarr.zip \
+ref_groups_regular.cdl ref_byte.cdl ref_byte_fill_value_null.cdl
 
 CLEANFILES = ut_*.txt ut*.cdl tmp*.nc tmp*.cdl tmp*.txt tmp*.dmp tmp*.zip tmp*.nc
 

diff --git a/nczarr_test/ref_byte.cdl b/nczarr_test/ref_byte.cdl
@@ -0,0 +1,34 @@
+netcdf ref_byte {
+dimensions:
+	.zdim_20 = 20 ;
+variables:
+	ubyte byte(.zdim_20, .zdim_20) ;
+		byte:_Storage = "chunked" ;
+		byte:_ChunkSizes = 20, 20 ;
+
+// global attributes:
+		:_Format = "netCDF-4" ;
+data:
+
+ byte =
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ;
+}
diff --git a/nczarr_test/ref_byte.zarr.zip b/nczarr_test/ref_byte.zarr.zip