Skip to content

Check support for named fields in update/upsert #232

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

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

nicktorwald
Copy link

Add required tests that show support for named fields introduced in
Tarantool 2.3. This feature affects update and upsert operation
triplets allowing to use field names specified as a space format.

Extend operation DSL part to use named field in addition to numeric
ones.

Closes: #220

@coveralls
Copy link

coveralls commented Oct 28, 2019

Coverage Status

Coverage increased (+0.05%) to 77.686% when pulling 5fa568c on nicktorwald/gh-220-update-by-field-name into f32bafa on master.

@Totktonada
Copy link
Member

Answered questions in cc8d73a comments. In short, those operations are possible when we're working with nullable fields.

As far as I see named fields works properly with (client ops).update() API? So we need to just add tests on them to close the issue? If so, we can merge those tests separately if you'll open a separate PR about that.

We have Lua style guide: it would be good to follow it in examples. I mean, { foo='bar' } should be {foo = 'bar'} — whitespaces around =, but not after { and before }.

@nicktorwald nicktorwald force-pushed the nicktorwald/gh-220-update-by-field-name branch 2 times, most recently from 27b8436 to cdaffba Compare November 20, 2019 10:11
@nicktorwald
Copy link
Author

Thank you for the answers. Now it is clear. I extended the test cases where I Included two more operation types mentioned above. Also, I removed extra whitespaces to follow the style guide. Hope, after schema update is merged It will be a good candidate to be released next.

@Totktonada
Copy link
Member

The new tests against ops.{update,upsert}() looks good for me, other parts of the PR are subjects to extract into PR #247.

@nicktorwald nicktorwald force-pushed the nicktorwald/gh-220-update-by-field-name branch from cdaffba to 9988eb3 Compare November 26, 2019 13:05
Add required tests that show support for named fields introduced in
Tarantool 2.3. This feature affects update and upsert operation
triplets allowing to use field names specified as a space format.

Closes: #220
@nicktorwald nicktorwald force-pushed the nicktorwald/gh-220-update-by-field-name branch from 9988eb3 to 5fa568c Compare November 26, 2019 14:44
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

LGTM.

@Totktonada Totktonada merged commit 061c8d4 into master Nov 26, 2019
@Totktonada Totktonada deleted the nicktorwald/gh-220-update-by-field-name branch November 26, 2019 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure update by a field name works
3 participants