Merge pull request #2730 from DennisHeimbigner/filtervlen2.dmh

Explicitly suppress variable length type compression

libdispatch/dfilter.c

@@ -126,19 +126,9 @@ nc_def_var_filter(int ncid, int varid, unsigned int id, size_t nparams, const un
 {
     int stat = NC_NOERR;
     NC* ncp;
-    int fixedsize;
-    nc_type xtype;
 
     TRACE(nc_inq_var_filter);
     if((stat = NC_check_id(ncid,&ncp))) return stat;
-    /* Get variable' type */
-    if((stat = nc_inq_vartype(ncid,varid,&xtype))) return stat;
-    /* If the variable's type is not fixed-size, then signal error */
-    if((stat = NC4_inq_type_fixed_size(ncid, xtype, &fixedsize))) return stat;
-    if(!fixedsize) {
-	nclog(NCLOGWARN,"Filters cannot be applied to variable length data types.");
-        return NC_NOERR; /* Deliberately suppress */
-    }
     if((stat = ncp->dispatch->def_var_filter(ncid,varid,id,nparams,params))) goto done;
 done:
     return stat;

libhdf5/nc4hdf.c

@@ -914,11 +914,7 @@ var_create_dataset(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var, nc_bool_t write_dimid
                     BAIL(NC_EFILTER);
             } else {
                 herr_t code = H5Pset_filter(plistid, fi->filterid,
-#if 1
-                                            H5Z_FLAG_MANDATORY,
-#else
-                                            H5Z_FLAG_OPTIONAL,
-#endif
+                                            H5Z_FLAG_OPTIONAL, /* always make optional so filters on vlens are ignored */
                                            fi->nparams, fi->params);
 		if(code < 0)
                     BAIL(NC_EFILTER);

libnczarr/zfilter.c

@@ -130,12 +130,13 @@ ncz_hdf5_clear(NCZ_HDF5* h) {
 
 typedef struct NCZ_Filter {
     int flags;             	/**< Flags describing state of this filter. */
-#	define FLAG_VISIBLE 1 /* If set, then visible parameters are defined */
-#	define FLAG_WORKING 2 /* If set, then WORKING parameters are defined */
-#	define FLAG_CODEC 4 /* If set, then visbile parameters come from an existing codec string */
-#	define FLAG_HDF5  8 /* If set, => visible parameters came from nc_def_var_filter */
-#	define FLAG_NEWVISIBLE  16 /* If set, => visible parameters  were modified */
-#	define FLAG_INCOMPLETE  32 /* If set, => filter has no complete matching plugin */
+#	define FLAG_VISIBLE	1 /* If set, then visible parameters are defined */
+#	define FLAG_WORKING	2 /* If set, then WORKING parameters are defined */
+#	define FLAG_CODEC	4 /* If set, then visbile parameters come from an existing codec string */
+#	define FLAG_HDF5	8 /* If set, => visible parameters came from nc_def_var_filter */
+#	define FLAG_NEWVISIBLE	16 /* If set, => visible parameters  were modified */
+#	define FLAG_INCOMPLETE	32 /* If set, => filter has no complete matching plugin */
+#	define FLAG_SUPPRESS	64 /* If set, => filter should not be used (probably because variable is not fixed size */
     NCZ_HDF5 hdf5;
     NCZ_Codec codec;
     struct NCZ_Plugin* plugin;  /**< Implementation of this filter. */
@@ -389,6 +390,12 @@ NCZ_addfilter(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var, unsigned int id, size_t
 	    nclistpush((NClist*)var->filters, fi);
     }
 
+    /* If this variable is not fixed size, mark filter as suppressed */
+    if(var->type_info->varsized) {
+	fi->flags |= FLAG_SUPPRESS;
+	nclog(NCLOGWARN,"Filters cannot be applied to variable length data types; ignored");
+    }
+
     if(!FILTERINCOMPLETE(fi)) {
         /* (over)write the HDF5 parameters */
         nullfree(fi->hdf5.visible.params);
@@ -870,6 +877,7 @@ fprintf(stderr,">>> current: alloc=%u used=%u buf=%p\n",(unsigned)current_alloc,
         if(encode) {
             for(i=0;i<nclistlength(chain);i++) {
 	        f = (struct NCZ_Filter*)nclistget(chain,i);	
+		if(f->flags & FLAG_SUPPRESS) continue; /* this filter should not be applied */		
 	        ff = f->plugin->hdf5.filter;
 	        /* code can be simplified */
 	        next_alloc = current_alloc;
@@ -889,6 +897,7 @@ fprintf(stderr,">>> next: alloc=%u used=%u buf=%p\n",(unsigned)next_alloc,(unsig
 	    /* Apply in reverse order */
             for(i=nclistlength(chain)-1;i>=0;i--) {
 	        f = (struct NCZ_Filter*)nclistget(chain,i);	
+		if(f->flags & FLAG_SUPPRESS) continue; /* this filter should not be applied */
 	        ff = f->plugin->hdf5.filter;
 	        /* code can be simplified */
 	        next_alloc = current_alloc;

nc_test4/Makefile.am

@@ -81,7 +81,7 @@ endif
 if USE_HDF5
 if ENABLE_FILTER_TESTING
 extradir =
-check_PROGRAMS += test_filter test_filter_misc test_filter_order test_filter_repeat test_filter_vlen
+check_PROGRAMS += test_filter test_filter_misc test_filter_order test_filter_repeat
 check_PROGRAMS += tst_multifilter tst_filter_vlen
 TESTS += tst_filter.sh
 TESTS += tst_specific_filters.sh

nc_test4/tst_filter_vlen.c

@@ -17,11 +17,7 @@
 
 #undef DEBUG
 
-#ifndef H5Z_FILTER_BZIP2
-#define H5Z_FILTER_BZIP2      307
-#endif
-
-#define TEST_ID 32768
+#define FILTER_ID 1 /*deflate*/
 
 #define MAXERRS 8
 
@@ -51,8 +47,7 @@ static int nerrs = 0;
 
 static int ncid, varid;
 static int dimids[MAXDIMS];
-static float* array = NULL;
-static float* expected = NULL;
+static char** array = NULL;
 
 /* Forward */
 static int test_test1(void);
@@ -100,26 +95,142 @@ defvar(nc_type xtype)
     return NC_NOERR;
 }
 
+static int
+reopen(void)
+{
+    size_t i;
+
+    CHECK(nc_open(testfile, NC_NETCDF4, &ncid));
+    for(i=0;i<ndims;i++) {
+        char dimname[1024];
+        snprintf(dimname,sizeof(dimname),"dim%u",(unsigned)i);
+        CHECK(nc_inq_dimid(ncid, dimname, &dimids[i]));
+        CHECK(nc_inq_dim(ncid, dimids[i], NULL, &dimsize[i]));
+    }
+    CHECK(nc_inq_varid(ncid, "var", &varid));
+    return NC_NOERR;
+}
+
+
+
+/* Test that a filter is a variable length var is defined */
 static int
 test_test1(void)
 {
     int ok = 1;
-    int id = -1;
-    size_t nparams;    
+    size_t nfilters = 0;
+    unsigned filterids[64];
+    unsigned params[NPARAMS] = {5};
+    size_t nparams = 0;
 
-    reset();
     fprintf(stderr,"test4: filter on a variable length type.\n");
     create();
     defvar(NC_STRING);
     /* Do explicit filter; should never fail, but may produce log warning */
-    CHECK(nc_def_var_filter(ncid,varid,H5Z_FILTER_BZIP2,0,NULL));
+    CHECK(nc_def_var_filter(ncid,varid,FILTER_ID,1,params));
     /* Now see if filter was defined or not */
-    CHECK(nc_inq_var_filter(ncid,varid,&id,&nparams,NULL));
-    if(id > 0) {
-	fprintf(stderr,"*** id=%d\n",id);
+    memset(filterids,0,sizeof(filterids));
+    params[0] = 5;
+    CHECK(nc_inq_var_filter_ids(ncid,varid,&nfilters,filterids));
+    fprintf(stderr,"test_test1: nc_var_filter_ids: nfilters=%u filterids[0]=%d\n",(unsigned)nfilters,filterids[0]);
+    if(nfilters != 1 && filterids[0] != FILTER_ID) {
+	fprintf(stderr,"test_test1: nc_var_filter_ids: failed\n");
 	ok = 0;
     }
-    CHECK(nc_abort(ncid));
+    params[0] = 0;
+    CHECK(nc_inq_var_filter_info(ncid, varid, filterids[0], &nparams, params));
+    fprintf(stderr,"test_test1: nc_inq_var_filter_info: nparams=%u params[0]=%u\n",(unsigned)nparams,(unsigned)params[0]);
+    return ok;
+}
+
+/* Test that a filter on a variable length var is suppressed */
+static int
+test_test2(void)
+{
+    int stat = NC_NOERR;
+    int ok = 1;
+    size_t i;
+
+    reset();
+    fprintf(stderr,"test4: write with filter on a variable length type.\n");
+    /* generate the data to write */
+    for(i=0;i<actualproduct;i++) {
+        char digits[64];
+	snprintf(digits,sizeof(digits),"%u",(unsigned)i);
+	array[i] = strdup(digits);
+    }
+    /* write the data */
+    if((stat=nc_put_var(ncid,varid,(void*)array))) {
+	fprintf(stderr,"test_test2: nc_put_var: error = (%d)%s\n",stat,nc_strerror(stat));
+	ok = 0;
+	goto done;
+    }
+    /* re-read the data */
+    reset();
+    if((stat=nc_get_var(ncid,varid,(void*)array))) {
+	fprintf(stderr,"test_test2: nc_get_var: error = (%d)%s\n",stat,nc_strerror(stat));
+	ok = 0;
+	goto done;
+    }
+    /* verify the data */    
+    for(i=0;i<actualproduct;i++) {
+	unsigned value = 0xffffffff;
+	if(array[i] != NULL)
+	    sscanf(array[i],"%u",&value);
+	if(array[i] == NULL || i != value) {
+	    fprintf(stderr,"test_test2: nc_get_var: value mismatch at %u\n",(unsigned)i);
+	    ok = 0;
+	    goto done;
+	}
+    }
+    nc_close(ncid);
+done:
+    return ok;
+}
+
+/* Test that a filter on a variable length var is suppressed */
+static int
+test_test3(void)
+{
+    int stat = NC_NOERR;
+    int ok = 1;
+    size_t i,nfilters;
+    unsigned filterids[64];
+
+    fprintf(stderr,"test4: re-open variable with filter on a variable length type and verify state.\n");
+
+    reopen();    
+
+    /* verify filter state */
+    memset(filterids,0,sizeof(filterids));
+    CHECK(nc_inq_var_filter_ids(ncid,varid,&nfilters,filterids));
+    fprintf(stderr,"test_test3: nc_var_filter_ids: nfilters=%u filterids[0]=%d\n",(unsigned)nfilters,filterids[0]);
+    if(nfilters != 1 && filterids[0] != FILTER_ID) {
+	fprintf(stderr,"test_test3: nc_var_filter_ids: failed\n");
+	ok = 0;
+	goto done;
+    }
+
+    /* re-read the data */
+    reset();
+    if((stat=nc_get_var(ncid,varid,(void*)array))) {
+	fprintf(stderr,"test_test3: nc_get_var: error = (%d)%s\n",stat,nc_strerror(stat));
+	ok = 0;
+	goto done;
+    }
+    /* verify the data */    
+    for(i=0;i<actualproduct;i++) {
+	unsigned value = 0xffffffff;
+	if(array[i] != NULL)
+	    sscanf(array[i],"%u",&value);
+	if(array[i] == NULL || i != value) {
+	    fprintf(stderr,"test_test3: nc_get_var: value mismatch at %u\n",(unsigned)i);
+	    ok = 0;
+	    goto done;
+	}
+    }
+    nc_close(ncid);
+done:
     return ok;
 }
 
@@ -129,7 +240,11 @@ test_test1(void)
 static void
 reset()
 {
-    memset(array,0,sizeof(float)*actualproduct);
+    size_t i;
+    for(i=0;i<actualproduct;i++) {
+        if(array[i]) free(array[i]);
+	array[i] = NULL;
+    }
 }
 
 static void
@@ -153,8 +268,7 @@ init(int argc, char** argv)
         }
     }
     /* Allocate max size */
-    array = (float*)calloc(1,sizeof(float)*actualproduct);
-    expected = (float*)calloc(1,sizeof(float)*actualproduct);
+    array = (char**)calloc(1,sizeof(char*)*actualproduct);
 }
 
 /**************************************************/
@@ -167,6 +281,8 @@ main(int argc, char **argv)
 #endif
     init(argc,argv);
     if(!test_test1()) ERRR;
+    if(!test_test2()) ERRR;
+    if(!test_test3()) ERRR;
     fprintf(stderr,"*** %s\n",(nerrs > 0 ? "FAILED" : "PASS"));
     exit(nerrs > 0?1:0);
 }

nc_test4/tst_filter_vlen.sh

@@ -1,8 +1,6 @@
 #!/bin/bash 
 
-# Test the filter install
-# This cannot be run as a regular test
-# because installation will not have occurred
+# Test filters on non-fixed size variables.
 
 if test "x$srcdir" = x ; then srcdir=`pwd`; fi
 . ../test_common.sh
@@ -95,6 +93,7 @@ if test "x$TESTNCZARR" = x1 ; then
     testset file
     if test "x$FEATURE_NCZARR_ZIP" = xyes ; then testset zip ; fi
     if test "x$FEATURE_S3TESTS" = xyes ; then testset s3 ; fi
+    if test "x$FEATURE_S3TESTS" = xyes ; then s3sdkdelete "/${S3ISOPATH}" ; fi # Cleanup
 else
     testset nc
 fi