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

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]. Fixes tarantool#26 Fixes tarantool#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 0a94df5 Compare March 31, 2022 10:35

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

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

parihaaraka and 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 9ed3291 Compare July 13, 2022 14:13

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

ligurio force-pushed the master branch 8 times, most recently from 6141671 to 792fc4c Compare 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 b621263 Compare July 18, 2022 14:32

Totktonada self-assigned this Feb 3, 2023

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[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 commented Dec 1, 2021

ligurio commented Mar 25, 2022 •

edited

Loading

parihaaraka commented Mar 28, 2022

ligurio commented Mar 28, 2022

Totktonada commented Mar 31, 2022

parihaaraka commented Mar 31, 2022

Totktonada commented Mar 31, 2022

parihaaraka commented Jun 27, 2022

Totktonada commented Jun 27, 2022

parihaaraka commented Oct 2, 2023

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

Are you sure you want to change the base?

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

Conversation

parihaaraka commented Dec 1, 2021

ligurio commented Mar 25, 2022 • edited Loading

parihaaraka commented Mar 28, 2022

ligurio commented Mar 28, 2022

Totktonada commented Mar 31, 2022

parihaaraka commented Mar 31, 2022

Totktonada commented Mar 31, 2022

parihaaraka commented Jun 27, 2022

Totktonada commented Jun 27, 2022

parihaaraka commented Oct 2, 2023

ligurio commented Mar 25, 2022 •

edited

Loading