Hi,

I was reading about PHP persisten connections (http://es2.php.net/mysql_pconnect) and found out a commenct about temporary tables. The mysql documentation says (http://dev.mysql.com/doc/refman/5.0/en/create-table.html)

"TEMPORARY table is visible only to the current connection, and is dropped automatically when the connection is closed."

The API documentation for Drupal 5 says: (http://api.drupal.org/api/function/db_query_temporary/5)

"Temporary tables exist for the duration of the page request".

This is not true if you use persistent connections. I even would propose a change in the code of the function "db_query_temporary", to execute a "DROP TEMPORARY {table}" before doing the "CREATE TEMPORARY" command. This would fix problems with persistent connections, IMHO.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nielsbom’s picture

Component: Misc » Correction/Clarification

Changed the component to reflect the new component categorization. See http://drupal.org/node/301443
-nielsbom

add1sun’s picture

Project: Documentation » Drupal core
Version: » 5.x-dev
Component: Correction/Clarification » documentation

Moving to better queue.

jhodgdon’s picture

Version: 5.x-dev » 6.x-dev
Category: feature » bug

Looks like this fix should be made to Drupal 6 and Drupal 5 documentation.

In Drupal 7, there is nothing explicit about how long a temporary table sticks around, which I think is OK, since in Drupal 7 it is meant to be database-independent code and it may depend on the database being used.

jhodgdon’s picture

Issue tags: +Novice

Could be a good project for someone new to Drupal but somewhat knowledgeable about MySQL and databases in general.

pillarsdotnet’s picture

Status: Active » Needs review
FileSize
2.22 KB

Documentation patch to note that the persistence of temporary tables depends on whether persistent connections are enabled, and linking to available documentation pages on dev.mysql.com and php.net.

jhodgdon’s picture

Wow, this is a pretty old issue.

In Drupal 7/8, the doc for db_query_temporary() doesn't say anything about the duration of temporary tables. I imagine it's because it's so dependent on how you have things set up. But I think probably the D6 doc should be done the same, or else we should move this to D7/8 first and add the note about duration of temporary tables there? What I'd like to avoid is having the D6 doc in better shape than D7/8, which is kind of a regression of sorts. Thoughts?

jhodgdon’s picture

Status: Needs review » Needs work

I think we should go ahead and make the d6 doc consistent with d7/8 as noted in several comments above.

achton’s picture

FileSize
1.91 KB

It seems that temporary tables in PostgreSQL live for the duration of the session, unless you have a non-standard setup, in which case they can be either truncated or emptied at the end of the transaction. So obviously, the current docs aren't correct in all cases, and I propose that the sentence be removed entirely. Patch attached.

achton’s picture

Status: Needs work » Needs review

Go bots.

jhodgdon’s picture

Status: Needs review » Needs work

That sounds like the right course of action, in parallel to Drupal 7, and given the discussion above.

But could we also fix this line just above:
* Use this as a substitute for db_query() when the results need to stored

"need to stored"... should be "need to be stored".

achton’s picture

FileSize
2.36 KB

New patch attached with fix from #10. My editor also fixed a trailing whitespace, hope that's ok.

jhodgdon’s picture

That looks good... but there is one more thing that needs to be fixed that I didn't notice, in all three fix areas:

- * Use this as a substitute for db_query() when the results need to stored
- * in a temporary table. Temporary tables exist for the duration of the page
- * request.
+ * Use this as a substitute for db_query() when the results need to be stored
+ * in a temporary table.
  * User-supplied arguments to the query should be passed in as separate parameters
  * so that they can be properly escaped to avoid SQL injection attacks.

After the changed lines, if the next line starts a new paragraph, then there needs to be a blank line in between. And if it doesn't start a new paragraph, the text needs to be moved up to the previous line and wrapped as a paragraph.

I think it's probably best as a separate paragraph, since it's kind of a different topic.

achton’s picture

FileSize
3.63 KB

New patch attached. For those functions, I also fixed the following (as per http://drupal.org/node/1354):

  • There should be no newline between multiple directives (of the same type)
  • There should be a blank line between the @param section and @return directive

There are more of these throughout those files (and the rest of D6 core, I suspect), but I have not attended to them in this patch.

achton’s picture

Status: Needs work » Needs review

I want the testbots to read my mind, please.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks!

Regarding the other standards in http://drupal.org/node/1354 ... We have an effort underway to fix up the docs for Drupal 7/8 to conform with the standards, but Drupal 6 is not included in that effort (mostly because many of the standards were adopted after Drupal 6 was released). If you'd like to help, the coordinating issue is at:
#1310084: [meta] API documentation cleanup sprint
There are plenty of issues waiting to be taken on there, and your help would be welcome!

achton’s picture

Cool, I'll definitely check that queue out. See you around :)

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed, pushed.

Status: Fixed » Closed (fixed)
Issue tags: -Novice

Automatically closed -- issue fixed for 2 weeks with no activity.