PGNumeric support - Githubissues

lippling commented 8 years ago

I only saw a PGNumeric instance for IsSqlType. Am I missing something or is the PGNumeric support incomplete?

tomjaguarpaw commented 8 years ago

There's instance PGOrd T.PGNumeric here: https://github.com/tomjaguarpaw/haskell-opaleye/blob/7b5e780513ba77d51f43aa78899a78d87fa93ac5/src/Opaleye/Order.hs#L79

But I guess you are saying that we are missing

pgNumeric :: Integer -> Column PGNumeric

instance C.PGNum PGNumeric where
  pgFromInteger = pgNumeric

Do we need anything else? Please feel free to submit a PR!

mzabani commented 8 years ago

I'm pretty new to Haskell and even newer to Opaleye, so sorry for sticking in like this. Shouldn't Numeric be arbitrary precision decimal numbers? Considering that, wouldn't it be better to use Rational in Haskell as the equivalent type for postgresql's Numeric? I was thinking something like

pgNumeric :: (Integral a) => Ratio a -> Column PGnumeric

https://www.postgresql.org/docs/9.2/static/datatype-numeric.html

tomjaguarpaw commented 8 years ago

Sure, that sounds better.

saurabhnanda commented 7 years ago

Just got bitten by this. Is the Default instance (or is it QueryRunnerColumnDefault?) absent because there's no real consensus on what PGNumeric should map to on the Haskell side?

tomjaguarpaw commented 7 years ago

It's absent because no one's ever got round to doing anything about it. According to the postgresql-simple documentation PGNumeric can map to Scientific and Ratio Integer. Does anyone want to submit a PR?

https://hackage.haskell.org/package/postgresql-simple-0.5.0.0/docs/Database-PostgreSQL-Simple-FromField.html#v:pgArrayFieldParser

saurabhnanda commented 7 years ago

Will submit a PR after getting it to work in our app.

saurabhnanda commented 7 years ago

So, we need both instances, right? Default AND QueryRunnerColumnDefault?

saurabhnanda commented 7 years ago

which HPQ.Literal should I be using to complete the following:

pgRatioToNumeric :: Ratio Integer -> Column PGNumeric
pgRatioToNumeric r = undefined

instance ProDef.Default Constant (Ratio Integer) (Column PGNumeric) where
  def = Constant pgRatioToNumeric

tomjaguarpaw commented 7 years ago

You need QueryRunnerColumnDefault and Default Constant, I guess.

You can try DoubleLit or OtherLit.

tomjaguarpaw commented 7 years ago

Perhaps ideally you need to create NumericLit but you can try one of those two to start with.

saurabhnanda commented 7 years ago

I've gotten it to work with the following, but I'm not happy:

instance QueryRunnerColumnDefault PGNumeric (Ratio Integer) where
  queryRunnerColumnDefault = fieldQueryRunnerColumn

pgRatioToNumeric :: Ratio Integer -> Column PGNumeric
pgRatioToNumeric r = IPT.literalColumn $ HPQ.DoubleLit (nr/dr)
  where
    nr :: Double
    nr = fromIntegral $ numerator r

    dr:: Double
    dr = fromIntegral $ denominator r

Problems:

The double conversion is not have any fixed precision and may blow-up if the PG column has a fixed precision+scale
Ratio Integer is horrible to work with. Especially for monetary/currency values, which is the biggest reason why we're using NUMERIC columns in our schema.

How about https://hackage.haskell.org/package/Decimal-0.4.2 ? Anything better?

tomjaguarpaw commented 7 years ago

The double conversion is not have any fixed precision and may blow-up if the PG column has a fixed precision+scale

That's fair enough. Use OtherLit then (probably combined with the show of the Ratio Integer).

Ratio Integer is horrible to work with. Especially for monetary/currency values, which is the biggest reason why we're using NUMERIC columns in our schema.

Why not Scientific?

tomjaguarpaw commented 7 years ago

Announcement

NB I'm taking a holiday from my open source projects until at least 16th January :) Good luck getting this to work. If you really need help from someone before then perhaps you can contact some of the other users who have been posting in the issues pages.

saurabhnanda commented 7 years ago

@tomjaguarpaw one last question before you go :)

Is this error related?

convertAmountToINR :: Query PaymentPGRead -> Query PaymentPGRead
convertAmountToINR payQuery = proc () -> do
  p <- payQuery -< ()
  fx <- queryTable fxRateTable -< ()
  -- NOTE: This is an inner-join and in the edge-case that paymentCurrency=>INR
  -- rate is not available, it might give incorrect results
  restrict -< ((p ^. currency) .== (fx ^. fromCurrency)
               .&& (fx ^. toCurrency) .== (constant "INR"))
  returnA -< p{payCurrency=(constant "INR"), payAmount=((p ^. amount) * (fx ^. rate))}

   314  57 error           error:
     • No instance for (Opaleye.Internal.Column.PGNum PGNumeric)
         arising from a use of ‘*’
     • In the ‘payAmount’ field of a record
       In the expression:
         p {payCurrency = (constant "INR"),
            payAmount = ((p ^. amount) * (fx ^. rate))}
       In the command: returnA -< p {payCurrency = (constant "INR"),
                                     payAmount = ((p ^. amount) * (fx ^. rate))} (intero)

