tarantool / nginx_upstream_module

Tarantool NginX upstream module (REST, JSON API, websockets, load balancing)
Other
174 stars 18 forks source link

tnt_upsert and tnt_update should be like lua's functions #110

Closed ja1cap closed 6 years ago

ja1cap commented 6 years ago
  location /upsert {
    tnt_upsert 514 "key=%s,new_value=%n" "updated_value=%on";
    tnt_pass tnt;
  }

Request example

/upsert?key=test_inc&new_value=1&updated_value=%2B,2,5

Tuple inserted ['test_inc', 1], but the second request doesn't update field 2, if you change operation from "+"(%2B) to "=", then tuple would be updated ['test_inc', 1, 5], instead of changing field 2, field 3 is changed

dedok commented 6 years ago

Hi,

It isn't a really bug, exept delete, I forget to add '#' in switch-case, it was added in f857bfb1af6535371a2f2b77ff0b1ac6c24e0a52. About next part of the issue. I guess you did url encoding twice.

Also I did update README.md, and I did write examples: https://github.com/tarantool/nginx_upstream_module/blob/master/examples/simple_rest_client.sh -- here is using urlencoding

https://github.com/tarantool/nginx_upstream_module/blob/master/examples/simple_rest_client.py -- here isn't using url encoding, since urlib2 has this feature.

ja1cap commented 6 years ago

Do you mean that fieldno would be 0 for the first field?

Fieldno should start with 1, as I know from tarantool docs

box.space.tester:update(999, {{'=', 2, 'B'}}) 
The first argument is tester, that is, the affected space is tester. The second argument is 999, that is, the affected tuple is identified by primary key value = 999. The third argument is =, that is, there is one operation — assignment to a field. The fourth argument is 2, that is, the affected field is field[2]. The fifth argument is 'B', that is, field[2] contents change to 'B'. Therefore, after this update, field[1] = 999 and field[2] = 'B'.
dedok commented 6 years ago

Yes it does. Since It pure binary communication between nginx and tarantool and about yours example it starts from 1 because yours example written in lua.

What do you think, should I add +1 for compatibility?

ja1cap commented 6 years ago

Yes, I think it's better to add +1 to fieldno in this case, 0-based fieldno can be unexpected for those who used tarantool and decided to try nginx module

dedok commented 6 years ago

Sound good, So I reopen this issue.

dedok commented 6 years ago

fixed in 9e38881160e3c00cc0bf92bdfe05ef61a1547f53