Moodle
  1. Moodle
  2. MDL-33018

Add context index to substantially improve system performance on large PostgreSQL installations

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.2, 2.3
    • Fix Version/s: 2.4
    • Component/s: Performance
    • Labels:
    • Testing Instructions:
      Hide
      1. install moodle on pg before this patch
      2. manually create the index in question:
        CREATE INDEX mdl-context-prepatch ON mdl_context USING btree (path varchar_pattern_ops);
        
      3. upgrade to post patch
      4. verify there are three indexes on mdl_context.path, two with varchar_pattern_ops (one ours and one autogenerated)
      5. Drop your installation and do a new install
      6. verify there are two indexes on mdl_context.path, the second with varchar_pattern_ops
      7. Use the xmldb editor - to add, edit and delete hints on indexes
      8. run phpunit tests
      Show
      install moodle on pg before this patch manually create the index in question: CREATE INDEX mdl-context-prepatch ON mdl_context USING btree (path varchar_pattern_ops); upgrade to post patch verify there are three indexes on mdl_context.path, two with varchar_pattern_ops (one ours and one autogenerated) Drop your installation and do a new install verify there are two indexes on mdl_context.path, the second with varchar_pattern_ops Use the xmldb editor - to add, edit and delete hints on indexes run phpunit tests
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w28_MDL-33018_m24_pgindex2
    • Rank:
      40194

      Description

      At the OU we upgraded to Moodle 2.2.x (from 2.1.x) in April and saw a substantial reduction in performance. Consequently, since then, our systems team have been examining ways to improve performance to regain some of what we lost.

      So far the most successful of these (which appears to have recovered most of the performance decline) is very simple: an additional index on the context table.

      We thought it would be helpful to report this, in case it can be included in standard Moodle (even though this issue is Postgres-specific). It should be noted that although the OU now runs an almost-standard Moodle 2.2.x core, we do use many custom plugins, so these results might not necessarily be applicable to other locations. However in this case it seems likely they probably will be.

      CREATE INDEX mdl_cont_pat2_ix
         ON mdl_context
         USING btree
         (path varchar_pattern_ops);
      

      varchar_pattern_ops is a Postgres-specific keyword that means the index can be used for certain types of pattern-matching search.

      Apparently this is only necessary if your database is set to a non-C locale, such as the UTF-8 locale (in a C locale, the 'normal' index would work). However, UTF-8 locale is probably a standard setting for Moodle users.

      I believe this index is used in addition to the standard index, rather than as a replacement for it.

      After creating this index using a local plugin, we found the following results:

      • On our database server, CPU load (measured over about a few hours before the change and half an hour afterward) dropped sharply from about 4.75 to about 3.0.
      • On our front-end servers, the time taken to serve a course view page dropped sharply from about 0.45 to about 0.3 seconds.

      By the way, neither database server nor the front-end servers are under heavy load at present. (That is to say, there are currently about 10 mdl_log entries per second, so the system isn't idle, but we currently have much more hardware than actually necessary to handle this load.)

      1. .jpg
        61 kB
      2. load-graph.png
        11 kB

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          Thanks a lot!

          Show
          Petr Škoda added a comment - Thanks a lot!
          Hide
          Eric Merrill added a comment -

          We added this to our production system (postgres). Since 2.2, during light loads (~100 active users), our system has always had a system load of ~1.2-2, never lower than one. Since applying this, are down around ~0.1-0.3. In no way scientific, but it seems to have significantly lightened the server load. I believe we have reports for our server on this, if so, I'll try to get ahold of it to post.

          Note if you are applying it to you system before an official patch, you'll likely want to add something to the index name (mdl_cont_pat2_ix) that will make it hightly unlikely to collide with the official moodle name, otherwise I'm pretty sure upgrade will fail.

          Show
          Eric Merrill added a comment - We added this to our production system (postgres). Since 2.2, during light loads (~100 active users), our system has always had a system load of ~1.2-2, never lower than one. Since applying this, are down around ~0.1-0.3. In no way scientific, but it seems to have significantly lightened the server load. I believe we have reports for our server on this, if so, I'll try to get ahold of it to post. Note if you are applying it to you system before an official patch, you'll likely want to add something to the index name (mdl_cont_pat2_ix) that will make it hightly unlikely to collide with the official moodle name, otherwise I'm pretty sure upgrade will fail.
          Hide
          Eric Merrill added a comment -

          I looked for all instances of 'LIKE' in the code, and pretty much all I found was on context.path, everything else seemed to be low use or upgrade code. Also some stuff in some modules, but pretty minimal. So just the one in that is here should be fine I think.

          Show
          Eric Merrill added a comment - I looked for all instances of 'LIKE' in the code, and pretty much all I found was on context.path, everything else seemed to be low use or upgrade code. Also some stuff in some modules, but pretty minimal. So just the one in that is here should be fine I think.
          Hide
          Eric Merrill added a comment -

          Here is the last 24hrs of load averages on our DB machine. Blue line is when we created this index.

          Show
          Eric Merrill added a comment - Here is the last 24hrs of load averages on our DB machine. Blue line is when we created this index.
          Hide
          Tony Levi added a comment -

          Hi,

          We've done this on some production systems too, and while we haven't seen such a large change it does help with certain queries significantly - I certainly haven't seen any negative side effects.

          Show
          Tony Levi added a comment - Hi, We've done this on some production systems too, and while we haven't seen such a large change it does help with certain queries significantly - I certainly haven't seen any negative side effects.
          Hide
          Martin Dougiamas added a comment -

          What about other databases?

          Show
          Martin Dougiamas added a comment - What about other databases?
          Hide
          Sam Marshall added a comment -

          Martin: So far as I am aware other databases don't require a special type of index to use with LIKE, even when the database is set to UTF-8. So they are probably OK, but somebody with more expertise in the other databases might be able to confirm, perhaps Eloy knows?

          By the way, the performance effect of this missing index in Postgres probably depends on the size of the table. I forgot to quote the size of the table in our system when I passed this on initially - it's currently 657,624 rows. So fairly large but not massive.

          Show
          Sam Marshall added a comment - Martin: So far as I am aware other databases don't require a special type of index to use with LIKE, even when the database is set to UTF-8. So they are probably OK, but somebody with more expertise in the other databases might be able to confirm, perhaps Eloy knows? By the way, the performance effect of this missing index in Postgres probably depends on the size of the table. I forgot to quote the size of the table in our system when I passed this on initially - it's currently 657,624 rows. So fairly large but not massive.
          Hide
          Tony Levi added a comment -

          Bit bigger than us; a few I looked at are in the 300 - 500k range, so that might explain some of the larger impact for you...

          Show
          Tony Levi added a comment - Bit bigger than us; a few I looked at are in the 300 - 500k range, so that might explain some of the larger impact for you...
          Hide
          Eric Merrill added a comment -

          I would agree that this is a problem specific to postgest - there is already a index on this column - postgres just offers a special index type because of design choices they made with their indexes.

          Ours is about 550k records.

          Show
          Eric Merrill added a comment - I would agree that this is a problem specific to postgest - there is already a index on this column - postgres just offers a special index type because of design choices they made with their indexes. Ours is about 550k records.
          Hide
          Petr Škoda added a comment - - edited

          The question is should we remove the current index and replace it with this new one? I am asking because this decision affects the internal implementation in DDL layer - either we could add new index hint or completely new index type in install.xml.

          Such as:

                <INDEXES>
                  <INDEX NAME="contextlevel-instanceid" UNIQUE="true" FIELDS="contextlevel, instanceid" NEXT="instanceid"/>
                  <INDEX NAME="instanceid" UNIQUE="false" FIELDS="instanceid" PREVIOUS="contextlevel-instanceid" NEXT="path"/>
                  <INDEX NAME="path" UNIQUE="false" FIELDS="path" HINT="pattern,xxx,yyy" PREVIOUS="instanceid"/>
                </INDEXES>
          

          Implementing just one index type with optional hints seems easier to me because we could add more different hints for different databases in the future. The upgrade would be relatively simple too - if dbfamily then drop index and create new one.

          Show
          Petr Škoda added a comment - - edited The question is should we remove the current index and replace it with this new one? I am asking because this decision affects the internal implementation in DDL layer - either we could add new index hint or completely new index type in install.xml. Such as: <INDEXES> <INDEX NAME= "contextlevel-instanceid" UNIQUE= " true " FIELDS= "contextlevel, instanceid" NEXT= "instanceid" /> <INDEX NAME= "instanceid" UNIQUE= " false " FIELDS= "instanceid" PREVIOUS= "contextlevel-instanceid" NEXT= "path" /> <INDEX NAME= "path" UNIQUE= " false " FIELDS= "path" HINT= "pattern,xxx,yyy" PREVIOUS= "instanceid" /> </INDEXES> Implementing just one index type with optional hints seems easier to me because we could add more different hints for different databases in the future. The upgrade would be relatively simple too - if dbfamily then drop index and create new one.
          Hide
          Tony Levi added a comment -

          Well it seems the existing one is basically unused in PG so it should ideally be replaced to avoid the extra insert/update costs.

          We've been looking into severely trimming other indexes throughout core because many don't get used and just cost resources.

          Show
          Tony Levi added a comment - Well it seems the existing one is basically unused in PG so it should ideally be replaced to avoid the extra insert/update costs. We've been looking into severely trimming other indexes throughout core because many don't get used and just cost resources.
          Hide
          Sam Marshall added a comment -

          Petr:

          1) I like your proposal for the HINT= option, that looks exactly right.

          2) In Postgres with locale set, according to the manual, the standard index will be used for locale-sensitive sorting/ordering, i.e. if you did something > whatever or < or ORDER BY with a limit. So there are situations where you might need both but it sounds likely those are probably rare and could probably be ignored at the moment. If you really wanted a way to include this too it should probably use two different hint values, like HINT="pattern,ordered" say, that way you only create the two indexes if it includes both hints. See: http://www.postgresql.org/docs/9.0/static/indexes-opclass.html

          Show
          Sam Marshall added a comment - Petr: 1) I like your proposal for the HINT= option, that looks exactly right. 2) In Postgres with locale set, according to the manual, the standard index will be used for locale-sensitive sorting/ordering, i.e. if you did something > whatever or < or ORDER BY with a limit. So there are situations where you might need both but it sounds likely those are probably rare and could probably be ignored at the moment. If you really wanted a way to include this too it should probably use two different hint values, like HINT="pattern,ordered" say, that way you only create the two indexes if it includes both hints. See: http://www.postgresql.org/docs/9.0/static/indexes-opclass.html
          Hide
          Eric Merrill added a comment -

          It is pretty rare. To give you an idea, since our last db reboot, the standard path index (mdl_cont_pat_ix) was used 9159 times out of... 2,297,187,179,655 (2.3 trillion) reads, or 0.0000004%. I think it could be pretty safely replaced by the new index, instead of keeping both. In just the 2 days since we added the new index, it has been used about 54 million times.

          Show
          Eric Merrill added a comment - It is pretty rare. To give you an idea, since our last db reboot, the standard path index (mdl_cont_pat_ix) was used 9159 times out of... 2,297,187,179,655 (2.3 trillion) reads, or 0.0000004%. I think it could be pretty safely replaced by the new index, instead of keeping both. In just the 2 days since we added the new index, it has been used about 54 million times.
          Hide
          Dan Poltawski added a comment -

          I think one important point here is that its a char field with an index, but not well optimised in postgres - and I wouldn't be surprised by others - has anyone done a quick look at our other non-int indexes specifically to see if there are other targets like this?

          Show
          Dan Poltawski added a comment - I think one important point here is that its a char field with an index, but not well optimised in postgres - and I wouldn't be surprised by others - has anyone done a quick look at our other non-int indexes specifically to see if there are other targets like this?
          Hide
          Eric Merrill added a comment -

          The flaw is specific to LIKE operations (strait lookups work fine). If you look back a few comments to one of my previous ones, I did a search through the moodle code base for uses of LIKE, and context.path seems to be the only one that is used regularly.

          Show
          Eric Merrill added a comment - The flaw is specific to LIKE operations (strait lookups work fine). If you look back a few comments to one of my previous ones, I did a search through the moodle code base for uses of LIKE, and context.path seems to be the only one that is used regularly.
          Hide
          Tony Levi added a comment -

          Yeah, what Eric said; I did a few greps after finding this too, nothing else seems particularly important.

          Show
          Tony Levi added a comment - Yeah, what Eric said; I did a few greps after finding this too, nothing else seems particularly important.
          Hide
          Petr Škoda added a comment - - edited

          I have realised that 'pattern' is not good hint because we would have to know the column type which in not available in xmldb_index, so in the end I used full "varchar_pattern_ops" as the hint.

          Please note that you can safely create this index in 2.2 and the upgrade code drops both indexes before creating the new one.

          Thanks everybody!

          Show
          Petr Škoda added a comment - - edited I have realised that 'pattern' is not good hint because we would have to know the column type which in not available in xmldb_index, so in the end I used full "varchar_pattern_ops" as the hint. Please note that you can safely create this index in 2.2 and the upgrade code drops both indexes before creating the new one. Thanks everybody!
          Hide
          Martin Dougiamas added a comment -

          Good to see, thanks!

          Show
          Martin Dougiamas added a comment - Good to see, thanks!
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          I briefly discussed this with Eloy and were not that keen on adding conditional to DB engine things like this into XMLDB. Since its sort of counter to the goal of XMLDB.

          So, I wonder out loud if this is something the postgres driver (and maybe other engines) can work out inteligently.

          Show
          Dan Poltawski added a comment - I briefly discussed this with Eloy and were not that keen on adding conditional to DB engine things like this into XMLDB. Since its sort of counter to the goal of XMLDB. So, I wonder out loud if this is something the postgres driver (and maybe other engines) can work out inteligently.
          Hide
          Petr Škoda added a comment -

          I believe the patch is the only reasonable way to give the DDL layer enough info to create the suitable type of index, that is why I called it a "hint" - something that can be ignored by DDL generators that do not understand it. I have spent a few days on this solution, I would expect you to propose something more concrete if you do not like my approach.

          Show
          Petr Škoda added a comment - I believe the patch is the only reasonable way to give the DDL layer enough info to create the suitable type of index, that is why I called it a "hint" - something that can be ignored by DDL generators that do not understand it. I have spent a few days on this solution, I would expect you to propose something more concrete if you do not like my approach.
          Hide
          Dan Poltawski added a comment -

          I'm communicating the thought process around this issue, explaining why we have multiple sites reporting the positive benefit of this but it hasn't instantly been deployed.

          For something more concrete (from this xmldb novice) - varchar_pattern_ops in the xmldb schema seems like a postgres specific hint rather than a generic hint that could be potentially be read and implemented by one of the other xmldb drivers in the future.

          What happens if for that same index we need to tell another DB engine another hint on the same index? Surely that then makes: HINT="varchar_pattern_ops" incorrect, do we then have to do HINT="psql:varchar_pattern_ops mssql:some_microsoft_thingy", supporting a non-prefixed hint as postgres specific?

          Show
          Dan Poltawski added a comment - I'm communicating the thought process around this issue, explaining why we have multiple sites reporting the positive benefit of this but it hasn't instantly been deployed. For something more concrete (from this xmldb novice) - varchar_pattern_ops in the xmldb schema seems like a postgres specific hint rather than a generic hint that could be potentially be read and implemented by one of the other xmldb drivers in the future. What happens if for that same index we need to tell another DB engine another hint on the same index? Surely that then makes: HINT="varchar_pattern_ops" incorrect, do we then have to do HINT="psql:varchar_pattern_ops mssql:some_microsoft_thingy" , supporting a non-prefixed hint as postgres specific?
          Hide
          Tony Levi added a comment -

          Hmm, surely you can cross that bridge when coming to it? There is hardly any reason to over-engineer right now; it isn't like this code is set in stone forever.

          Show
          Tony Levi added a comment - Hmm, surely you can cross that bridge when coming to it? There is hardly any reason to over-engineer right now; it isn't like this code is set in stone forever.
          Hide
          Petr Škoda added a comment -

          I ended up using the postgresql name because it contains every info we need:

          • varchar - we somehow need to tell the DDL which field the index belongs to because we do not have access to xmldb_field in our index creation code
          • pattern_ops - tells us that we do only patter operations on this index

          We do not need imo any db family there because we can interpret this easily in our ddl generators.

          Show
          Petr Škoda added a comment - I ended up using the postgresql name because it contains every info we need: varchar - we somehow need to tell the DDL which field the index belongs to because we do not have access to xmldb_field in our index creation code pattern_ops - tells us that we do only patter operations on this index We do not need imo any db family there because we can interpret this easily in our ddl generators.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          1) I don't like introducing db-dependent code in the XMLDB definitions, and "varchar_pattern_ops" is that. I'd use some "conceptual" hint, just in case other DBs have a different way to achieve the same improvement.

          2) I agree we don't need to specify any family if we move to those "conceptual" hints.

          3) But the use of hints, db-dependent or conceptual needs to be richer, covering things (rules) like:

          a) can it be created on non-unique indexes?
          b) does it support multiple columns?
          c) does it replace the normal index, or both need to be created? (note there are some hard implications/limitations right now in our DDL layer about that tight now).
          d) any other rule we can find in the future.

          4) I'm not sure the implementation is really correct. Perhaps it can be helping the exact context->path case but, or I'm wrong, or those indexes only help with letf-constant-prefix LIKE queries and don't help at all with equalities, subqueries or ordering. And the implementation is dropping the two indexes and creating only the hinted one by default.

          Surely, if implemented with richer definitions (3) and higher level of abstraction (2) it could find its route towards core.

          In the mean time, I think this could be documented @ install/upgrade docs, or create one report (xmldb, health...) about it.

          But current implementation has my -1. 100%. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - 1) I don't like introducing db-dependent code in the XMLDB definitions, and "varchar_pattern_ops" is that. I'd use some "conceptual" hint, just in case other DBs have a different way to achieve the same improvement. 2) I agree we don't need to specify any family if we move to those "conceptual" hints. 3) But the use of hints, db-dependent or conceptual needs to be richer, covering things (rules) like: a) can it be created on non-unique indexes? b) does it support multiple columns? c) does it replace the normal index, or both need to be created? (note there are some hard implications/limitations right now in our DDL layer about that tight now). d) any other rule we can find in the future. 4) I'm not sure the implementation is really correct. Perhaps it can be helping the exact context->path case but, or I'm wrong, or those indexes only help with letf-constant-prefix LIKE queries and don't help at all with equalities, subqueries or ordering. And the implementation is dropping the two indexes and creating only the hinted one by default. Surely, if implemented with richer definitions (3) and higher level of abstraction (2) it could find its route towards core. In the mean time, I think this could be documented @ install/upgrade docs, or create one report (xmldb, health...) about it. But current implementation has my -1. 100%. Ciao
          Hide
          Martin Dougiamas added a comment -

          If there's any performance improvements to be had here then I don't want to see this going around for too long ... let's get something in 2.3.1 please!

          Show
          Martin Dougiamas added a comment - If there's any performance improvements to be had here then I don't want to see this going around for too long ... let's get something in 2.3.1 please!
          Hide
          Petr Škoda added a comment -

          Eloy, it seems to me you are overcomplicating this - all we need is to speed up one type of index, and that is index on varchar column that is using LIKE operations heavily, so the name "varchar_patter_ops" makes perfect sense to me no matter what postgresql is using internally. After all it is called "hint" - if driver does not understand it it can safely ignore it, I do not get what the problem is.

          Hints do not require any detailed information, it is just a tiny bit of extra optional information included with standard index definition - no need for your 3a)b)c)d)

          http://dictionary.reference.com/browse/hint describes it nicely: hint == an indirect, covert, or helpful suggestion

          Show
          Petr Škoda added a comment - Eloy, it seems to me you are overcomplicating this - all we need is to speed up one type of index, and that is index on varchar column that is using LIKE operations heavily, so the name "varchar_patter_ops" makes perfect sense to me no matter what postgresql is using internally. After all it is called "hint" - if driver does not understand it it can safely ignore it, I do not get what the problem is. Hints do not require any detailed information, it is just a tiny bit of extra optional information included with standard index definition - no need for your 3a)b)c)d) http://dictionary.reference.com/browse/hint describes it nicely: hint == an indirect, covert, or helpful suggestion
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm not overcomplicating anything.

          I'm just saying that current implementation MAY be causing more problems than benefits. I know what a hint is, surely I've been using them before you knew what SQL was, so, please, avoid any idiot comment (said with love).

          What I'm saying is that the hinted index will benefit "some" LIKE queries, 100% agree, like the ones used a lot in accesslib. I'm not negating that point at all.

          But, at the same time, the decision of dropping the non-hinted index can have also a big impact for all the non-LIKE queries. And that impact is, right now, undefined, because all the people here have tried ADDING the new hinted index, keeping the old one. Not REPLACING, that is what the current patch does. That's point 4 in my list above.

          And all the rest of ideas explained in 3 are, simply, a consequence of that, trying to achieve some way to define if we want to add/replace/restrict/whatever the new index, so the DDL will know what to do.

          So the question is, simply, if REPLACING is ok always or no. And I'm 99% sure it's not ok ALWAYS (it depends of the uses of that information in codebase).

          Just that. Really simple (in my mind). Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm not overcomplicating anything. I'm just saying that current implementation MAY be causing more problems than benefits. I know what a hint is, surely I've been using them before you knew what SQL was, so, please, avoid any idiot comment (said with love). What I'm saying is that the hinted index will benefit "some" LIKE queries, 100% agree, like the ones used a lot in accesslib. I'm not negating that point at all. But, at the same time, the decision of dropping the non-hinted index can have also a big impact for all the non-LIKE queries. And that impact is, right now, undefined, because all the people here have tried ADDING the new hinted index, keeping the old one. Not REPLACING, that is what the current patch does. That's point 4 in my list above. And all the rest of ideas explained in 3 are, simply, a consequence of that, trying to achieve some way to define if we want to add/replace/restrict/whatever the new index, so the DDL will know what to do. So the question is, simply, if REPLACING is ok always or no. And I'm 99% sure it's not ok ALWAYS (it depends of the uses of that information in codebase). Just that. Really simple (in my mind). Ciao
          Hide
          Eric Merrill added a comment -

          Just to re-enforce Eloys point that we shouldn't drop the standard index, from the Postges manual.

          Note that you should also create an index with the default operator class if you want queries involving ordinary comparisons to use an index. Such queries cannot use the xxx_pattern_ops operator classes. It is allowed to create multiple indexes on the same column with different operator classes. If you do use the C locale, you do not need the xxx_pattern_ops operator classes, because an index with the default operator class is usable for pattern-matching queries in the C locale.

          Show
          Eric Merrill added a comment - Just to re-enforce Eloys point that we shouldn't drop the standard index, from the Postges manual. Note that you should also create an index with the default operator class if you want queries involving ordinary comparisons to use an index. Such queries cannot use the xxx_pattern_ops operator classes. It is allowed to create multiple indexes on the same column with different operator classes. If you do use the C locale, you do not need the xxx_pattern_ops operator classes, because an index with the default operator class is usable for pattern-matching queries in the C locale.
          Hide
          Petr Škoda added a comment -

          Ah, I do not have any problem with creating standard index + the new one, I am going to update the patch today to always create two indexes when the new hint is present. The deleting of indexes may get a bit tricky, but I guess I can deal with that too.

          Show
          Petr Škoda added a comment - Ah, I do not have any problem with creating standard index + the new one, I am going to update the patch today to always create two indexes when the new hint is present. The deleting of indexes may get a bit tricky, but I guess I can deal with that too.
          Hide
          Petr Škoda added a comment -

          done

          Show
          Petr Škoda added a comment - done
          Hide
          Eloy Lafuente (stronk7) added a comment -

          a) Oki, so always both indexes are handled together (both on create and drop), correct? Just upgrade needs to drop it (or them if both are present) in order to be created again.

          b) And also, we assume that "varchar_patter_ops" is a conceptual hit, so if, for example, oracle can get some benefit (creating whichever is needed), it will implement its own support for that "conceptual" hint correct?

          c) And finally, if we need to specify behaviors in the future for other hints (the hints support may be expanded to support those "attributes" or "rules" needed to specify them, correct?

          If I get "3 affirmative answers" this has my +0.5.

          Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - a) Oki, so always both indexes are handled together (both on create and drop), correct? Just upgrade needs to drop it (or them if both are present) in order to be created again. b) And also, we assume that "varchar_patter_ops" is a conceptual hit, so if, for example, oracle can get some benefit (creating whichever is needed), it will implement its own support for that "conceptual" hint correct? c) And finally, if we need to specify behaviors in the future for other hints (the hints support may be expanded to support those "attributes" or "rules" needed to specify them, correct? If I get "3 affirmative answers" this has my +0.5. Thanks and ciao
          Hide
          Petr Škoda added a comment -

          a) yes, for postgresql always the standard and only if "varchar_patter_ops" is specified on one column index the extra index is created
          b) yes, "varchar_patter_ops" is conceptual and means "very many LIKE searches on varchar column expected"
          c) yes, if there is a need we can add more "hints" or any other mechanism for defining complex index stuff

          Show
          Petr Škoda added a comment - a) yes, for postgresql always the standard and only if "varchar_patter_ops" is specified on one column index the extra index is created b) yes, "varchar_patter_ops" is conceptual and means "very many LIKE searches on varchar column expected" c) yes, if there is a need we can add more "hints" or any other mechanism for defining complex index stuff
          Hide
          Petr Škoda added a comment -

          oh, and if anybody creates custom multiple indexes on the same columns they are now dropped properly - previously only one index was dropped which could result in errors during upgrades.

          Show
          Petr Škoda added a comment - oh, and if anybody creates custom multiple indexes on the same columns they are now dropped properly - previously only one index was dropped which could result in errors during upgrades.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks, my +0.5 to this, then.

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks, my +0.5 to this, then.
          Hide
          Petr Škoda added a comment -

          I have added support for UNIQUE indexes too, I suppose it is better to not use "UNIQUE" in the new extra index for performance reasons because one unique index should be enough.

          Show
          Petr Škoda added a comment - I have added support for UNIQUE indexes too, I suppose it is better to not use "UNIQUE" in the new extra index for performance reasons because one unique index should be enough.
          Hide
          Dan Poltawski added a comment -

          Hi,

          I'm sending this to the integration_held state since this is targeted at 2.4 at the moment. (I suppose we need to discuss the backport possibilities too)

          Show
          Dan Poltawski added a comment - Hi, I'm sending this to the integration_held state since this is targeted at 2.4 at the moment. (I suppose we need to discuss the backport possibilities too)
          Hide
          Dan Poltawski added a comment -

          I've integrated this to 2.4 now. Unfortunately I don't think it is safe to backport this patch to 2.3.

          Petr, i've changed the testing instructions a bit, please could you check them. Especially as I don't think they were updated to reflect the decisiont to create two indexes rather than just the one.

          Show
          Dan Poltawski added a comment - I've integrated this to 2.4 now. Unfortunately I don't think it is safe to backport this patch to 2.3. Petr, i've changed the testing instructions a bit, please could you check them. Especially as I don't think they were updated to reflect the decisiont to create two indexes rather than just the one.
          Hide
          Petr Škoda added a comment -

          thanks dan, looks ok

          Show
          Petr Škoda added a comment - thanks dan, looks ok
          Hide
          Frédéric Massart added a comment -

          Tested and passed on master!

          Show
          Frédéric Massart added a comment - Tested and passed on master!
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          You've made it into the weekly release!

          Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
          http://www.youtube.com/watch?v=_QhpHUmVCmY

          Show
          Dan Poltawski added a comment - Congratulations! You've made it into the weekly release! Thanks for your contribution - here are some random drummers to keep you inspired for the next week! http://www.youtube.com/watch?v=_QhpHUmVCmY

            People

            • Votes:
              10 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: