Apply filters to variable-length data by crusaderky · Pull Request #6178 · HDFGroup/hdf5

crusaderky · 2026-01-28T16:54:25Z

Closes can_apply and set_local filter callbacks skipped for H5T_VARIABLE dtypes #5942. A more detailed explanation of the rationale for this PR is in the linked issue.
Mandatory filters can be now applied to variable-length data. This notably includes h5py object strings and NpyStrings.
Optional filters applied to variable-length data no longer skip can_apply and set_local, only to jump directly to filter. This fixes hdf5_blosc and HDF5_Blosc2, which rely on set_local. Note that all filters which have neither can_apply nor set_local have always worked fine with vlen data, as long as they were tagged as optional. Notable examples are deflate, lzf, bzip2 (from hdf5plugin), and lz4 (also from hdf5plugin).
Lossy filters, such as scale-offset, are and remain untested and most likely broken, as I expect them to treat the vlen metadata as numbers and corrupt it. The right place to ensure proper behaviour is their can_apply function, which however is broken (filters: can_apply does nothing for optional filters #6161).

Important

Enhance filter application to variable-length data, fixing optional filter handling and adding tests.

Behavior:
- Mandatory filters can now be applied to variable-length data, including h5py strings.
- Optional filters no longer skip can_apply and set_local steps, fixing issues with hdf5_blosc and HDF5_Blosc2.
- Lossy filters remain untested and potentially broken for vlen data.
Code Changes:
- Modify H5Z_ignore_filters() in H5Z.c to handle optional filters correctly.
- Add tests in dsets.c for scalar and null datasets with optional filters, and for set_local updates with vlen data.
Tests:
- Add test_optional_filters_scalar, test_optional_filters_null, test_set_local_updates_cd, test_set_local_updates_cd_vlen, and test_deflate_vlen in dsets.c.

^{This description was created by}^{for 70ab78a. You can customize this summary. It will automatically update as commits are pushed.}

crusaderky · 2026-01-28T16:58:53Z

test/dsets.c

+} /* end test_optional_filters_scalar() */
+
+/*-------------------------------------------------------------------------
+ * Function:    test_optional_filters_null


this was previously untested.

crusaderky · 2026-01-28T17:00:21Z

test/dsets.c

+} /* end test_set_local_updates_cd() */
+
+/*-------------------------------------------------------------------------
+ * Function:    test_set_local_updates_cd_vlen


This test fails in the main branch, complaining that filters can't be applied to vlen data.
If I replace H5Z_FLAG_MANDATORY with H5Z_FLAG_OPTIONAL below, it crashes on assert(0) inside the filter callback, highlighting that set_local has not been called.

crusaderky · 2026-01-28T17:01:41Z

test/dsets.c

+} /* end test_set_local_updates_cd_vlen() */
+
+/*-------------------------------------------------------------------------
+ * Function:    test_deflate_vlen


This tests what happens when filter actually processes the contiguous raw chunk data.

This test fails in the main branch, complaining that filters can't be applied to vlen data.
However, if I replace H5Z_FLAG_MANDATORY with H5Z_FLAG_OPTIONAL below, it passes in the main branch, thanks to the fact that the deflate filter has no set_local callback.

Running deflate on more substantial data shows that the output file size is much smaller than the uncompressed version.

crusaderky · 2026-01-28T18:10:15Z

Downstream tests in hdf5-blosc: Blosc/hdf5-blosc#38

crusaderky · 2026-01-28T18:20:10Z

test/dsets.c


-    /* Create dataset with optional filter */
-    if ((dsid = H5Dcreate2(file, DSET_OPTIONAL_SCALAR, strtid, sid, H5P_DEFAULT, dcplid, H5P_DEFAULT)) < 0)
+    /* Write data to the dataset */


This checks that the filter callback is never invoked, which the previous version of this test left untested.

crusaderky · 2026-01-29T12:07:27Z

@hyoklee is it possible to avoid having CI re-authorised by a maintainer every time I need to fix a trivial error? This is hurting velocity quite badly.

fortnern · 2026-02-13T19:07:05Z

test/dsets.c

