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

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

        Gliffy Diagrams

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

          Issue Links

            Activity

            Hide
            Petr Skoda added a comment -

            Thanks a lot!

            Show
            Petr Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda added a comment -

            done

            Show
            Petr Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda added a comment -

            thanks dan, looks ok

            Show
            Petr Skoda 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:
                14 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: