Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Open
mikusaikou opened this issue Jun 25, 2018 · 3 comments
Open

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

mikusaikou opened this issue Jun 25, 2018 · 3 comments

Comments

@mikusaikou
Copy link

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
Copy link
Collaborator

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

@mikusaikou
Copy link
Author

@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
Copy link
Author

mikusaikou commented Sep 5, 2018 •
edited
Loading

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jeremyrsmith @mikusaikou