-    /* Create the datatype */
-    if ((strtid = H5Tcreate(H5T_STRING, H5T_VARIABLE)) < 0)
+    /* The filter is optional. Callbacks will crash if ever invoked. */
+    if (H5Pset_filter(dcplid, H5Z_FILTER_CRASH, H5Z_FLAG_OPTIONAL, 0, NULL) < 0)


Where is this filter registered?

It wasn't. I think the test worked because hdf5 skips the whole filter stack due to the data type of the dataset. This highlights that H5Pset_filter is lacking input validation (out of scope for this PR).

fortnern

Library changes look good, some comments about the tests

fortnern · 2026-02-13T19:12:40Z

I've confirmed that disabling can_apply and set_local for vlens was a mistake

fortnern · 2026-02-13T20:44:38Z

It makes sense you might want to compress a compound containing variable length data as well as fixed length data.

Absolutely! I would however leave it as a follow-up.

This should actually work fine with just this change. We can write a test for it at some point if it's not already tested (it's possible the test passes because it uses a filter that doesn't use can_apply or set_local).

crusaderky · 2026-02-23T12:05:45Z

It makes sense you might want to compress a compound containing variable length data as well as fixed length data.

Absolutely! I would however leave it as a follow-up.

This should actually work fine with just this change. We can write a test for it at some point if it's not already tested (it's possible the test passes because it uses a filter that doesn't use can_apply or set_local).

My demo above does not change its output after this PR, so the bit that feeds the actual data through the filter is missing.

crusaderky · 2026-02-23T12:35:29Z

@fortnern all comments have been addressed. This is ready for another round of review.

src/H5Z.c

crusaderky · 2026-02-23T23:07:24Z

@fortnern I applied the nit you suggested but it removed your approval

fortnern · 2026-02-24T16:39:09Z

src/H5Z.c

 */
 htri_t
-H5Z_ignore_filters(hid_t dcpl_id, const H5T_t *type, const H5S_t *space)
+H5Z_ignore_filters(hid_t dcpl_id, void H5_ATTR_UNUSED *type, const H5S_t *space)


Suggested change

H5Z_ignore_filters(hid_t dcpl_id, void H5_ATTR_UNUSED *type, const H5S_t *space)

H5Z_ignore_filters(hid_t dcpl_id, const H5S_t *space)

You will also need to change the prototype declaration in H5Zprivate.h

And the invocation in H5D__create()

I went ahead and pushed the changes myself, I hope that's ok. We're trying to get this in the release.

Closes can_apply and set_local filter callbacks skipped for H5T_VARIABLE dtypes HDFGroup#5942. A more detailed explanation of the rationale for this PR is in the linked issue. Mandatory filters can be now applied to variable-length data. This notably includes h5py object strings and NpyStrings. Optional filters applied to variable-length data no longer skip can_apply and set_local, only to jump directly to filter. This fixes hdf5_blosc and HDF5_Blosc2, which rely on set_local. Note that all filters which have neither can_apply nor set_local have always worked fine with vlen data, as long as they were tagged as optional. Notable examples are deflate, lzf, bzip2 (from hdf5plugin), and lz4 (also from hdf5plugin). Lossy filters, such as scale-offset, are and remain untested and most likely broken, as I expect them to treat the vlen metadata as numbers and corrupt it. The right place to ensure proper behaviour is their can_apply function, which however is broken (filters: can_apply does nothing for optional filters HDFGroup#6161).

github-project-automation bot added this to HDF5 - TRIAGE & TRACK Jan 28, 2026

github-project-automation bot moved this to To be triaged in HDF5 - TRIAGE & TRACK Jan 28, 2026

crusaderky commented Jan 28, 2026

View reviewed changes

crusaderky marked this pull request as ready for review January 28, 2026 17:06

crusaderky requested review from bmribler, brtnfld, derobins, fortnern, glennsong09, jhendersonHDF, lrknox, mattjala, qkoziol and vchoi-hdfgroup as code owners January 28, 2026 17:06

Apply filters to variable-length data

a56f335

crusaderky force-pushed the ignore_filters branch from 70ab78a to a56f335 Compare January 28, 2026 17:10

crusaderky added a commit to crusaderky/hdf5-pixi that referenced this pull request Jan 28, 2026

