Message 64906

Author
nicoe
Date
2021-02-27.11:12:00
Message id
64906

Content

* Cédric Krier  [2021-02-27 00:24 +0100]: 
>
>Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
>On 2021-02-26 18:59, Nicolas Évrard wrote:
>> I have made the proposal in the review which is kind of a mix of all the ideas in this issue.
>>
>> - Definition of the index in a specific attribute in \_\_setup\_\_
>
>I do not think it is correct to store a backend specific object on
>the Model.

I don't see a problem with that.
It's not like we could change from one backend to another easily.

>Indeed by looking at syntax of both backends they share almost the
>same syntax/parameters.

SQLite is like the minimum of index creation.
I also looked at MariaDB and Oracle's version of CREATE INDEX (that's
why 'WHERE' is in the kwargs instead of the arguments). The similarity
of the syntax is kind of an artefact of SQLite simplicity.

>I think that some of the PG parameter are too specific to require our
>support like:
>
>    - CONCURRENTLY: not really useful for trytond-admin as it is not
>      supposed to be concurrent
>    - ONLY: not supported by all PG version and only useful for
>      partitioned table which could exist only if the DBA create the
>      schema and so he could create the index also
>    - INCLUDE: could be general argument that is ignored by sqlite
>    - WITH: it seems to me that it is very specific and only DBA could
>      choose them.
>
>So I think we could have an object in modelsql.py like the Constraint
>that store the index definition to be passed to the table handler.

First I think that the TableHandler class is already way too big. It's
why I chose to make another class.

Then the thing is that if we ever support another database there might
by another parameter that become useful (eg FULLTEXT for MariaDB) so I
opted for the strict minimum in the arguments and the rest in the
kwargs rather than a kind of rosetta stone that tries to cover
some use cases. Mainly because having the kwargs prefixed with the
database name allows to know which feature is enabled for which
backend (instead of not supporting it somehow silently).

>The only missing point is to index method to use. We could probably
>use the PG name and just make SQL support only btree and hash.

Methods are there for PG. SQLite do not support those AFAIK.

>> - Allowing to define index on expression (works in postgres, will
>>   fail silently in SQLite)
>
>Normally SQLite support some expression: https://sqlite.org/expridx.html

Indeed, you're right.

>But I think index creation is just a best effort.

>> - Allow to specify some backend specific customization of the index
>
>For me as long as a DBA can modify (drop/create using the same name) it
>for fine tuning, I think we should not need to support too much
>specific.

To be honest I thought that only USING / WHERE might be used sometimes
but since it was not difficult to add the others I thought that I
would do so so that we can decide.

>I would say that only the method is a specific that is good to
>support but it is mainly related to the kind of operation for which
>the index it created.

I don't understand. Are you saying that you don't think the people
should specify the method but they should rather define the operation?
History
Date User Action Args
2021-02-27 11:12:00nicoesetrecipients: + ced, yangoon, albertca, pokoli, jcavallo, reviewbot
2021-02-27 11:12:00nicoelinkissue5757 messages
2021-02-27 11:12:00nicoecreate

Showing 10 items. Show all history (warning: this could be VERY long)