Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Spec compliant number representation & precision #45

Closed
5 tasks
wschella opened this issue Aug 7, 2019 · 6 comments
Closed
5 tasks

Spec compliant number representation & precision #45

wschella opened this issue Aug 7, 2019 · 6 comments
Labels
difficulty:challenging Implementing or fixing this will present a doable challenge priority:low

Comments

@wschella
Copy link
Member

wschella commented Aug 7, 2019

The [SPARQL-spec] uses multiple numeric datatypes from the xsd-spec.
These types have a fixed representation, minimal value, maximum value, precision, ...

Currently none of this is checked by us, and everything is the JS number datatype, which is a "a double-precision 64-bit binary format IEEE 754 value", i.e. a double in C, f64 in Rust.

  • Parse (and mostly validate) incoming values correctly
  • Store internally at the correct precision through libraries such as decimal.js
  • Handle correctly during operations such as addition (i.e. should we overflow, widen the type?)
  • Handle correctly during Type Promotion (Implement Type Promotion #12 )
  • Export and print in the correct lexical form (although I think this is already the case)

Ideally the strictness is all quite configurable.

@wschella wschella added difficulty:challenging Implementing or fixing this will present a doable challenge priority:low labels Aug 7, 2019
@wschella wschella changed the title Use decimal.js for number representation internally Spec compliant number representation & precision Aug 25, 2021
@jitsedesmet
Copy link
Member

Handle correctly during operations such as addition (i.e. should we overflow, widen the type?)

We already fixed and documented this I think? In code: https://github.com/comunica/sparqlee/blob/master/lib/functions/Helpers.ts#L214-L224

And more precisely in the url talked about there: https://www.w3.org/TR/xpath20/#mapping
We see a table that talks about the types.

@wschella
Copy link
Member Author

wschella commented Sep 2, 2021

I'm aware we do it correctly at the type level. I added it here among the checklist to remind us that we should also keep it in mind when doing it with at the value/representation level.

@jitsedesmet
Copy link
Member

I think all of these things are already handled?

1: Should be okey? (after this many years)
2: #44
3: I think the OverloadTree handles these things
4: #12 ?
5: Just like you, I also think this is done

@wschella
Copy link
Member Author

wschella commented Sep 6, 2023

I'm not sure about anything, it's been a while and there have been many changes.

I do think however that 3 might not be implemented correctly. If you add two limited width integers (e.g. shorts), and the result of their addition or multiplication exceeds the bounds of the datatype, what are we supposed to do? JS won't complain, and we will serialize a number too big, but still call it xsd:short. What does the spec mention about overflow? Either we have to widen the datatype, do modulo arithmetics?

Related, I don't think we validate that when we receive an xsd:short, whether it iactually fits into an xsd:short. Not saying we should error, but a warning would be good, or at least an optino to trigger a warning.

I would make two separate new issues for this in the communica repo.

@jitsedesmet
Copy link
Member

Oh yeah! I misunderstood. Thank you!

@jitsedesmet
Copy link
Member

@jitsedesmet jitsedesmet closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
difficulty:challenging Implementing or fixing this will present a doable challenge priority:low
Projects
None yet
Development

No branches or pull requests

2 participants