Re: [PATCH v2 0/1] MR2498: msado15: Implement _Recordset Find

[Zebediah Figura (＠zfigura)]

Zebediah Figura (@zfigura) commented about dlls/msado15/recordset.c:
> - if (!recordset->criteria || wcscmp(recordset->criteria, criteria) != 0)
> - {
> -
>    clear_recordset_filter(recordset);
> -
> -
>    recordset->criteria = SysAllocString(criteria);
> -
> -
>    /* Parse criteria */
> -
>    recordset_parse_filter(criteria, recordset);
> - }
> -
> - if (list_count(&recordset->filters) != 1)
> - {
> -
>    WARN("Invalid number of filters.\n");
> -
>    clear_recordset_filter(recordset);
> -
>    return MAKE_ADO_HRESULT( adErrDataConversion );
> - }

This bothers me. As noted above, currently the code can return multiple filters, but it doesn't look right. I imagine we may want to remove that code (in the initial version), in which case the current code will never return more than one filter, and this should really just be an assert.

More than that, though: the fact that conditions are chained together in arbitrary trees of AND and OR makes me think that a list is not expressive enough to handle this, and hence we should find some other way to structure multiple filters.