[u]int64_t parameters; decimal->number cast removed in favor of original textual representation by parihaaraka · Pull Request #28 · tarantool/pg

parihaaraka · 2021-12-01T11:29:30Z

No description provided.

ligurio · 2022-03-25T16:16:05Z

Patch breaks tests:

Log

sergeyb@pony:~/sources/MRG/pg$ git describe 2.0.2-7-g4043e43
sergeyb@pony:~/sources/MRG/pg$ curl -O https://patch-diff.githubusercontent.com/raw/tarantool/pg/pull/28.patch
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 1694 0 1694 0 0 3948 0 --:--:-- --:--:-- --:--:-- 3948
sergeyb@pony:~/sources/MRG/pg$ patch -p1 < 28.patch patching file pg/driver.c
sergeyb@pony:~/sources/MRG/pg$ make
[100%] Built target driver
sergeyb@pony:~/sources/MRG/pg$ tarantool --version
Tarantool 2.8.3-0-g01023dbc2
sergeyb@pony:~/sources/MRG/pg$ pg_config --version
PostgreSQL 14.2 (Ubuntu 14.2-1.pgdg20.04+1)
sergeyb@pony:~/sources/MRG/pg$ sudo -u postgres tarantool test/pg.test.lua
TAP version 13
# connection old api
1..15
ok - connection
ok - ping
ok - SELECT 123::text AS bla, 345
ok - SELECT -1 AS neg, NULL AS abc
not ok - SELECT -1.1 AS neg, 1.2 AS pos
---
path: //1/1/1/neg
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: -1.1
got: '-1.1'
...
not ok - SELECT ARRAY[1,2] AS arr, 1.2 AS pos
---
path: //1/1/1/pos
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: 1.2
got: '1.2'
...
ok - SELECT $1 AS val % ["abc"]
not ok - SELECT $1 AS val % [123]
---
path: //1/1/1/val
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: 123
got: '123'
...
ok - SELECT $1 AS val % [true]
ok - SELECT $1 AS val % [false]
not ok - SELECT $1 AS val, $2 AS num, $3 AS str % [false,123,"abc"]
---
path: //1/1/1/num
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: 123
got: '123'
...
ok - SELECT * FROM (VALUES (1,2), (2,3)) t
# tx
1..7
ok - begin
ok - SELECT * FROM _tx_test
ok - roolback
ok - SELECT * FROM _tx_test
ok - begin
ok - commit
ok - SELECT * FROM _tx_test
# tx: end
ok - tx
# numeric string binding
1..1
ok - SELECT TEST1_REQUESTID FROM TEST1;# numeric string binding: end
ok - numeric string binding
ok - error
# connection old api: end# failed subtest: 4
TAP version 13
# connection old api via pool
1..15
ok - connection
ok - ping
ok - SELECT 123::text AS bla, 345
ok - SELECT -1 AS neg, NULL AS abc
not ok - SELECT -1.1 AS neg, 1.2 AS pos
---
path: //1/1/1/neg
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: -1.1
got: '-1.1'
...
not ok - SELECT ARRAY[1,2] AS arr, 1.2 AS pos
---
path: //1/1/1/pos
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: 1.2
got: '1.2'
...
ok - SELECT $1 AS val % ["abc"]
not ok - SELECT $1 AS val % [123]
---
path: //1/1/1/val
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: 123
got: '123'
...
ok - SELECT $1 AS val % [true]
ok - SELECT $1 AS val % [false]
not ok - SELECT $1 AS val, $2 AS num, $3 AS str % [false,123,"abc"]
---
path: //1/1/1/num
line: 0
strict: false
filename: test/pg.test.lua
trace:
- line: 26
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: method
name: q
src: test/pg.test.lua
- line: 22
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: Lua
namewhat: local
name: fun
src: test/pg.test.lua
- line: 218
source: '@builtin/tap.lua'
filename: builtin/tap.lua
what: Lua
namewhat: field
name: test
src: builtin/tap.lua
- line: 0
source: '@test/pg.test.lua'
filename: test/pg.test.lua
what: main
namewhat: src: test/pg.test.lua
expected: 123
got: '123'
...
ok - SELECT * FROM (VALUES (1,2), (2,3)) t
# tx
1..7
ok - begin
ok - SELECT * FROM _tx_test
ok - roolback
ok - SELECT * FROM _tx_test
ok - begin
ok - commit
ok - SELECT * FROM _tx_test
# tx: end
ok - tx
# numeric string binding
1..1
ok - SELECT TEST1_REQUESTID FROM TEST1;# numeric string binding: end
ok - numeric string binding
ok - error
# connection old api via pool: end# failed subtest: 4
TAP version 13
# test collection connections
1..1
ok - gc connections
# test collection connections: end
TAP version 13
# connection concurrent
1..1
ok - concurrent connections
# connection concurrent: end
TAP version 13
# int64
1..1
ok - int64 test# int64: end

parihaaraka · 2022-03-28T09:53:09Z

Patch breaks tests:

Because client-side decimal->number cast is a faulty one. Take a look at Qt's NumericalPrecisionPolicy. If a user has no choice then we should preserve original precision.

ligurio · 2022-03-28T11:56:12Z

Patch breaks tests:
Because client-side decimal->number cast is a faulty one.

Patch cannot be merged if it makes CI red. Please fix tests.

Totktonada · 2022-03-31T15:42:14Z

Thanks for the patch!

