Issue 5757

Title
Make better index
Priority
feature
Status
resolved
Nosy list
albertca, ced, jcavallo, nicoe, pokoli, reviewbot, roundup-bot, yangoon
Assigned to
ced
Keywords
review

Created on 2016-07-27.19:58:53 by ced, last changed 1 month ago by roundup-bot.

Files

File name Uploaded Type Details
unnamed albertca, 2016-07-29.17:07:18 text/plain view
unnamed albertca, 2016-07-29.17:03:20 text/plain view

Messages

New changeset be7442b6f236 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/tryton-env/rev/be7442b6f236
New changeset 275f29e2379d by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/trytond/rev/275f29e2379d
New changeset c09ae2cdcd67 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/web_user/rev/c09ae2cdcd67
New changeset 52af06891189 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/web_shop_shopify/rev/52af06891189
New changeset df2db6e7f5a0 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/web_shop/rev/df2db6e7f5a0
New changeset 44f15b6e43b2 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/user_role/rev/44f15b6e43b2
New changeset 46b500b02c53 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/timesheet_cost/rev/46b500b02c53
New changeset 149b439baada by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/timesheet/rev/149b439baada
New changeset c0ab3bf31505 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_supply_day/rev/c0ab3bf31505
New changeset 711b4a72814b by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_supply/rev/711b4a72814b
New changeset f2784ad8f2c3 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_quantity_issue/rev/f2784ad8f2c3
New changeset 1e09356f645c by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_quantity_early_planning/rev/1e09356f645c
New changeset 22c8b22976e2 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_product_location/rev/22c8b22976e2
New changeset 8a8e2128abd9 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_package/rev/8a8e2128abd9
New changeset 18f6e733bef9 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_lot/rev/18f6e733bef9
New changeset 51308a390749 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_location_move/rev/51308a390749
New changeset fecb9c2e982a by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_inventory_location/rev/fecb9c2e982a
New changeset 7fd5975b9b15 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock_forecast/rev/7fd5975b9b15
New changeset 6f34b6ded128 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/stock/rev/6f34b6ded128
New changeset 45005ee0175a by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_supply_drop_shipment/rev/45005ee0175a
New changeset e667190f1e07 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_subscription_asset/rev/e667190f1e07
New changeset 0b8ca18964dd by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_subscription/rev/0b8ca18964dd
New changeset 995873d1a503 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_shipment_grouping/rev/995873d1a503
New changeset 90bbd37a63ff by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_promotion_coupon/rev/90bbd37a63ff
New changeset 78ef773390fa by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_promotion/rev/78ef773390fa
New changeset e027e2640a20 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_product_recommendation_association_rule/rev/e027e2640a20
New changeset fdb8b3231fe6 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_product_customer/rev/fdb8b3231fe6
New changeset 75b8b81f9e24 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_price_list/rev/75b8b81f9e24
New changeset 6f21f145a0f6 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_point/rev/6f21f145a0f6
New changeset 3b2835b38d07 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_opportunity/rev/3b2835b38d07
New changeset f6a1f2cd3845 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_invoice_grouping/rev/f6a1f2cd3845
New changeset 4d3eb8f4a7b7 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_gift_card/rev/4d3eb8f4a7b7
New changeset 026649a2f753 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_extra/rev/026649a2f753
New changeset 45193366c468 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_complaint/rev/45193366c468
New changeset 4abc73ff5b0d by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_blanket_agreement/rev/4abc73ff5b0d
New changeset 7ce9ae74a136 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_amendment/rev/7ce9ae74a136
New changeset 67f46b6b9293 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale_advance_payment/rev/67f46b6b9293
New changeset 87d018dd40a6 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/sale/rev/87d018dd40a6
New changeset a84506caae53 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_requisition/rev/a84506caae53
New changeset e54866b2727b by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_request_quotation/rev/e54866b2727b
New changeset 4b2579af7316 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_request/rev/4b2579af7316
New changeset c313145a8bb2 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_price_list/rev/c313145a8bb2
New changeset b25681ceeb4f by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_invoice_line_standalone/rev/b25681ceeb4f
New changeset 955cc110ac8c by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_blanket_agreement/rev/955cc110ac8c
New changeset 9d3ea3a3586c by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase_amendment/rev/9d3ea3a3586c
New changeset a246d4950c8f by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/purchase/rev/a246d4950c8f
New changeset abba0a89b3c1 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/project_revenue/rev/abba0a89b3c1
New changeset 5aecfa7faeae by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/project_plan/rev/5aecfa7faeae
New changeset 3a0434e3d4cd by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/project_invoice/rev/3a0434e3d4cd
New changeset a54b2e980d83 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/project/rev/a54b2e980d83
New changeset 24aa9865c54e by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/production_work/rev/24aa9865c54e
New changeset 02d7e0487d09 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/production_routing/rev/02d7e0487d09
New changeset d1d89af659d3 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/production_outsourcing/rev/d1d89af659d3
New changeset dd5f9ab0b15e by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/production/rev/dd5f9ab0b15e
New changeset 3035e14a0bc6 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product_price_list/rev/3035e14a0bc6
New changeset 7d91f3a44bca by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product_kit/rev/7d91f3a44bca
New changeset 472be8f503ad by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product_image/rev/472be8f503ad
New changeset 3310bb86ba3b by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product_classification_taxonomic/rev/3310bb86ba3b
New changeset 0e6388023e13 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product_classification/rev/0e6388023e13
New changeset 0722f7a1bf45 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product_attribute/rev/0722f7a1bf45
New changeset fa8b0cc88247 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/product/rev/fa8b0cc88247
New changeset ca0cd6fcd41d by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/party_relationship/rev/ca0cd6fcd41d
New changeset c0d49f1c6391 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/party/rev/c0d49f1c6391
New changeset 32880f8ede99 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/notification_email/rev/32880f8ede99
New changeset dda2f05a83cd by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/marketing_email/rev/dda2f05a83cd
New changeset 979429edf5b3 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/marketing_automation/rev/979429edf5b3
New changeset 4a4f957f9e6e by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/incoterm/rev/4a4f957f9e6e
New changeset 80ead4cccdac by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/dashboard/rev/80ead4cccdac
New changeset 0d341963f4cf by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/customs/rev/0d341963f4cf
New changeset 676141321e07 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/currency/rev/676141321e07
New changeset 2cd86bb8172e by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/country/rev/2cd86bb8172e
New changeset fa19dbc863a1 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/company/rev/fa19dbc863a1
New changeset e3b14c6a24ab by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/commission/rev/e3b14c6a24ab
New changeset 31b93ab55ecd by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/carrier_weight/rev/31b93ab55ecd
New changeset 1c365704c939 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/carrier_carriage/rev/1c365704c939
New changeset 7153212594a8 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/bank/rev/7153212594a8
New changeset 36171fada0ef by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/authentication_sms/rev/36171fada0ef
New changeset 52843af96d31 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/attendance/rev/52843af96d31
New changeset ffee41c793a2 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/analytic_budget/rev/ffee41c793a2
New changeset 5c89b1563670 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/analytic_account/rev/5c89b1563670
New changeset 2cefada544cf by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_stock_shipment_cost/rev/2cefada544cf
New changeset 26675add0041 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_stock_landed_cost/rev/26675add0041
New changeset aa78d2c53e82 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_statement/rev/aa78d2c53e82
New changeset 32ce93c7fe77 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_product/rev/32ce93c7fe77
New changeset 3f805f5c02d1 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_payment_stripe/rev/3f805f5c02d1
New changeset 53f68e5978bd by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_payment_sepa/rev/53f68e5978bd
New changeset 767b0033afb9 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_payment_clearing/rev/767b0033afb9
New changeset c83414c63fee by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_payment_braintree/rev/c83414c63fee
New changeset d0e5f60cf8f3 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_payment/rev/d0e5f60cf8f3
New changeset 5075864c2326 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_invoice_stock/rev/5075864c2326
New changeset 5db66bdb80c7 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_invoice_defer/rev/5db66bdb80c7
New changeset 512633c84688 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_invoice/rev/512633c84688
New changeset 5ca9b1b466a7 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_es_sii/rev/5ca9b1b466a7
New changeset 79bed88c32ce by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_dunning_fee/rev/79bed88c32ce
New changeset b3c9a61a669b by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_dunning/rev/b3c9a61a669b
New changeset ace59f4c0260 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_credit_limit/rev/ace59f4c0260
New changeset 06a9d2caacab by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_consolidation/rev/06a9d2caacab
New changeset 2e10cc6d79c4 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_budget/rev/2e10cc6d79c4
New changeset cda4766e92ae by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account_asset/rev/cda4766e92ae
New changeset df2d8e96dbe9 by Cédric Krier in branch 'default':
Use declarative index definition for ModelSQL
https://hg.tryton.org/modules/account/rev/df2d8e96dbe9
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-07.18:47:03

