Closed lidel closed 3 years ago
Do we need a match
parameter at all? Can we not just look at the passed name
and if it contains a *
character it's a wildcard match and if not it's exact? Case sensitive/insensitive or search flags could be an option?
@achingbrain
If we replace match
with support for *
and add optional case=sensitive|insensitive
, then services need to implement parser and 4 strategies instead of two (exact-casesensitive, exact-caseinsensitive, partial-casesensitive, partial-caseinsensitive), and API users need to learn that *
has special meaning, and we need to spec that it can't be used in names (or spec how to escape it).
I'd argue match=exact|iexact|partial|ipartial
added in f96383a is much easier for services and users,
but I'm open for suggestions if there is a simpler way to represent this (when match
is missing, exact
is used).
PR for implementation on the ruby pinning api server over here: https://github.com/ipfs-shipyard/rb-pinning-service-api/pull/5
This PR looks great to me. The changes will be helpful in keeping things performant by default while also allowing for some flexibility.
tldr
This PR changes the default behavior of the
name
filterfrom case-insensitive, partial match to much faster case-sensitive, exact (full) match
and adds
match
parameter that enables opt-in into alternative strategy when performance is not an object:PREVIEW: https://ipfs.github.io/pinning-services-api-spec/#specUrl=https://raw.githubusercontent.com/ipfs/pinning-services-api-spec/fix/default-to-fast-name-match/ipfs-pinning-service.yaml
Rationale for this change
Case-insensitive partial "Fuzzy" search (
ipartial
) makes a bad default. (performance-wise) Database indexes and b-trees do not solve the performance fully. Exact match is always faster and less expensive.Fuzzy search may produce bugs This PR introduces
match
query parameter, so developers who need fuzzy search can still opt-in into slower, but more flexible text matching strategy by passingmatch
parameter.We are moving away from slow defaults in IPFS ecosystem. Better to do this now, than later.
ipfs pin ls
, which is abysmally slow when one has >1k pins. Nearly all pins in real life are recursive, andipfs pin ls --type=recursive
executes instantly, but is rarely used due to not being the default.ipfs pin remote
andipfs pin local
APIs will be inspired by this spec and will havename
attribute, however we don't want to re-introduce bad choices into IPFS ecosystem. The default should be the fastest option available, and in this case it is the exactname
match.Implementation notes and migration plan
This change does not change the wire format and does not impact ongoing MVP integration in go-ipfs and ipfs-webui (https://github.com/ipfs/ipfs-gui/issues/91, https://github.com/ipfs/go-ipfs/issues/7559), but we want to include it in this spec to ensure MVP does the right thing.
Existing Pinning Services already implement the more difficult
fuzzy
ipartial
strategy. Adding support formatch
filter and implementing much simplerexact
strategy should be a small task, but will save everyone headache in the long run.@aschmahmann @jacobheun @petar @gammazero @obo20 @andrew @GregTheGreek @priom @jsign @sanderpick @andrewxhill @ipfs/wg-pinning-services