Skip to content

api: any msgpack supported type as a request key #244

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

Conversation

DifferentialOrange
Copy link
Member

Inserting a tuple with connector does not have any type restrictions on its contents: any MessagePack supported type is allowed. If there are any type violations on the Tarantool side, server would return the corresponding error. There is no reason to do any explicit type checks for request keys.

This patch fixed using extended types as keys. Tarantool 2.10 does not support indexing intervals, so there are no tests for INTERVAL extension type.

Closes #240

@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-240-extention-type-keys branch from c3faa88 to 6675bb1 Compare October 13, 2022 13:09
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-240-extention-type-keys branch from 6675bb1 to 30d8cd3 Compare October 14, 2022 08:37
@oleg-jukovec
Copy link
Contributor

If there are any type violations on the Tarantool side, server would return the corresponding error.

Is there a test for this behavior?

@DifferentialOrange
Copy link
Member Author

If there are any type violations on the Tarantool side, server would return the corresponding error.

Is there a test for this behavior?

To be honest, I don't think it's worth it because it would be a test for Tarantool only.

  • Create some schema with eval
  • Insert something
  • Select something with invalid key type, get format mismatch

This behavior haven't even changed with this patch.

Copy link
Contributor

@GRISHNOV GRISHNOV left a comment

Choose a reason for hiding this comment

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

Hi! LGTM

Inserting a tuple with connector does not have any type restrictions on
its contents: any MessagePack supported type is allowed. If there are
any type violations on the Tarantool side, server would return the
corresponding error. There is no reason to do any explicit type checks
for request keys.

This patch fixed using extended types as keys. Tarantool 2.10 does not
support indexing intervals, so there are no tests for INTERVAL extension
type.

Closes #240
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-240-extention-type-keys branch from 30d8cd3 to 0011ff6 Compare October 17, 2022 13:11
@DifferentialOrange
Copy link
Member Author

If there are any type violations on the Tarantool side, server would return the corresponding error.

Is there a test for this behavior?

To be honest, I don't think it's worth it because it would be a test for Tarantool only.

* Create some schema with eval

* Insert something

* Select something with invalid key type, get format mismatch

This behavior haven't even changed with this patch.

In other words, there is no such thing as "unsupported type of a request key". Anything could be a key until it matches space format (and connector does not do any excessive format validations between tuples/keys and spaces). There is such a thing as "unsupported type for a primary index", but it is not related to connector at all.

@oleg-jukovec
Copy link
Contributor

oleg-jukovec commented Oct 19, 2022

In other words, there is no such thing as "unsupported type of a request key".

I get an idea with an unsupported MessagePack extension ID. Tarantool should return the error in this case.

@DifferentialOrange
Copy link
Member Author

In other words, there is no such thing as "unsupported type of a request key".

I get an idea with an unsupported MessagePack extension ID. Tarantool should return the error in this case.

Yeah, it should be like this. But it is still not related to this patch. This patch is simply about removing unnecessary type checkup. I think it was here for Tarantool 1.5

@DifferentialOrange DifferentialOrange merged commit 318b91f into master Oct 21, 2022
@DifferentialOrange DifferentialOrange deleted the DifferentialOrange/gh-240-extention-type-keys branch October 21, 2022 11:58
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.

Extention types as keys
3 participants