search

finagle / finagle-postgres

PostgreSQL protocol support for Finagle
Apache License 2.0
81 stars 39 forks source link

BinaryParameter(true) with preparedStatement produce weird value for BigDecimal #80

Open mikusaikou opened 6 years ago

mikusaikou commented 6 years ago

Hi, I am testing finagle-postgres with CockroachDb, and noticed all negative decimal values inserted into database are strange. After some test I narrowed it down to the binaryParameter.

Here's how I reproduced it :

root@:26257/it_test_db> show create table test;
+-------+-----------------------------------------------+
| Table |                  CreateTable                  |
+-------+-----------------------------------------------+
| test  | CREATE TABLE test (                           |
|       |                                               |
|       |     a INT NOT NULL,                           |
|       |                                               |
|       |     b DECIMAL NULL,                           |
|       |                                               |
|       |     CONSTRAINT "primary" PRIMARY KEY (a ASC), |
|       |                                               |
|       |     FAMILY "primary" (a, b)                   |
|       |                                               |
|       | )                                             |
+-------+-----------------------------------------------+

then create a client withBinaryParams(true) and tried to insert some data:

  val c =Postgres
    .Client()
    .withCredentials("root", None)
    .database("it_test_db")
    .withBinaryParams(true)
    .newRichClient("localhost:26257")

cala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 1, 1.1))
res0: Int = 1

scala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 2, -1.1))
res1: Int = 1

scala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 3, BigDecimal(1.1)))
res2: Int = 1

scala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 4, BigDecimal(-1.1)))
res3: Int = 1

scala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 5, BigDecimal(1)))
res4: Int = 1

scala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 6, BigDecimal(-1)))
res5: Int = 1

but in the database it became :

root@:26257/it_test_db> select * from test;
+---+------------------------+
| a |           b            |
+---+------------------------+
| 1 |                    1.1 |
| 2 |                   -1.1 |
| 3 |                    1.1 |
| 4 | -1844674407370955161.5 |
| 5 |                 100000 |
| 6 |                     -0 |
+---+------------------------+

however if client is created without withBinaryParams(true)

scala>   val c2 =Postgres.Client().withCredentials("root", None).database("it_test_db").newRichClient("localhost:26257")
c2: com.twitter.finagle.postgres.PostgresClientImpl = com.twitter.finagle.postgres.PostgresClientImpl@45682e3e

scala> com.twitter.util.Await.result(c2.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 7, BigDecimal(1.1)))
res6: Int = 1

scala> com.twitter.util.Await.result(c2.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 8, BigDecimal(-1.1)))
res7: Int = 1

scala> com.twitter.util.Await.result(c2.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 9, BigDecimal(1)))
res8: Int = 1

scala> com.twitter.util.Await.result(c2.prepareAndExecute("INSERT INTO test VALUES ($1,$2)", 10, BigDecimal(-1)))
res9: Int = 1

in the database it seems ok:

root@:26257/it_test_db> select * from test;
+----+------------------------+
| a  |           b            |
+----+------------------------+
|  1 |                    1.1 |
|  2 |                   -1.1 |
|  3 |                    1.1 |
|  4 | -1844674407370955161.5 |
|  5 |                 100000 |
|  6 |                     -0 |
|  7 |                    1.1 |
|  8 |                   -1.1 |
|  9 |                      1 |
| 10 |                     -1 |
+----+------------------------+
(10 rows)
jeremyrsmith commented 6 years ago

Looks like an issue with BigDecimal's encoder. Does this issue also occur against PostgreSQL, or just CockroachDB?

mikusaikou commented 6 years ago

@jeremyrsmith so I tested it with postgreSQL 10.4 on docker, and it got error response for negative BigDecimal value. create table with same definition 

postgres=# create table test (a INT, b DECIMAL, PRIMARY KEY (a));

create client with BinaryParameter

scala> val c =Postgres.Client().withCredentials("postgres", None).database("postgres").withBinaryParams(true).newRichClient("localhost:5432")

try insert some data

scala> com.twitter.util.Await.result(c.prepareAndExecute("INSERT INTO test VALUES($1,$2)", 1,BigDecimal(-1.1)))
com.twitter.finagle.postgres.codec.ServerError: invalid digit in external "numeric" value
  at com.twitter.finagle.postgres.codec.Errors$.server(Errors.scala:42)
  at com.twitter.finagle.postgres.codec.HandleErrorsProxy$HandleErrors$.$anonfun$apply$2(PgCodec.scala:44)
  at com.twitter.util.Future.$anonfun$flatMap$1(Future.scala:1755)
  at com.twitter.util.Promise$Transformer.liftedTree1$1(Promise.scala:331)
  at com.twitter.util.Promise$Transformer.k(Promise.scala:331)
  at com.twitter.util.Promise$Transformer.apply(Promise.scala:342)
  at com.twitter.util.Promise$Transformer.apply(Promise.scala:323)
  at com.twitter.util.Promise$WaitQueue.com$twitter$util$Promise$WaitQueue$$runDepth(Promise.scala:121)
  at com.twitter.util.Promise$WaitQueue$$anon$6.run(Promise.scala:109)
  at com.twitter.concurrent.LocalScheduler$Activation.run(Scheduler.scala:198)
  at com.twitter.concurrent.LocalScheduler$Activation.submit(Scheduler.scala:157)
  at com.twitter.concurrent.LocalScheduler.submit(Scheduler.scala:274)
  at com.twitter.concurrent.Scheduler$.submit(Scheduler.scala:109)
  at com.twitter.util.Promise$WaitQueue.runDepthInScheduler(Promise.scala:109)
  at com.twitter.util.Promise$WaitQueue.runDepthInScheduler$(Promise.scala:108)
  at com.twitter.util.Promise$WaitQueue$$anon$8.runDepthInScheduler(Promise.scala:206)
  at com.twitter.util.Promise$WaitQueue.runInScheduler(Promise.scala:96)
  at com.twitter.util.Promise$WaitQueue.runInScheduler$(Promise.scala:94)
  at com.twitter.util.Promise$WaitQueue$$anon$8.runInScheduler(Promise.scala:206)
  at com.twitter.util.Promise.updateIfEmpty(Promise.scala:909)
  at com.twitter.util.Promise.update(Promise.scala:881)
  at com.twitter.util.Promise.setValue(Promise.scala:857)
  at com.twitter.concurrent.AsyncQueue.offer(AsyncQueue.scala:122)
  at com.twitter.finagle.netty3.transport.ChannelTransport.handleUpstream(ChannelTransport.scala:58)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
  at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
  at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:281)
  at com.twitter.finagle.postgres.codec.PgClientChannelHandler.$anonfun$messageReceived$9(PgCodec.scala:244)
  at com.twitter.finagle.postgres.codec.PgClientChannelHandler.$anonfun$messageReceived$9$adapted(PgCodec.scala:244)
  at scala.Option.foreach(Option.scala:257)
  at com.twitter.finagle.postgres.codec.PgClientChannelHandler.messageReceived(PgCodec.scala:244)
  at org.jboss.netty.channel.SimpleChannelHandler.handleUpstream(SimpleChannelHandler.java:88)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
  at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
  at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:281)
  at com.twitter.finagle.postgres.codec.BackendMessageDecoder.messageReceived(PgCodec.scala:133)
  at org.jboss.netty.channel.SimpleChannelHandler.handleUpstream(SimpleChannelHandler.java:88)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
  at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
  at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:296)
  at org.jboss.netty.handler.codec.frame.FrameDecoder.unfoldAndFireMessageReceived(FrameDecoder.java:462)
  at org.jboss.netty.handler.codec.frame.FrameDecoder.callDecode(FrameDecoder.java:443)
  at org.jboss.netty.handler.codec.frame.FrameDecoder.messageReceived(FrameDecoder.java:303)
  at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
  at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
  at org.jboss.netty.channel.SimpleChannelHandler.messageReceived(SimpleChannelHandler.java:142)
  at com.twitter.finagle.netty3.channel.ChannelStatsHandler.messageReceived(ChannelStatsHandler.scala:72)
  at org.jboss.netty.channel.SimpleChannelHandler.handleUpstream(SimpleChannelHandler.java:88)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
  at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
  at org.jboss.netty.channel.SimpleChannelHandler.messageReceived(SimpleChannelHandler.java:142)
  at com.twitter.finagle.netty3.channel.ChannelRequestStatsHandler.messageReceived(ChannelRequestStatsHandler.scala:36)
  at org.jboss.netty.channel.SimpleChannelHandler.handleUpstream(SimpleChannelHandler.java:88)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
  at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:559)
  at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:268)
  at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:255)
  at org.jboss.netty.channel.socket.nio.NioWorker.read(NioWorker.java:88)
  at org.jboss.netty.channel.socket.nio.AbstractNioWorker.process(AbstractNioWorker.java:108)
  at org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:337)
  at org.jboss.netty.channel.socket.nio.AbstractNioWorker.run(AbstractNioWorker.java:89)
  at org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:178)
  at org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
  at org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
  at com.twitter.finagle.util.BlockingTimeTrackingThreadFactory$$anon$1.run(BlockingTimeTrackingThreadFactory.scala:23)
  at java.lang.Thread.run(Thread.java:745)

without BinaryParameter(true) it do works well

scala> val c2 =Postgres.Client().withCredentials("postgres", None).database("postgres").newRichClient("localhost:5432")
c2: com.twitter.finagle.postgres.PostgresClientImpl = com.twitter.finagle.postgres.PostgresClientImpl@f6e9c18

scala> com.twitter.util.Await.result(c2.prepareAndExecute("INSERT INTO test VALUES($1,$2)", 2,BigDecimal(-1.1)))
res4: Int = 1
postgres=# select * from test;
 a |  b
---+------
 2 | -1.1
(1 row)
mikusaikou commented 6 years ago

just provide some update info, I found this on cockroachdb's doc: https://www.cockroachlabs.com/docs/stable/known-limitations.html#silent-validation-error-with-decimal-values