The idea looks right for me. My doubts are about backward compatibility. I would prefer a safe way and keep everything as before if no new options are passed. So existing applications will not need to update its code if everything works for them. We'll suggest to enable the new (more correct) behaviour in README and release notes, but we should not make it the only option.

I see that (comparing to the first version) you added ability to pass an option, but:

It changes :execute() arguments order. So the old code will need to be updated anyway. I would add an opts table argument at the end and make it optional. As the side good effect: we'll not need so much arguments and will able to add more options later.
Ideally I would add the options as on the connection level as well as on the request level. It is how, for example, our json encoder works: you can configure an instance or pass option to particular json.encode() / json.decode() call.
The idea with type decoding hook is interesting. I don't mind to add it (so a user will able to get decimal value in tarantool, that's nice), but I'm not sure that we'll keep such API if more type related works will be planned (IOW, it is possible that we'll deprecate it after a half of the year).

Also I would split the changes into two commits (one regarding numeric decoding, one about cdata numbers encoding), because now it is not quite obvious what actually is changed.

parihaaraka · 2022-03-31T17:49:08Z

It changes :execute() arguments order. So the old code will need to be updated anyway.

No, this is not connection's execute() - it's internal one. Everything is absolutely backward compatible. Previous behaviour is the default one.

Totktonada · 2022-03-31T18:26:44Z

NB: link #26, check whether we should copy the static buffer before passing it to libpq (in binding parameters handling)

parihaaraka · 2022-06-27T10:50:07Z

@Totktonada Ъ?

Totktonada · 2022-06-27T11:07:34Z

Sorry, I'm on finishing my current quarter tasks. Hopefully I'll end on this week.

…nal textual representation

… is 'n')

[REMOVE BEFORE MERGE] Changes proposed in fixup commit: documentation, regression tests, code style. Patch adds support of PostgreSQL decimal numbers - it is possible to choose a Lua/Tarantool type that will be used for casting values with NUMERIC type [1]. By default Lua "number" type is used, the same behaviour was used before changes. Additionally user could choose two other types: Lua "string" and Tarantool's decimal type [2], [3]. Fixestarantool#26Fixestarantool#25 1. https://www.postgresql.org/docs/current/datatype-oid.html 2. https://www.tarantool.io/en/doc/latest/reference/reference_lua/decimal/ 3. https://www.tarantool.io/en/doc/latest/book/box/data_model/#data-types Co-authored-by: Sergey Bronnikov <sergeyb@tarantool.org>

1. https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough

parihaaraka · 2023-10-02T09:37:00Z

@Totktonada Ъ?

parihaaraka mentioned this pull request Dec 3, 2021
unable to use [u]int64 as parameter #26
Open

Totktonada self-requested a review January 25, 2022 19:18

parihaaraka force-pushed the master branch from d039af2 to 0a94df5Compare March 31, 2022 10:35

parihaaraka force-pushed the master branch from d0303c1 to db13044Compare March 31, 2022 19:06

ligurio self-requested a review July 11, 2022 10:12

parihaarakaand others added 4 commits July 13, 2022 17:06

[u]int64_t parameters; decimal->number cast removed in favor of origi…
3b0acaf
…nal textual representation

new connection.dec_cast option ('s' - string, 'n' - number, default…
6b93cc8
… is 'n')

optional pg decimal to tnt decimal cast
6b7de56

multiple [u]int64_t parameters usage fix
9ed3291

ligurio force-pushed the master branch from db13044 to 9ed3291Compare July 13, 2022 14:13

Totktonada force-pushed the master branch from 9ed3291 to 84208b9Compare July 13, 2022 14:13

ligurio force-pushed the master branch 8 times, most recently from 6141671 to 792fc4cCompare July 18, 2022 13:02

ligurio added 2 commits July 18, 2022 17:24

Fix fallthrough compiler warnings
b621263
1. https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough

ligurio force-pushed the master branch from 792fc4c to b621263Compare July 18, 2022 14:32

Totktonada self-assigned this Feb 3, 2023

ligurio removed their request for review June 6, 2023 13:26

Navigation Menu

Uh oh!

[u]int64_t parameters; decimal->number cast removed in favor of original textual representation - #28

[u]int64_t parameters; decimal->number cast removed in favor of original textual representation#28
parihaaraka wants to merge 6 commits into
tarantool:masterfrom
parihaaraka:master

parihaaraka commented Dec 1, 2021

Uh oh!

ligurio commented Mar 25, 2022•
edited
Loading

Uh oh!

parihaaraka commented Mar 28, 2022

Uh oh!

ligurio commented Mar 28, 2022

Uh oh!

Totktonada commented Mar 31, 2022

Uh oh!

parihaaraka commented Mar 31, 2022

Uh oh!

Totktonada commented Mar 31, 2022

Uh oh!

parihaaraka commented Jun 27, 2022

Uh oh!

Totktonada commented Jun 27, 2022

Uh oh!

parihaaraka commented Oct 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

parihaaraka commented Dec 1, 2021

Uh oh!

ligurio commented Mar 25, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parihaaraka commented Mar 28, 2022

Uh oh!

ligurio commented Mar 28, 2022

Uh oh!

Totktonada commented Mar 31, 2022

Uh oh!

parihaaraka commented Mar 31, 2022

Uh oh!

Totktonada commented Mar 31, 2022

Uh oh!

parihaaraka commented Jun 27, 2022

Uh oh!

Totktonada commented Jun 27, 2022

Uh oh!

parihaaraka commented Oct 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ligurio commented Mar 25, 2022•
edited
Loading