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

Support field id in select/pairs conditions #352

Open
DifferentialOrange opened this issue Mar 22, 2023 · 6 comments
Open

Support field id in select/pairs conditions #352

DifferentialOrange opened this issue Mar 22, 2023 · 6 comments
Labels
feature A new functionality question Further information is requested

Comments

@DifferentialOrange
Copy link
Member

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

function utils.convert_operations(user_operations, space_format)
local converted_operations = {}
for _, operation in ipairs(user_operations) do
if type(operation[2]) == 'string' then
local field_id
for fieldno, field_format in ipairs(space_format) do
if field_format.name == operation[2] then
field_id = fieldno
break
end
end
if field_id == nil then
return nil, ParseOperationsError:new(
"Space format doesn't contain field named %q", operation[2])
end
table.insert(converted_operations, {
operation[1], field_id, operation[3]
})
else
table.insert(converted_operations, operation)
end
end
return converted_operations
end
). Select/pairs are not, see #241 example.

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.

@DifferentialOrange DifferentialOrange added feature A new functionality question Further information is requested teamE labels Mar 22, 2023
@DifferentialOrange DifferentialOrange changed the title Support field id in select/pairs condition Support field id in select/pairs conditions Mar 22, 2023
@DifferentialOrange
Copy link
Member Author

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.

@ArtDu
Copy link
Contributor

ArtDu commented Feb 5, 2024

@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

@DifferentialOrange
Copy link
Member Author

May we better add index id cause we can't use PK without knowing the name

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

@bitgorbovsky
Copy link

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.

@ArtDu @DifferentialOrange

@ArtDu
Copy link
Contributor

ArtDu commented Feb 11, 2024

@bitgorbovsky I think it's relative to this ticket. Need to create a new one "Add contains method"

@DifferentialOrange
Copy link
Member Author

DifferentialOrange commented Feb 14, 2024

However when we deal with spring-data repository.containsById method, we may encounter a problem with unpredictable record size.

I have no idea what repository.containsById is, but you can always limit crud.select output with first=N option or use crud.count which responds with a single number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new functionality question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants