-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarify the handling of +0 and -0 in keys #375
Comments
Ooof, I can't believe we overlooked this. I'd be in favor of saying that keys should be normalized to +0 but we should survey implementations and see what they are all doing. @abacabadabacaba do you have details about what each implementation does? |
Firefox is normalizing the keys, Chromium doesn't. I reported the Firefox behavior here as violating the spec. |
Thanks so much for the spec bug and Firefox bug with easy reproduction! Firefox was not intentionally normalizing here, bit manipulations just went wrong when trying to create a binary encoding that sorted sensibly. That said, normalizing to +0 intentionally does seem desirable because of the uniqueness problem (because of the language around key equal to). |
I put together a test & normalization fix for Blink. The WPT is here: Without a code change, as expected these are the failures: FAIL put(v, -0) key should normalize to +0 assert_equals: key should be normalized to +0 expected 0 but got -0 Anyone want to take a look at the WPT and see if I missed any important cases? One that comes to mind is |
And a spec PR at #386 but maybe we want to say more. Feedback welcome! |
@marcoscaceres - know anyone on WebKit that would interested in reviewing the WebKit behavior and/or spec/WPTs here? @asutherland - same question, but for Gecko? |
I'll try to find someone... |
WebKit does not normalize negative 0, so for the proposed test, here are the failed tests: Looks like we have a different case than Blink in the no-normalization implementation. Generally I think the normalization would simplify the spec and implementation, i.e. we can always assume no -0 in keys. But with existing implementation we have, that probably means we need to convert -0 in database to 0 after the spec change. How is Blink going to deal with this? @inexorabletash For the test, I think we can add test for extracting key with key path from value (e.g. ensure -0 is converted to 0 in key, but not in value). |
Apologies for the delay, I've been OOO for a bit.
@szewai - In Blink, all internal key comparisons treat -0 and 0 the same. The patch just ensures that when doing a key-to-JS conversion any -0 is mapped to +0. Whatever was previously stored in the database gets "fixed" on retrieval. This only happens to keys, not values. I can't think of a case that doesn't cover, but I may be missing something. |
Hello! This sounds good to me. I filed https://bugs.webkit.org/show_bug.cgi?id=258053. |
Currently, according to the spec, both
+0
and-0
are valid keys. So, when a value is stored with the key-0
, a later call togetAllKeys
should return-0
as the key. However, some implementations instead are replacing-0
with+0
.Also, given that
-0 == +0
, an object store or a unique index can have either-0
or+0
as a key, but not both.The value
+0
or-0
can be the whole key, or a (possibly nested) array element of the key.I think that the spec should be clarified to note the existence of equal but different keys, and to provide examples of correct behavior (e.g.
put
replacing an entry with a different key).The text was updated successfully, but these errors were encountered: