Database::tableExists appears to be called repeatedly during normal use of a Drupal site. In PostgreSQL, this is implemented by querying the information_schema.tables view, which in our PostgreSQL 9.2 environment completes in the low-single-digit milliseconds; e.g.:
LOG: duration: 5.150 ms statement: SELECT 1 FROM information_schema.tables WHERE (table_catalog = 'drupal') AND (table_schema = 'public') AND (table_name = 'node');
An equivalent query using the pg_tables view (see attached schema.inc.patch) is typically several times faster, however:
LOG: duration: 0.966 ms statement: SELECT 1 FROM pg_tables WHERE (schemaname = 'public') AND (tablename = 'node');
(Note that in PostgreSQL, the information_schema.tables view essentiallydefines table_catalog as current_database(), so it should be unnecessary to specify it)
While several milliseconds would normally be unnoticeable to a user, I recently encountered a case where the performance of Database::tableExists significantly impacted the performance of the Tripal module during an operation where Drupal nodes are created for records in an external Chado schema. During this operation, which can take hours depending on the number of records in the Chado schema, roughly a third of the total run time was spent in Database::tableExists.
There is precedent for this optimization: the MySQL version of Database::tableExists has been overridden as well to bypass MySQL's relatively-slow information_schema.
Comments
Comment #1
nathanweeks CreditAttribution: nathanweeks commentedComment #2
bzrudi71 CreditAttribution: bzrudi71 commentedJust stumbled over this while looking for remaining issues for #2157455: [Meta] Make Drupal 8 work with PostgreSQL or remove support from core before release. @nathanweeks, could you rewrite your patch for D8 please. To get this in D7 it needs back port from D8 branch. Adding a reference to the parent issue and tagging PostgreSQL.
Comment #3
daffie CreditAttribution: daffie commentedMoving this over to #1744302: [meta] Resolve known performance regressions in Drupal 8.
Comment #4
daffie CreditAttribution: daffie commentedMoved the Database::queryTableInformation to #2451723: Database::queryTableInformation optimization for PostgreSQL.
Comment #5
bzrudi71 CreditAttribution: bzrudi71 commentedPatch looks good to me, but I think we will need a complete test run first before tagging RTBC. Will report results later...
Comment #6
daffie CreditAttribution: daffie commented@bzrudi71: Thank for the review!
Comment #7
bzrudi71 CreditAttribution: bzrudi71 commentedBot run completed with no new fails or exceptions. This one seems ready to me and as even MySQL overrides tableExists() we can do the same I think.
Comment #9
webchickCommitted and pushed to 8.0.x. Thanks!
Looks like this is a candidate for 7.x as well?
Comment #10
bzrudi71 CreditAttribution: bzrudi71 commentedThanks @webchick. Yes, this is a candidate for D7 also.
Comment #11
daffie CreditAttribution: daffie commentedComment #12
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedPatch for Drupal 7.
Comment #14
nathanweeks CreditAttribution: nathanweeks at USDA-ARS commentedWe have been using the Drupal 7 patch by @sivaji in production for two Drupal/Tripal sites running PostgreSQL 9.3.x for approximately five months. Is there anything formal testing that needs to be done before declaring the patch to be "Reviewed & tested by the community"?
Comment #19
joseph.olstadwake up testbot
Comment #20
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedI have updated the D7 backport patch slightly to address this change made recently: #3262341: [D7] Database test table TEST_UPPERCASE causes PostgreSQL tests to fail
Also the testbot is now correctly running D7 PostgreSQL tests, so hopefully we can proceed with these improvements.
Comment #21
joseph.olstadWow, great work poker10! First time I've seen so much green on these postgres test runs.
Comment #22
joseph.olstadalso this one:
#2823488: Backport DatabaseSchema_pgsql::queryTableInformation() improvements
Comment #23
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedActually there are two more backports ready :)
#3265837: Backport DatabaseSchema_pgsql::fieldExists() improvements
#3265842: Backport DatabaseSchema_pgsql::findTables() improvements
Comment #24
mcdruidLooks like a good backport to D7.
I might tweak the comment about the strtolower a little (will do the same in #3265837: Backport DatabaseSchema_pgsql::fieldExists() improvements), but that can be done on commit.
Thanks!
Comment #25
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedRTBC + 1, approved for Merge! Thanks all!
Comment #27
mcdruidTweaked the comment on commit per #24
Thank you everyone!
Comment #29
edame CreditAttribution: edame commentedNote: This modifies the behavior of tableExists for PostgreSQL; it used to be that db_table_exists() also found views, but with this change that's no longer the case.