Another point that I found is that when unaccent extension is activated, most of the Similarity become pointless. So I'm working on a conditional Unaccent wrapper for index expression.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-07.14:37:51
On 2022-10-07 14:24, Sergi Almacellas Abellana wrote:
> El 7/10/22 a les 14:17, Cédric Krier ha escrit:
> > I'm wondering if we should create a Similarity() index for the `_rec_name` field if it exists and is not a Function field. This is because `search_rec_name` is the most common query that will be done.
> 
> Makes sense for me as far as the index is not duplicated. For example we already have an index over `number` field and `_rec_name` is set to `number` we just need to create a single index.

_sql_indexes is a set so normally the indexes are unique.
But I just found that the hash method does not provide the same value
for equal Index.
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2022-10-07.14:24:13

El 7/10/22 a les 14:17, Cédric Krier ha escrit:

I'm wondering if we should create a Similarity() index for the _rec_name field if it exists and is not a Function field. This is because search_rec_name is the most common query that will be done.

Makes sense for me as far as the index is not duplicated. For example we already have an index over number field and _rec_name is set to number we just need to create a single index.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-07.14:17:45

I'm wondering if we should create a Similarity() index for the _rec_name field if it exists and is not a Function field. This is because search_rec_name is the most common query that will be done.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-06.12:16:54

For the record, I changed how we name the indexes. Instead of being based on the Index definition, it is based on the query to create the index in the database (and include the table name to distinct 2 similar indexes on 2 different tables). The goal is that if we improve the index translators later and that an existing index with the same definition is indeed better suited by another translator, it must have a different name to be updated (created/dropped).

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-05.19:18:45

In the last version, I ensured that the index defined on a ModelSQL is always created on its table by using the TableHandler to create the indexes (like we do for constraint).
Also now any former index with the pattern idx_* or *_index are dropped if they are no more defined. This will ease the maintenance and ensure to always have the latest setup of indexes. But it will still be possible to customize the database by creating custom indexes manually and give them a different name.

Now it is ready for testing.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-10-03.19:27:35

I'm working on removing the select=True in all modules and here are the rules I try to follow when define an Index:

  • for SQL queries that are build in the module (using as most expression as possible)
  • for name, code, number and reference for Similarity
  • for MPTT left and right for Range

also I always try to set first the column/expression which have the most different values.

I do not define indexes for:

  • active nor company fields (because they are not enough discriminatory)
  • on boolean nor selection field (except if it is a partial)
  • for xxx2Many because they are defined automatically
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-03-01.09:41:24

For me it is a no go as long as the index definition is not designed like the Constraint in ModelSQL.

Author: [hidden] (nicoe) Tryton committer
Date: 2021-03-01.09:32:52
* Cédric Krier  [2021-02-27 14:32 +0100]: 

>> >The Model definition can not depend on the backend.
>>
>> From my point of view the Model definition does not depend on the
>> backend even with this addition.
>
>It is as soon as you store on it an object that are different
>depending on the backend.

What I mean by definition is what you read.
What you mean is what is executed.

>> Because it is only when the definition is realized (ie executed), that
>> the index it contains turn into their implementations.
>
>No because the class which store the index object is different per
>backend. It is not the execution that is different.

The class is different as soon as the python interpreter has executed
the __setup__. From my POV, it is the execution that is different.

>> >> >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.
>> >
>> >There is no point to have arguments that depend on the backend. They can
>> >never be used when defining a Model because it does not depend on the
>> >backend.
>>
>> In a perfect world there would be no index information in the models
>> and this work would be left to DBAs.
>
>I do not agree with this statement. The DBA is the last resort but
>developer create queries that must be optimized by indexes. So it is
>the developer who knows what index must be created.

It would be a priori knowledge that have to be confronted to reality.

So if we really want purity we shouldn't rely on those and let the
reality speak because in the end we don't know.

>> So since we are compromising with the purity of the model we might as
>> well make this explicit.
>
>I do not want to compromise on purity. Model must never depend directly
>on the backend.

We compromise on purity on a lot of things: using lists instead of
sets in a lot of places, the TableHandler class which could be split,
the 2 phase commit protocol, in the GTK client we do not adhere to
GTK's way of doing stuffs in some places, etc.

And it's not an issue, if we have good reasons to do so. And btw
purity could be restored by clearing the list after it has done its
job (although I don't see the point in doing it so I consider it a bad
idea).

>> >> First I think that the TableHandler class is already way too big. It's
>> >> why I chose to make another class.
>> >
>> >I do not think it simplify anything to move two methods from one class
>> >to another in the same file.
>>
>> At least it's a start. Having those backend information structured
>> instead of having everything packed in the same class seems like a
>> good idea.
>
>I see no improvement there, just backward compatibility breakage.

Yeah sometimes people update their code when there is a release. It's
not a big deal.

Moreover it will break anyway because we can not keep the same
signature for the function handling the index.

>Also with this separation we loose the optimisation that prevent to run
>index creation query if the index already exist. Of course there is the
>"IF NOT EXISTS" but it is an extra query. (Now we could argue that
>retrieving the indexes can be complex).

The database is probably way better than us at managing this also.

>But as index is indeed a manipulation of the table (index can not be
>cross-table), it is logical that the table handler handles them.

I don't say that it does something else than manipulating the table.
What I say is that there's something like 20 public methods it could
be way more simple.

>> >But I really think it is not good to have to define the method to use
>> >but we must define which properties we need for the index and the
>> >backend should choose the type that works the best.
>>
>> I don't think that creating another abstraction will help. It will
>> just add another indirection and the mapping between our "langage" and
>> the different indexes implementation will be awkward.
>
>So with such reasoning let's just throw away the ORM, it is just an
>indirection to SQL.

You're comparing apples and pears. The ORM is a central piece of the
framework but indeed it's not a perfect mapping and that's why there
is python-sql.

>This is precisely the added value of the framework to provide a good
>abstract to such thing.

What is the added value of such abstraction? What use case does it
solve?

>> Moreover it will add another point where beginners will have to learn
>> how we do stuffs and make the learning curve steeper.
>
>I do not see why a beginner will know better the specific type of
>indexes of PostgreSQL and in which case which one should be used.

Because he might know already SQL but not Tryton.

>> >> >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?
>> >
>> >Indeed, I can see this kind of operations:
>> >
>> >    - equality
>> >    - range (<, >, etc)
>> >    - begin pattern
>> >    - 2d or vector
>> >    - multi (inverted)
>> >
>> >For postgresql it would result in:
>> >
>> >    ['equality'] -> HASH
>> >    ['equality', 'range'] -> BTREE
>> >    ['begin pattern'] -> BTREE
>> >    ['vector'] -> GIST
>> >    ['multi'] -> GIN
>>
>> I think this is trying to be more clever than what the feature
>> requires.
>
>This is the added value of the framework.

As said, I don't think it's the role of Tryton to add value wrt the
indexes. Our strength is somewhere else.

>Probably the naming and definition must be refined. We could also have
>good default value depending of the type column used.

And it's even more code for something that could be simple. With more
code comes more bugs. Moreover if there is a default how will it work
when it will be an index on multiple columns each with different
defaults?

I don't think we could / should do this.

>> >Now if we have ['range', 'vector'], this may be difficult to choose.
>> >We have three options:
>> >
>> >    - create no index
>> >    - create multi index for each group of cases
>> >    - create one index as best effort
>> >
>> >I think we should do the last one, just find the type of index that
>> >provide the most of operations (+ internal preferences for equality
>> >ex: we prefer BTREE over HASH).
>>
>> This is precisely the kind of difficulty I want to avoid so that we
>> don't have to make a guess. If we let the people decide according to
>> their knowledge of their Model than this question is not there at all.
>
>Again this is the added value otherwise we just let developer create the
>indexes themselves.

Indeed it's somehow the idea: allow to create the indexes themselves
while at the same time being kind of database independent.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-02-27.14:32:03
On 2021-02-27 13:31, Nicolas Évrard wrote:
> >On 2021-02-27 11:12, 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.
> >
> >It just makes the object unpractical.
> >The Model definition can not depend on the backend.
> 
>  From my point of view the Model definition does not depend on the
> backend even with this addition.

It is as soon as you store on it an object that are different depending
on the backend.

> Because it is only when the definition is realized (ie executed), that
> the index it contains turn into their implementations.

No because the class which store the index object is different per
backend. It is not the execution that is different.

> >It must be the same no matter the backend because it is an
> >abstraction. Otherwise a model is linked to a specific backend.
> 
> The model is linked to the specific backend only when the code is
> executed. It's a transient state.

It is not when the code is executed, it is directly at the __setup__.
So yes it is a transient state on the model and this is precisely why it
is a bad design. Model must be static.

> >> >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.
> >
> >There is no point to have arguments that depend on the backend. They can
> >never be used when defining a Model because it does not depend on the
> >backend.
> 
> In a perfect world there would be no index information in the models
> and this work would be left to DBAs.

I do not agree with this statement. The DBA is the last resort but
developer create queries that must be optimized by indexes. So it is the
developer who knows what index must be created.
The DBA is there for the unattended queries.

> So since we are compromising with the purity of the model we might as
> well make this explicit.

I do not want to compromise on purity. Model must never depend directly
on the backend.

> >> >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.
> >
> >I do not think it simplify anything to move two methods from one class
> >to another in the same file.
> 
> At least it's a start. Having those backend information structured
> instead of having everything packed in the same class seems like a
> good idea.

I see no improvement there, just backward compatibility breakage.
Also with this separation we loose the optimisation that prevent to run
index creation query if the index already exist. Of course there is the
"IF NOT EXISTS" but it is an extra query. (Now we could argue that
retrieving the indexes can be complex).
But as index is indeed a manipulation of the table (index can not be
cross-table), it is logical that the table handler handles them.

> >But I really think it is not good to have to define the method to use
> >but we must define which properties we need for the index and the
> >backend should choose the type that works the best.
> 
> I don't think that creating another abstraction will help. It will
> just add another indirection and the mapping between our "langage" and
> the different indexes implementation will be awkward.

So with such reasoning let's just throw away the ORM, it is just an
indirection to SQL.
This is precisely the added value of the framework to provide a good
abstract to such thing.

> Moreover it will add another point where beginners will have to learn
> how we do stuffs and make the learning curve steeper.

I do not see why a beginner will know better the specific type of
indexes of PostgreSQL and in which case which one should be used.
The framework must add value!

