Key generator behavior with explicit keys above 2^53

[inexorabletash]

cc: @bevis-tseng @aliams @beidson

@brettz9 noticed this in #144 as a spec oversight, but implementations are inconsistent and the desired behavior is not obvious to me, so calling this out separately.

Test case is included in web-platform-tests/wpt#4814

As background, if you have a key generator (stored created with autoIncrement: true) you can explicitly advance the generator, e.g.

store.put(value); // gets generated key, e.g. 1
store.put(value, 1000); // gets explicit key and sets generator to 1000
store.put(value); // gets generated key, i.e. 1001
store.put(value, 555); // gets explicit key but is < generator so no change to generator
store.put(value); // gets generated key, i.e. 1002

So far so good.

Now what happens in the following cases:

• Explicit key is greater than the max generator value (2^53 i.e. Number.MAX_SAFE_INTEGER)
• Explicit key is at 2^63 (max int64_t)
• Explicit key is at 2^64 (max uint64_t)
• Explicit key is greater than 2^64
• Explicit key is Infinity

e.g. for Infinity:

store.put(value, Infinity); // gets explicit key and sets generator to ????
store.put(value).onsuccess = e => console.log(e.target.result); // ????
store.put(value).onsuccess = e => console.log(e.target.result); // ????

It looks like implementations are inconsistent and outright buggy here. With Infinity:

• Firefox generates -9223372036854776000 as the next key (!), then ConstraintError for subsequent puts (!!)
• Safari resets the generator to 0, so you start getting 1, 2, 3, etc. again (!)
• Chrome does not modify the generator, but that's only "by accident".

For reference, Chrome internally coerces the input Number (double) to an int64_t before checking to see if it should increment the generator (also an int64_t) so our behavior goes wonky above the 53 bit mark but at the 63 bit mark it ends up being ignored. Wheee.

There are two possible behaviors for any of these ranges of values:

1. Update generator to maximum value (same as if maximum value was passed)
2. Do not modify the generator (same as if a anything lower than current number was passed)

I definitely think treating bleeding implementation details and treating different finite values above the generator range inconsistently is bad. (i.e. I want to fix Chrome here). I don't particularly care what we do for finite vs. infinite values - they could both be (1), or (2), or differ.

Thoughts?

[aliams]

I think updating to the maximum value makes the most sense here.

[brettz9]

As just a small nit, Number.MAX_SAFE_INTEGER is technically 2^53 - 1; it is apparently to prevent clashes between 2^53 and 2^53 + 1 that 2^53 is made unsafe.

It would appear to me that 2^53 itself could safely be used as a current number if numbers above that number were disallowed, though in such a case 2^53 + 1 could also be supplied without error.

[inexorabletash]

I put up a spec PR (linked above) to update to the maximum value per @aliams - feedback welcome.

[brettz9]

If a number higher than the max is supplied (including Infinity), per the PR as well as the current spec, the key provided back to the user in the result (as opposed to the current number) will be kept as their originally supplied key rather than being set down to the max. Is that the desired behavior? If not, I think "possibly update the key generator" needs to be redefined to return a key.

Similarly, is it also the expected behavior (as appears to be the case per the current spec and PR) that if a fractional number is supplied (including > 1 and thus effective at changing the current number), that same fractional key will be returned in the result rather than the rounded value?

[inexorabletash]

Yes. If an explicit key is specified the key generator is NOT used to generate a key.

There's special behavior to update the key generator in certain cases if an explicit key is specified, but the explicit key is never changed.

[inexorabletash]

Blink changed per @aliams's suggest, tests landed, and spec updated. Dissenting feedback still welcome.