Improve efficiency when checking if a table field exists

Bug #1827445 reported by Robert Lyon
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Medium
Unassigned

Bug Description

When upgrading we use a field_exists() check to see if a table has a certain column or not which is fine there as upgrade chunks are a one-time execution run when users are all logged out.

However we have begun to use field_exists() within other parts of the code and so this can get called on every page load by multiple users at once. This requires fetching the ddl.lib file and setting ul an XMLDB table object and an XMLDB field object which has overhead.

Instead we should just query the database directly and ask it if the table has the column or not

So instead of doing:

require_once('ddl.php');
$table = new XMLDBTable('tablename');
$field = new XMLDBField('fieldname');
if (field_exists($table, $field)) {
    ...
}

We could just do:

if (column_exists('tablename', 'fieldname')) {
    ...
}

And have the column_exists function that call the database directly

Robert Lyon (robertl-9)
description: updated
Changed in mahara:
milestone: none → 19.10.0
status: New → In Progress
importance: Undecided → Medium
Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/9950

Revision history for this message
Robert Lyon (robertl-9) wrote :

Ok, have made patch
https://reviews.mahara.org/#/c/9950/

And tested in the following way:

// Old way

    $stime = time();
    log_debug('start: ' . $stime);
    for ($i = 1; $i <= 10000; $i++) {
        require_once('ddl.php');
        $table = new XMLDBTable('view_access');
        $field = new XMLDBField('id');
        if (field_exists($table, $field)) { }
        if (($i % 1000) == 0) {
            log_debug('.');
        }
    }
    $etime = time();
    log_debug('end: ' . $etime);
    log_debug($etime - $stime);

had time diff of approx 130 seconds per run for postgres
had time diff of approx 90 seconds per run for mysql

// vs New way

$stime = time();
log_debug('start: ' . $stime);
for ($i = 1; $i <= 10000; $i++) {
  if (column_exists('view_access', 'id')) { }
  if (($i % 1000) == 0) {
    log_debug('.');
  }
}
$etime = time();
log_debug('end: ' . $etime);
log_debug($etime - $stime);

had time diff of approx 40 seconds for postgres
had time diff of approx 4 seconds for mysql

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Patch for "master" branch: https://reviews.mahara.org/10287

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/10287
Committed: https://git.mahara.org/mahara/mahara/commit/6a26a0c72f6e595187a86befc5418c33276ee5ef
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: master

commit 6a26a0c72f6e595187a86befc5418c33276ee5ef
Author: Robert Lyon <email address hidden>
Date: Wed Sep 4 13:15:41 2019 +1200

Bug 1827445: Check if a table exists in a quicker way

Without needing to include the lib/ddl.php file, and create
the xmldb table objects

We do this by trying to query the table to return one value
and capture the SQLException if it doesn't exist

The places changed are where we don't manipulate the table - so have
left upgrade places the same as we will need to do things the ddl.php
way anyway

behatnotneeded

Change-Id: Ic38cee95a221a22e644370b308356cf687a1e831
Signed-off-by: Robert Lyon <email address hidden>

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/9950
Committed: https://git.mahara.org/mahara/mahara/commit/3e0cd370e245bd0eb011667430f30df6ab42c4f9
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: master

commit 3e0cd370e245bd0eb011667430f30df6ab42c4f9
Author: Robert Lyon <email address hidden>
Date: Fri May 3 10:14:51 2019 +1200

Bug 1827445: Check if a table column exists in a quicker way

Without needing to create the xmldb table / field objects

We do this by trying to query the column and capture the SQLException
if it doesn't exist

behatnotneeded

Change-Id: I0a15dd4d1d4e47e6650576c084ee9e1d36b4ae62
Signed-off-by: Robert Lyon <email address hidden>

Changed in mahara:
status: In Progress → Fix Committed
Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/10368

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

To test the latest patch (see comment 6):

On a new instance, go to Admin menu -> Institutions -> Settings

Expected result: You see a list of institutions.
Actual result: You see the error message.

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/10368
Committed: https://git.mahara.org/mahara/mahara/commit/4bcd9ee6dbc46237ab680ec85eb84104ab96b4f0
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: master

commit 4bcd9ee6dbc46237ab680ec85eb84104ab96b4f0
Author: Robert Lyon <email address hidden>
Date: Mon Sep 23 12:47:12 2019 +1200

Bug 1827445: fixing problem table_exists() call

Making it a db_table_exists() call instead

behatnotneeded

Change-Id: I5206657a7e76a96dbe878147e07d42a241790764
Signed-off-by: Robert Lyon <email address hidden>

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/10384
Committed: https://git.mahara.org/mahara/mahara/commit/ca2b5406848c44ab0cd8742f9f68a4cc235d68d6
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: master

commit ca2b5406848c44ab0cd8742f9f68a4cc235d68d6
Author: Robert Lyon <email address hidden>
Date: Sun Sep 29 09:34:30 2019 +1300

Bug 1827445: Need to revert to old way during upgrade

Otherwise there is brokenness in upgrade

behatnotneeded

Change-Id: I1523ec044a9caecd52f5baa2a86308460ba6c284
Signed-off-by: Robert Lyon <email address hidden>

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/10392

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/10392
Committed: https://git.mahara.org/mahara/mahara/commit/fb37ef6e28a2156efcd35ce3d0a53268b4f93d59
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit fb37ef6e28a2156efcd35ce3d0a53268b4f93d59
Author: Cecilia Vela Gurovic <email address hidden>
Date: Thu Oct 3 13:50:55 2019 +1300

Bug 1827445: db_table_exists alternative without changing SQL exceptions

and the same for db_column_exists

behatnotneeded

Change-Id: Id69e285b39aff1dca7edbbf16847c3b7a436baa5

Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.