> >> >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?
> >
> >Indeed, I can see this kind of operations:
> >
> >    - equality
> >    - range (<, >, etc)
> >    - begin pattern
> >    - 2d or vector
> >    - multi (inverted)
> >
> >For postgresql it would result in:
> >
> >    ['equality'] -> HASH
> >    ['equality', 'range'] -> BTREE
> >    ['begin pattern'] -> BTREE
> >    ['vector'] -> GIST
> >    ['multi'] -> GIN
> 
> I think this is trying to be more clever than what the feature
> requires.

This is the added value of the framework.
Probably the naming and definition must be refined. We could also have
good default value depending of the type column used.

> >Now if we have ['range', 'vector'], this may be difficult to choose.
> >We have three options:
> >
> >    - create no index
> >    - create multi index for each group of cases
> >    - create one index as best effort
> >
> >I think we should do the last one, just find the type of index that
> >provide the most of operations (+ internal preferences for equality
> >ex: we prefer BTREE over HASH).
> 
> This is precisely the kind of difficulty I want to avoid so that we
> don't have to make a guess. If we let the people decide according to
> their knowledge of their Model than this question is not there at all.

Again this is the added value otherwise we just let developer create the
indexes themselves.
Author: [hidden] (nicoe) Tryton committer
Date: 2021-02-27.13:31:57
* Cédric Krier  [2021-02-27 11:45 +0100]: 
>
>Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
>On 2021-02-27 11:12, 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.
>
>It just makes the object unpractical.
>The Model definition can not depend on the backend.

 From my point of view the Model definition does not depend on the
backend even with this addition.

Because it is only when the definition is realized (ie executed), that
the index it contains turn into their implementations.

>It must be the same no matter the backend because it is an
>abstraction. Otherwise a model is linked to a specific backend.

The model is linked to the specific backend only when the code is
executed. It's a transient state.

>> >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.
>
>There is no point to have arguments that depend on the backend. They can
>never be used when defining a Model because it does not depend on the
>backend.

In a perfect world there would be no index information in the models
and this work would be left to DBAs.

Indexes are not modeling information just performance enhancers. So
since we are compromising with the purity of the model we might as
well make this explicit.

>> >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.
>
>I do not think it simplify anything to move two methods from one class
>to another in the same file.

At least it's a start. Having those backend information structured
instead of having everything packed in the same class seems like a
good idea.

>> 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 goal is to have an abstraction on index definition. This is not an
>abstraction if you have to care about which parameter are used by which
>backend etc.
>
>> >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.
>
>SQLite supports only one type of index. I think it is a kind of btree so
>it works for equality and sort.
>But I really think it is not good to have to define the method to use
>but we must define which properties we need for the index and the
>backend should choose the type that works the best.

I don't think that creating another abstraction will help. It will
just add another indirection and the mapping between our "langage" and
the different indexes implementation will be awkward.

Moreover it will add another point where beginners will have to learn
how we do stuffs and make the learning curve steeper.

>> >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?
>
>Indeed, I can see this kind of operations:
>
>    - equality
>    - range (<, >, etc)
>    - begin pattern
>    - 2d or vector
>    - multi (inverted)
>
>For postgresql it would result in:
>
>    ['equality'] -> HASH
>    ['equality', 'range'] -> BTREE
>    ['begin pattern'] -> BTREE
>    ['vector'] -> GIST
>    ['multi'] -> GIN

I think this is trying to be more clever than what the feature
requires.

>Now if we have ['range', 'vector'], this may be difficult to choose.
>We have three options:
>
>    - create no index
>    - create multi index for each group of cases
>    - create one index as best effort
>
>I think we should do the last one, just find the type of index that
>provide the most of operations (+ internal preferences for equality
>ex: we prefer BTREE over HASH).

This is precisely the kind of difficulty I want to avoid so that we
don't have to make a guess. If we let the people decide according to
their knowledge of their Model than this question is not there at all.
Author: [hidden] (albertca)
Date: 2021-02-27.11:53:13
Missatge de Cédric Krier <bugs@tryton.org> del dia dv., 26 de febr. 2021 a
les 23:11:

>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> On 2021-02-26 23:02, Albert Cervera i Areny wrote:
> > I don't seem to find a way to easily remove an index created by one of
> the core modules. Am I right?
> >
> > I think we need a mechanism to allow removing the index or that the
> trytond-admin --all does not rebuild it.
>
> I do not think so. Just like we can not remove required, there is no
> point to remove an index. Even less if we create more carefully default
> indexes.
>

"Required" is very different from "Index" because the former is part of the
business logic and constraints. Tryton may even have undesirable behaviour
if a required constraint is removed.

Index influences performance "only" which can differ from one database to
another, simply because of its size or the amount of records on one table
or another.

>
> ______________________________________
> Tryton issue tracker <bugs@tryton.org>
> <https://bugs.tryton.org/issue5757>
> ______________________________________
>

-- 

<https://www.nan-tic.com>

Albert Cervera

Tel. (+34) 935 531 803

