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.

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

box: support varbinary in splice update operation #9998

Merged

Conversation

alyapunov
Copy link
Contributor

@alyapunov alyapunov commented May 8, 2024

Just in case splice is an update operation that can modify string
field, deleting a part of it and inserting something instead.

tarantool> box.tuple.new{'1234567'}:update{{':', 1, 2, 3, '!'}}
---
- ['1!567']
...

In the this example splice operation ':' takes field 1 of tuple,
takes position 2 in string, removes 3 symbols and inserts '!' to
that position.

Both deletion and insertion can naturally degrade to no-op.

Before this patch a splice operation required both field and
inserting argument to be strings.

This patch allows the field and the argument to be varbinary.

tarantool> varb = require('varbinary')
tarantool> t = box.tuple.new{varb.new('1234567')}
tarantool> t:update{{':', 1, 2, 3, varb.new('!')}}
---
- [!!binary MSE1Njc=]
...

That also allows to insert strings into varbinary fields and
insert varbinary data into string. The actual field type after
update such operation remains the same, so updated string will
be string, while updated varbinary will be varbinary.

Closes #9997

@coveralls
Copy link

coveralls commented May 8, 2024

Coverage Status

coverage: 87.099% (-0.01%) from 87.11%
when pulling f1777cb on alyapunov:gh-9997-support-splice-for-varbinary
into ef3152c
on tarantool:master
.

@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch 3 times, most recently from 99beff2 to 82f3cfa Compare May 15, 2024 15:17
@alyapunov alyapunov requested a review from a team as a code owner May 15, 2024 15:17
@alyapunov alyapunov changed the title WIP box: support varbinary in splice update operation May 15, 2024
@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch from 82f3cfa to 648d5e6 Compare May 15, 2024 15:21
@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch from 648d5e6 to 4686b36 Compare May 20, 2024 15:52
@alyapunov alyapunov requested a review from nshy May 20, 2024 15:56
@alyapunov alyapunov assigned nshy and unassigned alyapunov and CuriousGeorgiy May 20, 2024
src/box/xrow_update_field.c Outdated Show resolved Hide resolved
test/box-luatest/gh_9997_update_splice_test.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_10032_update_upsert_splice_test.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_10032_update_upsert_splice_test.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_10032_update_upsert_splice_test.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_10032_update_upsert_splice_test.lua Outdated Show resolved Hide resolved
@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch from 4686b36 to 403d699 Compare May 21, 2024 12:58
@alyapunov alyapunov requested a review from nshy May 21, 2024 12:58
@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch from 403d699 to 22721d5 Compare May 21, 2024 13:29
@nshy nshy assigned alyapunov and unassigned nshy May 22, 2024
src/box/xrow_update_field.c Show resolved Hide resolved
src/box/xrow_update_field.c Outdated Show resolved Hide resolved
@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch from 5c07c45 to fd967c8 Compare May 22, 2024 14:03
@alyapunov alyapunov requested a review from nshy May 22, 2024 14:04
@alyapunov alyapunov assigned nshy and unassigned alyapunov May 22, 2024
@nshy nshy assigned alyapunov and unassigned nshy May 23, 2024
@alyapunov alyapunov assigned nshy and unassigned alyapunov May 23, 2024
@TarantoolBot document
Title: splice update operation now supports varbinary

Just in case splice is an update operation that can modify string
field, deleting a part of it and inserting something instead.
```
tarantool> box.tuple.new{'1234567'}:update{{':', 1, 2, 3, '!'}}
---
- ['1!567']
...
```
In the this example splice operation ':' takes field 1 of tuple,
takes position 2 in string, removes 3 symbols and inserts '!' to
that position.

Both deletion and insertion can naturally degrade to no-op.

Before this patch a splice operation required both field and
inserting argument to be strings.

This patch allows the field and the argument to be varbinary.
```
tarantool> varb = require('varbinary')
tarantool> t = box.tuple.new{varb.new('1234567')}
tarantool> t:update{{':', 1, 2, 3, varb.new('!')}}
---
- [!!binary MSE1Njc=]
...

```

That also allows to insert strings into varbinary fields and
insert varbinary data into string. The actual field type after
update such operation remains the same, so updated string will
be string, while updated varbinary will be varbinary.

Closes tarantool#9997
This patch tests update splice operation more thoroughly:
* All variants of splice: delete/insert/replace/noop.
* All variants of position: negative/positive/zero/OOB.
* One operation and several sequential operations.
* Tuple update, space update, space upsert.
* String of varbinary field/arg.

Follow up tarantool#9997
Closes tarantool#10032

NO_DOC=tests
NO_CHANGELOG=tests
@alyapunov alyapunov force-pushed the gh-9997-support-splice-for-varbinary branch from fd967c8 to f1777cb Compare May 24, 2024 10:02
@nshy nshy assigned alyapunov and CuriousGeorgiy and unassigned nshy and alyapunov May 24, 2024
Copy link
Member

@CuriousGeorgiy CuriousGeorgiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great that you managed to reduce that patch to just changing the input and output points of the slice operation!

This is out of the scope of this patch series, however, regarding the slice operation testing: would be great to have fuzzing tests for xrow_update_execute or some lower level xrow_update_* functions just like we already have test/fuzz/xrow_decode_* tests — see, for instance, #9022.

@alyapunov alyapunov added the full-ci Enables all tests for a pull request label May 24, 2024
@alyapunov alyapunov merged commit a9846f4 into tarantool:master May 24, 2024
81 checks passed
@alyapunov alyapunov deleted the gh-9997-support-splice-for-varbinary branch May 24, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support splice update operation for varbinary fields
6 participants