Level / abstract-leveldown

An abstract prototype matching the leveldown API.
MIT License
146 stars 53 forks source link

Range tests on zero-length buffer+string are incompatible with bytewise, charwise & IDB #318

Closed vweevers closed 5 years ago

vweevers commented 5 years ago

They assume that a zero-length string or zero-length Buffer means "not defined". IMO that should be left up to an implementation.

https://github.com/Level/abstract-leveldown/blob/fd64104829e94b0187377d9b8b5d3ee546bb39f2/test/iterator-range-test.js#L313-L345

In bytewise, charwise and IndexedDB, an empty string is a significant type. In all three, the sort order of types is:

For example, charwise encodes an empty string as 'J'.

I suggest:

vweevers commented 5 years ago

Note that level-js is currently passing the test suite of abstract-leveldown@5 because abstract-leveldown used to filter out range options that were a zero-length string or buffer.

vweevers commented 5 years ago

Correction: unlike bytewise, IDB sorts binary after string. For reference, here's a quick test of IDB type order:

Click to expand ```js function cmp (a, b) { // # https://www.w3.org/TR/IndexedDB-2/#key-construct // "Let ta be the type of a, let tb be the type of b." var ta = type(a) var tb = type(b) // "If ta is array and tb is binary, string, date or number, return 1." if (ta === 'array' && (tb === 'binary' || tb === 'string' || tb === 'date' || tb === 'number')) return 1 // "If tb is array and ta is binary, string, date or number, return -1." if (tb === 'array' && (ta === 'binary' || ta === 'string' || ta === 'date' || ta === 'number')) return -1 // "If ta is binary and tb is string, date or number, return 1." if (ta === 'binary' && (tb === 'string' || tb === 'date' || tb === 'number')) return 1 // "If tb is binary and ta is string, date or number, return -1." if (tb === 'binary' && (ta === 'string' || ta === 'date' || ta === 'number')) return -1 // "If ta is string and tb is date or number, return 1." if (ta === 'string' && (tb === 'date' || tb === 'number')) return 1 // "If tb is string and ta is date or number, return -1." if (tb === 'string' && (ta === 'date' || ta === 'number')) return -1 // "If ta is date and tb is number, return 1." if (ta === 'date' && (tb === 'number')) return 1 // "If tb is date and ta is number, return -1." if (tb === 'date' && (ta === 'number')) return -1 throw new Error('value comparison is not implemented') } function type (value) { const t = typeof value if (t === 'string' || t === 'number') return t if (Buffer.isBuffer(value)) return 'binary' // stored same as arraybuffer if (Array.isArray(value)) return 'array' if (Object.prototype.toString.call(value) === '[object Date]') return 'date' throw new Error('unsupported type') } // [ 3, , '', , [] ] console.log([ new Date(), '', 3, [], Buffer.alloc(0) ].sort(cmp)) ```