<https://www.nan-tic.com/en/news/>  <http://twitter.com/nan_tic>
<https://www.linkedin.com/company/nan-tic/>

Pot consultar la informació sobre el tractament que realitzem de les dades
de caràcter personal de conformitat amb l’article 11 LOPD aquí
<https://www.nan-tic.com/lopd>.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-02-27.11:45:03
On 2021-02-27 11:12, 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.

It just makes the object unpractical.
The Model definition can not depend on the backend. It must be the same
no matter the backend because it is an abstraction. Otherwise a model is
linked to a specific backend.

> >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.

There is no point to have arguments that depend on the backend. They can
never be used when defining a Model because it does not depend on the
backend.

> >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.

I do not think it simplify anything to move two methods from one class
to another in the same file.

> 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 goal is to have an abstraction on index definition. This is not an
abstraction if you have to care about which parameter are used by which
backend etc.

> >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.

SQLite supports only one type of index. I think it is a kind of btree so
it works for equality and sort.
But I really think it is not good to have to define the method to use
but we must define which properties we need for the index and the
backend should choose the type that works the best.

> >> - 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.

We must work the other way, define a minimal requirement and then
expand.

> >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?

Indeed, I can see this kind of operations:

    - equality
    - range (<, >, etc)
    - begin pattern
    - 2d or vector
    - multi (inverted)

For postgresql it would result in:

    ['equality'] -> HASH
    ['equality', 'range'] -> BTREE
    ['begin pattern'] -> BTREE
    ['vector'] -> GIST
    ['multi'] -> GIN

Now if we have ['range', 'vector'], this may be difficult to choose. We
have three options:

    - create no index
    - create multi index for each group of cases
    - create one index as best effort

I think we should do the last one, just find the type of index that
provide the most of operations (+ internal preferences for equality ex:
we prefer BTREE over HASH).
Author: [hidden] (nicoe) Tryton committer
Date: 2021-02-27.11:12:00
* 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?
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-02-27.00:24:04
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.
Indeed by looking at syntax of both backends they share almost the same
syntax/parameters.
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.
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.

> - Allowing to define index on expression (works in postgres, will fail silently in SQLite)

Normally SQLite support some expression: https://sqlite.org/expridx.html
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.
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.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-02-26.23:47:03
On 2021-02-26 23:23, Jean CAVALLO wrote:
> I am pointing this out because that is something some of our customers' DBAs asked us about, and I think it is a valid use case for being able to remove a core index.

This is too specific to require special management that breaks the
modularity of Tryton.
For this case I would just edit the existing index.
Author: [hidden] (jcavallo)
Date: 2021-02-26.23:23:54

I think we need a mechanism to allow removing the index or that the trytond-admin --all does not rebuild it.

I do not think so. Just like we can not remove required, there is no
point to remove an index. Even less if we create more carefully default
indexes.

The use case for this would be, as I mentionned here[1], when a custom module adds a composite index that can be used in place of a standard index (for instance, a move / account index on account.move.line can replace a "pure" move index).

Though the standard index may still be useful (because it will be somehow lighter, so faster to load), we also may have good reasons to see it gone (for improved update / create performance, or disk on the DB side).

I am pointing this out because that is something some of our customers' DBAs asked us about, and I think it is a valid use case for being able to remove a core index.

[1] https://bugs.tryton.org/issue5757#msg27438

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-02-26.23:11:05
On 2021-02-26 23:02, Albert Cervera i Areny wrote:
> I don't seem to find a way to easily remove an index created by one of the core modules. Am I right?
> 
> I think we need a mechanism to allow removing the index or that the trytond-admin --all does not rebuild it.

I do not think so. Just like we can not remove required, there is no
point to remove an index. Even less if we create more carefully default
indexes.
Author: [hidden] (jcavallo)
Date: 2021-02-26.23:10:12

I don't seem to find a way to easily remove an index created by one of the core modules. Am I right?

I agree with this.

I think index creation should be delayed until the end of the upgrade process, so that a sub-module can disable an index before it is actually created in the database.

Author: [hidden] (albertca)
Date: 2021-02-26.23:02:38

I don't seem to find a way to easily remove an index created by one of the core modules. Am I right?

I think we need a mechanism to allow removing the index or that the trytond-admin --all does not rebuild it.

Author: [hidden] (nicoe) Tryton committer
Date: 2021-02-26.18:59:01

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__
  • Allowing to define index on expression (works in postgres, will fail silently in SQLite)
  • Allow to specify some backend specific customization of the index

I submit the code review before reviewing all the modules.
It would be nice to have some numbers about the index that are used in the real world also.

Author: [hidden] (albertca)
Date: 2019-05-04.00:43:08
Some more indexes we found unused (or almost unused):

account_move_state_index
purchase_request_uom_index
sale_sale_origin_index
party_contact_mechanism_active_index
Author: [hidden] (albertca)
Date: 2019-05-03.16:32:06
Checking several customer databases shows that the "type" fields in sale.line, purchase.line, and account.invoice.line are never used.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-04-18.11:56:56
Here is a post about finding missing indexes and unused indexes: https://www.geekytidbits.com/performance-tuning-postgres/#index-tuning

Maybe we could have a script that will allow us to collect statistics about Tryton databases which will allow us to better tune the default indexes.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2016-07-29.20:10:20
On 2016-07-29 19:26, Sergi Almacellas Abellana wrote:
> Why not adding a Python object like Contraint[1] to represent indexes, so anybody can add it's own indexes types if needed?

As I said index creation is not standardized by any SQL standard. So they
are very specific to the database. Also it is only the "method" (B-tree,
hash, GiST, and GIN for PostgreSQL) that could be missing.
But as Albert said, creating tailored index is the job of DB
administrator depending on the usage and the setup. There is no
restriction from trytond to create manually such indexes as far as the
name is not in collision with the trytond schema name.
Also it is still possible to execute custom SQL query in ModelStorage.__register__
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2016-07-29.19:26:13
Why not adding a Python object like Contraint[1] to represent indexes, so anybody can add it's own indexes types if needed?

I think it's the simpliest and more flexible solution. 

[1] http://hg.tryton.org/trytond/file/241eb36cdb45/trytond/model/modelsql.py#l28
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2016-07-29.17:35:05
On 2016-07-29 17:03, Albert Cervera i Areny wrote:
> 2016-07-28 22:51 GMT+02:00 Cédric Krier <issue_tracker@tryton.org>:
> > Cédric Krier <cedric.krier@b2ck.com> added the comment:
> >
> > I propose instead of overriding __register__ and use TableHandler to add
> > the indexes, we could have an attribute on the ModelSQL to list the indexes
> > '_sql_indexes' just like we have '_sql_constraint'.
> > The syntax could be:
> >
> > _sql_indexes = [
> >     ['column1', 'column2'],
> >     ['column1'],
> >     …
> >     ]
> >
> 
> This misses the type of index which is important, and I guess it will be
> even more important in the future. The storage part of PostgreSQL is
> becoming more and more extensible and I think we can expect more index
> types to be added in the future.

Indeed there is no standard for index. So I think we should just keep it
simple for now. The TableHandler could be improved if needed to guess
the best index depending on the column types. It should be there because
the index method depend on the backend.

> Also, we should see how you'd manage more complex indexes. For example one
> can create an index on an expression, not only a field.

I think this is out of the scope of the general design of model.
But I guess the TableHandler could be improved to allow using
expression.

> My feeling is that indexes should be something that is not part of the
> module and is more a DBA job.

Yes but Tryton must come with a good default setup for standard usage.

> If we feel we should help our users, we could create an automated tool that
> assists on that though I guess there is already something available.

This is out of the scope of Tryton. I'm pretty sure it already exist for
major database backend.
Author: [hidden] (albertca)
Date: 2016-07-29.17:07:19
2016-07-29 9:46 GMT+02:00 Jean CAVALLO <issue_tracker@tryton.org>:

>
> Jean CAVALLO <jean.cavallo@coopengo.com> added the comment:
>
> Agreed. We could also add some warnings if there are possible duplicate
> indexes, for instance in your example, the index only on column1 may
> not be necessary since the composite index can be used for queries on
> column1.
>
> @ Albert :
> One question though, I consider that almost all "reverse" fields of O2M
> fields should be indexed since every read on the "main" record will
> become slow after a time (e.g. account.move->lines). What do you think ?
>

I agree with Cédric here: it depends. My point is that "thinking" may not
bring us anywhere and only analysis will do. And as that depends on the
amount of data on each table, it can only happen outside of the module.

>
> ----------
> nosy: +jcavallo
>
> _______________________________________________
> Tryton issue tracker <issue_tracker@tryton.org>
> <https://bugs.tryton.org/issue5757>
> _______________________________________________
>
Author: [hidden] (albertca)
Date: 2016-07-29.17:03:21
2016-07-28 22:51 GMT+02:00 Cédric Krier <issue_tracker@tryton.org>:

>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> I propose instead of overriding __register__ and use TableHandler to add
> the indexes, we could have an attribute on the ModelSQL to list the indexes
> '_sql_indexes' just like we have '_sql_constraint'.
> The syntax could be:
>
> _sql_indexes = [
>     ['column1', 'column2'],
>     ['column1'],
>     …
>     ]
>

This misses the type of index which is important, and I guess it will be
even more important in the future. The storage part of PostgreSQL is
becoming more and more extensible and I think we can expect more index
types to be added in the future.

Also, we should see how you'd manage more complex indexes. For example one
can create an index on an expression, not only a field.

My feeling is that indexes should be something that is not part of the
module and is more a DBA job.

If we feel we should help our users, we could create an automated tool that
assists on that though I guess there is already something available.

>
> _______________________________________________
> Tryton issue tracker <issue_tracker@tryton.org>
> <https://bugs.tryton.org/issue5757>
> _______________________________________________
>
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2016-07-29.12:35:06
On 2016-07-29 11:41, Jean CAVALLO wrote:
> They may be useful in some cases, I just wanted to point out that in the
> particular case you gave they do not.
> 
> See http://dba.stackexchange.com/questions/27481/is-a-composite-index-also-good-for-queries-on-the-first-field
> and http://dba.stackexchange.com/questions/6115/working-of-indexes-in-postgresql
> 
> The scheduler will use the composite index for the first column and it
> will be as efficient as having an extra index only on this column.
> This allows to create less indexes and save some space :)

Not necessary, such composite index could be bigger and so more
expensive to load. So as my previous link shows, there are legitimate
use case to have "duplicate" indexes so a warning will be annoying.

> Regarding the reverse, I agree it should not be a rule, but IMHO in
> 90-95% of the case it should be set. Even on small tables, where the
> cost of adding the index is minimal.

No, index on small table is just a waste of resources.
Author: [hidden] (jcavallo)
Date: 2016-07-29.11:41:06
They may be useful in some cases, I just wanted to point out that in the
particular case you gave they do not.

See http://dba.stackexchange.com/questions/27481/is-a-composite-index-also-good-for-queries-on-the-first-field
and http://dba.stackexchange.com/questions/6115/working-of-indexes-in-postgresql

The scheduler will use the composite index for the first column and it
will be as efficient as having an extra index only on this column.
This allows to create less indexes and save some space :)

Regarding the reverse, I agree it should not be a rule, but IMHO in
90-95% of the case it should be set. Even on small tables, where the
cost of adding the index is minimal.

For active / company, I think they are not often used since those are
not very discriminative columns. Usually there is only one company,
and almost all records are active...
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2016-07-29.09:59:20
Indeed "duplicate" indexes can be useful, see https://www.postgresql.org/docs/current/static/indexes-bitmap-scans.html

For the reverse, I do not think we can make a rule because it depends on the size of the table. For small table, indexes are always useless.

Another thing we should think about is the two fields: active and company. They are added automatically to filter clauses but as far as I see indexes on those columns are almost never used.
Author: [hidden] (jcavallo)
Date: 2016-07-29.09:46:43
Agreed. We could also add some warnings if there are possible duplicate
indexes, for instance in your example, the index only on column1 may
not be necessary since the composite index can be used for queries on
column1.

@ Albert :
One question though, I consider that almost all "reverse" fields of O2M
fields should be indexed since every read on the "main" record will
become slow after a time (e.g. account.move->lines). What do you think ?
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2016-07-28.22:51:44
I propose instead of overriding __register__ and use TableHandler to add the indexes, we could have an attribute on the ModelSQL to list the indexes '_sql_indexes' just like we have '_sql_constraint'.
The syntax could be:

_sql_indexes = [
    ['column1', 'column2'],
    ['column1'],
    …
    ]
Author: [hidden] (albertca)
Date: 2016-07-28.01:16:59
+1 in general.

Yet I would only keep very, very few and very obvious indexes (date on account moves is the only one that comes to mind right now). Not even following the statistics given by some installs. Because, just like it happens with most modules, on each install we should tend to add indexes but not remove them. The same we try that fields should be added by modules but not removed.

For me, indexes should be very install-specific and other tools can be used to determine which are necessary.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2016-07-27.19:58:51
Indexes on table have a cost on update. On postgresql, all indexes must be updated when a row is updated even if the update does not change the indexed columns (see [1]). I think we have made the creation of index (with select=True) to easy and so we probably have too much indexes.
My proposal is to remove the attribute 'select' on the fields and to force developer to write code in __setup__ using the TableHandler.
This change will force us to review all the current index (and probably remove many) but also it will make us think if we do not need a more complex indexes (instead of simple column).

Later we could improve the TableHandler.index_action to allow to specify which kind of indexes (btree, trigram etc.) depending on the backend.

For the decision about keeping an index, it will be good to have statistics of production databases. Such statistics can be retrieved from PostgreSQL using this query:

    SELECT * FROM pg_stat_all_indexes;


[1] https://eng.uber.com/mysql-migration/
History
Date User Action Args
2022-10-11 01:05:44roundup-botsetmessages: + msg78871
2022-10-11 01:05:30roundup-botsetmessages: + msg78870
2022-10-11 01:05:22roundup-botsetmessages: + msg78869
2022-10-11 01:05:13roundup-botsetmessages: + msg78868
2022-10-11 01:05:04roundup-botsetmessages: + msg78867
2022-10-11 01:04:56roundup-botsetmessages: + msg78866
2022-10-11 01:04:51roundup-botsetmessages: + msg78865
2022-10-11 01:04:47roundup-botsetmessages: + msg78864
2022-10-11 01:04:38roundup-botsetmessages: + msg78863
2022-10-11 01:04:34roundup-botsetmessages: + msg78862

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