-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add Query support #18
Conversation
AlexAUtkin
commented
Dec 4, 2024
•
edited
Loading
edited
- Add Query class
- Rework Exceptions
- Change README.md file generator: now pydoc_markdown
… into feature/add_transaction_support
…s' into feature/add_transaction_support
# Conflicts: # pyreindexer/lib/include/queryresults_wrapper.h # pyreindexer/lib/src/rawpyreindexer.cc # pyreindexer/lib/src/rawpyreindexer.h
|
||
Py_XINCREF(keysList); | ||
|
||
std::vector<reindexer::Variant> keys; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я бы здесь и в других подобных местах предложил VariantArray вместо std::vector<reindexer::Variant>
- чуток сэкономим на аллокациях, да и по конексту этот тип подходит
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Знаешь, я бы тоже. Однако я не понимаю логики использования данного класса. Откроем, например, Query.h\Query.cc: Раз 8 есть код:
VariantArray values;
values.reserve(l.size());
for (auto it = l.begin(); it != l.end(); it++) {
values.emplace_back(*it);
}
Почему нет MarkArray()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Внимательно просмотрел опять весь код. Не вижу ни одного кейса где бы выигрывали. Почти все они заканчиваются на функции:
void QueryWrapper::putKeys(const reindexer::VariantArray& keys) {
ser_.PutVarUint(keys.size());
for (const auto& key : keys) {
ser_.PutVariant(key);
}
}
и что там реально это будет, нет никакой разницы. Аллокация там одна, когда перекладываем из Питон параметров, а дальше const & и всё. Нет ни хранения, ни перекладывания. Заменил, просто короче в API функции смотрится
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
поменял, стало хуже, появились рудименты:
std::vector<std::string> prets(itemPrecepts.begin(), itemPrecepts.end());
item.SetPrecepts(prets); // ToDo after migrate on v.4, do std::move
потому что там нет h_vector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Там в случае VariantArray рассчёт на то, что в принципе не будет аллокаций, если аргументов 1 или 2 - они останутся на стеке (в отличие от создания промежуточного вектора для keys, который аллоцирует в любом случае в момент reserve()).
Но да, с перцептами оно, конечно, не очень выглядит(
# Conflicts: # README.md # pyreindexer/example/main.py # pyreindexer/lib/include/queryresults_wrapper.h # pyreindexer/lib/include/transaction_wrapper.h # pyreindexer/lib/src/rawpyreindexer.cc # pyreindexer/lib/src/rawpyreindexer.h # pyreindexer/lib/src/reindexerinterface.cc # pyreindexer/lib/src/reindexerinterface.h # pyreindexer/query_results.py # pyreindexer/raiser_mixin.py # pyreindexer/rx_connector.py # pyreindexer/tests/tests/test_transaction.py # pyreindexer/transaction.py # readmegen.sh
Внимание! Мержить пока нельзя!!! UPDATE: всё лишнее выделено в отдельную ветку. Мержить можно |