-
Notifications
You must be signed in to change notification settings - Fork 14
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
Support field id in select/pairs conditions #352
Comments
The one reason why it isn't supported yet may be the support of both indexes and fields in conditions. User may try to set index id which will be treated as field id. |
@DifferentialOrange @dkasimovskiy May we better add index id cause we can't use PK without knowing the name? It's so important if you use API like spring-data. There you have methods that use primary key(findById) and it's pain to inject index name upd: I forgot about the get method. Get method fixes our problem, then we don't need select with index id. Sorry |
The main controversy behind this question is "whether id is field id or index id"? On the other hand, we have the same controversy for index/field names already. If you decide that you want that feature nonetheless, we may reconsider it. For now, yeah, you may use get |
However when we deal with spring-data repository.containsById method, we may encounter a problem with unpredictable record size. Receiving huge data over network just for checking for record presence may be problematic, especially in case when containsById is being called frequently. |
@bitgorbovsky I think it's relative to this ticket. Need to create a new one "Add contains method" |
I have no idea what |
Follows #350
Update conditions support field number ids (moreover, due to compatibility with older Tarantool, all non-number identificators are converted to number ones, see
crud/crud/common/utils.lua
Lines 591 to 618 in 3f2db88
I don't remember hearing any user requests about field id support, so I'll put a question label here. On the other hand, I don't see any reasons against adding this (like there were for space id in #255) since we expect space schema to be the same everywhere.
The text was updated successfully, but these errors were encountered: