Skip to content

Revise indexes subsections #1896

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Mar 16, 2021
Merged

Revise indexes subsections #1896

merged 9 commits into from
Mar 16, 2021

Conversation

Onvember
Copy link
Contributor

@Onvember Onvember commented Mar 4, 2021 •
edited
Loading

@Onvember Onvember requested review from NickVolynkin and veod32 March 4, 2021 14:58
@NickVolynkin
Copy link
Contributor

@Onvember please, rebase this branch on latest

NickVolynkin
NickVolynkin approved these changes Mar 10, 2021
View reviewed changes
olegrok
olegrok reviewed Mar 11, 2021
View reviewed changes
olegrok
olegrok reviewed Mar 11, 2021
View reviewed changes
olegrok
olegrok reviewed Mar 11, 2021
View reviewed changes
olegrok
olegrok reviewed Mar 11, 2021
View reviewed changes
olegrok
olegrok reviewed Mar 11, 2021
View reviewed changes
@olegrok
Copy link

olegrok commented Mar 11, 2021

are there any examples for "exclude_null" feature?

@Onvember
Copy link
Contributor Author

are there any examples for "exclude_null" feature?

will be described soon (#1782)

@Onvember Onvember requested a review from olegrok March 12, 2021 07:45
artur-barsegyan
artur-barsegyan reviewed Mar 12, 2021
View reviewed changes
Copy link
Contributor

@artur-barsegyan artur-barsegyan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Хорошая работа. Теперь один из самых важных разделов документации выглядит понятнее и дружелюбнее.

NickVolynkin
NickVolynkin reviewed Mar 12, 2021
View reviewed changes
doc/reference/reference_lua/box_space/create_index.rst Outdated

The function must be :ref:`persistent <box_schema-func_create_with-body>`
and deterministic.
* The space must have a memtx engine.
Copy link
Contributor

Choose a reason for hiding this comment

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

would a link to memtx page be useful here?

Onvember and others added 2 commits March 12, 2021 17:12
@Onvember @NickVolynkin
Apply suggestions from code review
518e8f3 
Co-authored-by: Nick Volynkin <nick.volynkin@gmail.com>
@Onvember
add changes after review
7248bda
@artur-barsegyan
Copy link
Contributor

artur-barsegyan commented Mar 12, 2021 •
edited
Loading

Я сделал второе ревью и у меня есть ряд замечаний:

  1. В разделе Creating an index нет примера с созданием мультикей индекса, а это довольно популярная задача.
  2. Раздел Index Operations выглядит абсолютно несвязным и не соответствует названию. Я думаю, в нем стоит оставить блок про Select с операторами сравнения и добавить описания того, как удалить индекс и как посмотреть все текущие индексы в спейсе.
  3. Возможно таблицу с фичами индексов стоит вынести в отдельный раздел под названием Index Features. В MongoDB он вынес отдельно и называется Index Properties.

Думаю, что после этих правок раздел станет значительно полезнее.

@Onvember
Copy link
Contributor Author

@artur-barsegyan

  1. В разделе Creating an index нет примера с созданием мультикей индекса, а это довольно популярная задача.

Я оставила в этой секции только две простейшие операции и дала ссылку на более детальное описание создания индексов в яркой желтой рамочке с надписью "Tip", а на странице create_index уже и про multikey рассказывается.

  1. Раздел Index Operations выглядит абсолютно несвязным и не соответствует названию. Я думаю, в нем стоит оставить блок про Select с операторами сравнения и добавить описания того, как удалить индекс и как посмотреть все текущие индексы в спейсе.

Не могу согласиться, но и не настаиваю на том, что эта секция достаточно хороша. Призываю @NickVolynkin и @olegrok для совета.

  1. Возможно таблицу с фичами индексов стоит вынести в отдельный раздел под названием Index Features. В MongoDB он вынес отдельно и называется Index Properties.

Идея неплохая, я тоже поняла, что в одном месте у нас Index features не собраны. Но тянет на отдельную задачу, в которой надо будет как минимум сделать секцию-карту, ну или описать фичи полноценно.
На данный момент таблица находится на своем месте, т.к. я делала акцент на различиях в типах индексов, а не на самих возможностях индексов.

@olegrok olegrok self-requested a review March 15, 2021 07:32
olegrok
olegrok reviewed Mar 15, 2021
View reviewed changes
@olegrok
Copy link

olegrok commented Mar 15, 2021

  1. Раздел Index Operations выглядит абсолютно несвязным и не соответствует названию. Я думаю, в нем стоит оставить блок про Select с операторами сравнения и добавить описания того, как удалить индекс и как посмотреть все текущие индексы в спейсе.

Не могу согласиться, но и не настаиваю на том, что эта секция достаточно хороша. Призываю @NickVolynkin и @olegrok для совета.

Перечитал ещё раз, потом зашел ещё сюда https://www.tarantool.io/ru/doc/2.2/reference/reference_lua/box_index . Информация об итераторах и т.д. дублируется. И может надо действительно переработать эту секцию. Но конкретных шагов не предложу, мне сложно представить себя на месте пользователя, который читает это в первый раз.

@Onvember
Copy link
Contributor Author

We decided to merge this pull request as is and continue our research here #1943

@Onvember Onvember merged commit 88f1983 into latest Mar 16, 2021
@Onvember Onvember deleted the onvember/gh-1871 branch March 16, 2021 14:41
Onvember added a commit that referenced this pull request Mar 25, 2021
@Onvember
Revise sections 'creating an index', 'index operations' and proofread…
30771b1 
… 'create_index()' (#1896)

* small bugfix

* revise content of indexes.rst

* revise subsections in create_index.rst

* Apply suggestions from code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NickVolynkin NickVolynkin NickVolynkin approved these changes

@olegrok olegrok olegrok left review comments

@artur-barsegyan artur-barsegyan artur-barsegyan left review comments

@veod32 veod32 Awaiting requested review from veod32

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[1pt] feedback: space_object:create_index() [5pt] Indexes: revise subsections about index features
4 participants
@Onvember @NickVolynkin @olegrok @artur-barsegyan