search

HaxeFoundation / record-macros

Macro-based ORM (object-relational mapping)
MIT License
49 stars 24 forks source link

SBigInt is a Null<Float> should be a Null<Int> #60

Open filt3rek opened 3 years ago

filt3rek commented 3 years ago

Hej,

If I understand well and as the title of the issue suggest, we should mabye requalify some typedefs https://community.haxe.org/t/php-mysql-bigint-in-resultset-given-as-string/3165/10

jonasmalacofilho commented 3 years ago

I disagree.

I think the original intent of typedef SBigInt = Null<Float> was to use a standard Haxe type (at the time) that could at least approximately represent SQL BIGINT values.

Given that, Null<Int> would be worse: instead of loss of precision we would get overflows, unless the data really is small. But why would data that reliably fits into a 32-bit integer (considering the lower bound of Int size) ever be stored on the DB in an 8-byte SQL BIGINT?

Additionally, I'm also concerned about breaking compatibility.

In the end, I think a new type is needed, mapping to Int64 (or Null<Int64>).

filt3rek commented 3 years ago

Hej Jonas, Thanks for your reply. I'm not very aware about all that 32/64bits, I just put this issue acording to what Aleksandr replied me on the forum, saying it should be Int. As you can see, by myself I put it into Float test case first. But if you think it should be Null I vote for that ! :)

jonasmalacofilho commented 3 years ago

Thanks for your reply. I'm not very aware about all that 32/64bits, I just put this issue acording to what Aleksandr replied me on the forum, saying it should be Int. As you can see, by myself I put it into Float test case first.

I trust that Aleksandr knows more about Haxe than me. Even more so now that I've been a few months without touching Haxe, and some details are no longer super fresh in my mind.

But it seems that on many targets Int is limited to 32 bits even on 64-bit platforms (which is, honestly, how I remember it)... and on the rest it's limited to 52 bits.

But if you think it should be Null I vote for that ! :)

I'm not sure about that part, if I'm honest. It seems that we map almost all types to Null<H>, which makes sense since SQL columns are by default nullable. But there a few exceptions where we just map to H...

RealyUniqueName commented 3 years ago

The question on forums was asked regarding PHP runtime, so I was not thinking about other targets. For cross-target libraries like record-macros Int is not appropriate indeed. But neither is Float. Once you get big enough values in a primary key typed as BIGINT, your Haxe app will start randomly crippling your data.