Build vs. HDFGroup/hdf5#6178

04c9101

Remove unused variable

ae74972

crusaderky added a commit to crusaderky/hdf5-blosc that referenced this pull request Jan 28, 2026

Demo HDFGroup/hdf5#6178

233849d

crusaderky added a commit to crusaderky/hdf5-blosc that referenced this pull request Jan 28, 2026

Demo HDFGroup/hdf5#6178

4b685fa

crusaderky mentioned this pull request Jan 28, 2026

DNM Demonstrate segfault with variable-width strings Blosc/hdf5-blosc#38

Draft

crusaderky commented Jan 28, 2026

View reviewed changes

hyoklee assigned fortnern Jan 28, 2026

hyoklee added the Component - C Library Core C library issues (usually in the src directory) label Jan 28, 2026

Fix compile error

859015a

crusaderky added a commit to crusaderky/hdf5-pixi that referenced this pull request Jan 29, 2026

Build vs. HDFGroup/hdf5#6178

c861800

fortnern reviewed Feb 13, 2026

View reviewed changes

fortnern requested changes Feb 13, 2026

View reviewed changes

vchoi-hdfgroup modified the milestones: HDF5 2.1.0, HDF5 2.2.0 Feb 20, 2026

crusaderky added 4 commits February 23, 2026 08:21

Merge remote-tracking branch 'upstream/develop' into HEAD

8ccbee4

Replace asserts with a global variable

915c833

Replace more asserts

0124084

register filter

d667bd7

Reinstate old test

0cfa667

fortnern reviewed Feb 23, 2026

View reviewed changes

src/H5Z.c Outdated Show resolved Hide resolved

fortnern previously approved these changes Feb 23, 2026

View reviewed changes

crusaderky commented Feb 23, 2026

View reviewed changes

src/H5Z.c Outdated Show resolved Hide resolved

Apply suggestion from @crusaderky

00dd5e6

crusaderky dismissed fortnern’s stale review via 00dd5e6 February 23, 2026 23:06

fortnern reviewed Feb 24, 2026

View reviewed changes

Remove type parameter from H5Z_ignore_filters

68bbcf3

fortnern approved these changes Feb 24, 2026

View reviewed changes

nbagha1 modified the milestones: HDF5 2.2.0, HDF5 2.1.0 Feb 24, 2026

jhendersonHDF approved these changes Feb 24, 2026

View reviewed changes

lrknox merged commit b074586 into HDFGroup:develop Feb 24, 2026
238 of 239 checks passed

github-project-automation bot moved this from In progress to Done in HDF5 - TRIAGE & TRACK Feb 24, 2026

fortnern added a commit to fortnern/hdf5 that referenced this pull request Feb 25, 2026

Add note to CHANGELOG.md for PR HDFGroup#6178

1643eab

fortnern added a commit that referenced this pull request Feb 25, 2026

Add note to CHANGELOG.md for PR #6178 (#6239)

c909e9a

	H5Z_ignore_filters(hid_t dcpl_id, void H5_ATTR_UNUSED type, const H5S_t space)
	H5Z_ignore_filters(hid_t dcpl_id, const H5S_t *space)

Uh oh!

Comments

Conversation

crusaderky commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crusaderky Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

crusaderky Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crusaderky Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crusaderky commented Jan 28, 2026

Uh oh!

crusaderky Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crusaderky commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fortnern Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

crusaderky Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

fortnern left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fortnern commented Feb 13, 2026

Uh oh!

fortnern commented Feb 13, 2026

Uh oh!

crusaderky commented Feb 23, 2026

Uh oh!

crusaderky commented Feb 23, 2026

Uh oh!

Uh oh!

Uh oh!

crusaderky commented Feb 23, 2026

Uh oh!

fortnern Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

fortnern Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

fortnern Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

crusaderky commented Jan 28, 2026 •

edited

Loading

crusaderky Jan 28, 2026 •

edited

Loading

crusaderky Jan 28, 2026 •

edited

Loading

crusaderky Jan 28, 2026 •

edited

Loading

crusaderky commented Jan 29, 2026 •

edited

Loading

fortnern left a comment •

edited

Loading