tomjaguarpaw commented 7 years ago

As this comment says https://github.com/tomjaguarpaw/haskell-opaleye/issues/230#issuecomment-258666597 you need a PGNum instance in PGTypes.hs

tomjaguarpaw commented 7 years ago

You'll have to work out how to implement pgNumeric. It should be no more difficult than your pgRatioToNumeric.

tomjaguarpaw commented 7 years ago

Now I really am out of here! Bye.

tomjaguarpaw commented 7 years ago

(If you need any more help with PGNumeric I'm sure @lippling can help you)

tomjaguarpaw commented 7 years ago

Closing as stale. Feel free to reopen if necessary. I'd like to add PGNumeric support to Opaleye but I'm not going to do it unless someone's going to use it.

ruhatch commented 7 years ago

@saurabhnanda What's the status on this?

tomjaguarpaw commented 7 years ago

@ruhatch I don't think anyone attempted it but it should be very easy.

saurabhnanda commented 7 years ago

We've got a version working in our project, but it's not correct. Creating a Decimal requires the knowledge of a precision, IIRC.

      instance FromField Decimal where
        fromField field maybebs = (realToFrac :: Rational -> Decimal)  <$> fromField field maybebs
      instance QueryRunnerColumnDefault PGNumeric Decimal where
        queryRunnerColumnDefault = fieldQueryRunnerColumn
      instance Default Constant Decimal (Column PGNumeric) where
        def = Constant f1
          where
          f1 :: Decimal -> (Column PGNumeric)
          f1 x = unsafeCoerceColumn $ pgDouble $ realToFrac x

tomjaguarpaw commented 7 years ago

@saurabhnanda I don't understand what's difficult about this. Could you explain? @ruhatch Perhaps you'd like to open a new issue with a specific request. I've never fully grasped what was being asked for in this thread.

saurabhnanda commented 7 years ago

@tomjaguarpaw can you please reopen this issue. I'd like to pick this up and submit a PR.

tomjaguarpaw commented 7 years ago

Sure, though perhaps you could clarify exactly what this issue is about.

saurabhnanda commented 7 years ago

This PR isn't going to be easy, and may need some brainstorming. Quoting from the relevant PG documentation:

The type numeric can store numbers with a very large number of digits. It is especially recommended for storing monetary amounts and other quantities where exactness is required. Calculations with numeric values yield exact results where possible, e.g. addition, subtraction, multiplication. However, calculations on numeric values are very slow compared to the integer types, or to the floating-point types described in the next section.

We use the following terms below: The scale of a numeric is the count of decimal digits in the fractional part, to the right of the decimal point. The precision of a numeric is the total count of significant digits in the whole number, that is, the number of digits to both sides of the decimal point. So the number 23.5141 has a precision of 6 and a scale of 4. Integers can be considered to have a scale of zero.

Both the maximum precision and the maximum scale of a numeric column can be configured. To declare a column of type numeric use the syntax:

NUMERIC(precision, scale)

The precision must be positive, the scale zero or positive. Alternatively:

NUMERIC(precision)

selects a scale of 0. Specifying:

NUMERIC

without any precision or scale creates a column in which numeric values of any precision and scale can be stored, up to the implementation limit on precision. A column of this kind will not coerce input values to any particular scale, whereas numeric columns with a declared scale will coerce input values to that scale. (The SQL standard requires a default scale of 0, i.e., coercion to integer precision. We find this a bit useless. If you're concerned about portability, always specify the precision and scale explicitly.)

If I'm reading this correctly, a NUMERIC column can behave like one of the following three things:

Arbitrary precision (mapping to a Scientific, Double, or Float on the Haskell side)
Zero precision (mapping to an Integral type on the Haskell side)
Fixed precision and scale (most probably mapping only to Decimal on the Haskell side)

saurabhnanda commented 7 years ago

Sure, though perhaps you could clarify exactly what this issue is about.

It seems that the Default Constant and Default QueryRunner instances for PGNumeric are missing.

tomjaguarpaw commented 7 years ago

I see, thanks.

tomjaguarpaw commented 7 years ago

I think the lowest common denominator here is to treat Opaleye PGNumeric as Postgres NUMERIC, i.e. arbitrary precision, and probably map it to Scientific. Double and Float seem like bad matches for this datatype.

tomjaguarpaw commented 7 years ago

Later one we can consider type-level Natural indexing for the types with specific precision and scale, but I think we should solve the simple case first.

saurabhnanda commented 7 years ago

I think the lowest common denominator here is to treat Opaleye PGNumeric as Postgres NUMERIC, i.e. arbitrary precision, and probably map it to Scientific. Double and Float seem like bad matches for this datatype.

An important use-case for NUMERIC in Postgres is for storing monetary values (even we're using it that way). The only Haskell data-type that we found, till now, capable of storing monetary values properly is Data.Decimal:

Quoting https://www.postgresql.org/docs/current/static/datatype-numeric.html -

It is especially recommended for storing monetary amounts and other quantities where exactness is required. Calculations with numeric values yield exact results where possible, e.g. addition, subtraction, multiplication.

Quoting https://hackage.haskell.org/package/Decimal-0.4.2 -

The "Decimal" type is mainly intended for doing financial arithmetic where the number of decimal places may not be known at compile time (e.g. for a program that handles both Yen and Dollars) and the application must not drop pennies on the floor.

I'd really like to setup proper NUMERIC<=>Data.Decimal support, but I'm not sure how to do it. The precision/mantissa part in Data.Decimal is at the type-level, not the value-level.

saurabhnanda commented 7 years ago

Do you know how persistent deals with this?

tomjaguarpaw commented 7 years ago

Do you know how persistent deals with this?

I have no idea. I've never used persistent.

tomjaguarpaw commented 7 years ago

postgresql-simple uses Ratio Integer and Scientific: https://github.com/lpsmith/postgresql-simple/blob/6361568f63766d655dba4a473d1aaec4365ef66d/src/Database/PostgreSQL/Simple/FromField.hs#L350 What's wrong with Scientific for your use case? Seems at least vaguely plausible.

saurabhnanda commented 7 years ago

I have no concrete answer, except that "arbitrary precision" and "monetary values" in the same sentence sound dangerous to me.

tomjaguarpaw commented 7 years ago

I have no concrete answer, except that "arbitrary precision" and "monetary values" in the same sentence sound dangerous to me.

Hmm, I would have thought it was the opposite. Ratio Integer also seems perfectly reasonable for monetary values. I'd suggest asking on Stackoverflow or Haskell Reddit for what other people use.

ruhatch commented 7 years ago

I have this working with Scientific and can submit a PR.

@saurabhnanda Can you explain why you think Scientific wouldn't work?

saurabhnanda commented 7 years ago

In the case of NUMERIC(precision, scale), will Scientific be able to represent every number that PG can, without any rounding errors (common IEEE problem with floating points)?

On 06-Nov-2017 9:59 PM, "Rupert Horlick" notifications@github.com wrote:

I have this working with Scientific and can submit a PR.

@saurabhnanda https://github.com/saurabhnanda Can you explain why you think Scientific wouldn't work?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tomjaguarpaw/haskell-opaleye/issues/230#issuecomment-342204136, or mute the thread https://github.com/notifications/unsubscribe-auth/AABu0aVG3WghmQ-X3IV6sgbJFRM5V7Cbks5szzQGgaJpZM4KqanA .

tomjaguarpaw commented 7 years ago

Oh, scientific isn't floating point! It's arbitrary precision: https://hackage.haskell.org/package/scientific

tomjaguarpaw commented 7 years ago

@saurabhnanda I see you said above that Ratio Integer is horrible to work with. Could you expand on that?

tomjaguarpaw commented 7 years ago

(Scientific is basically a worse version of Ratio Integer because it can only represent fractions in base 10)

Superpat commented 7 years ago

I'd like to express my support for working numerics in opaleye. If it helps, my use case is for storing monetary values as well, so Decimal -> PGNumeric would help. Unfortunately I dont have the time right now to fix it myself. I should have more free time after christmas though, so if this hasnt been fixed by then I'll try to make a pr.

tomjaguarpaw commented 7 years ago

I'm not really sure what's holding this up. postgresql-simple already supports conversions to Ratio Integer and Scientific. I propose we just expose them. We can convert to Decimal by adding support to postgresql-simple or quite easily by going via Scientific, for example.

saurabhnanda commented 7 years ago

I can pick this up next week, unless someone beats me to it before that.

On 14-Nov-2017 10:19 PM, "tomjaguarpaw" notifications@github.com wrote:

I'm not really sure what's holding this up. postgresql-simple already supports conversions to Ratio Integer and Scientific. I propose we just expose them. We can convert to Decimal by adding support to postgresql-simple or quite easily by going via Scientific, for example.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tomjaguarpaw/haskell-opaleye/issues/230#issuecomment-344321913, or mute the thread https://github.com/notifications/unsubscribe-auth/AABu0d7bYAXMj8CNif1pk_-bNd15_NQ8ks5s2cSSgaJpZM4KqanA .

mithrandi commented 6 years ago

@saurabhnanda did you have a chance to look at this yet? I'm considering jumping in here as well, but I won't have the time for another few weeks most likely.

tomjaguarpaw / haskell-opaleye

PGNumeric support #230

Announcement