| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | documentation |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Novice |
Issue Summary
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.
Comments
#1
Changed the component to reflect the new component categorization. See http://drupal.org/node/301443
-nielsbom
#2
Moving to better queue.
#3
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.
#4
Could be a good project for someone new to Drupal but somewhat knowledgeable about MySQL and databases in general.
#5
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.
#6
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?
#7
I think we should go ahead and make the d6 doc consistent with d7/8 as noted in several comments above.
#8
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.
#9
Go bots.
#10
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".
#11
New patch attached with fix from #10. My editor also fixed a trailing whitespace, hope that's ok.
#12
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.
#13
New patch attached. For those functions, I also fixed the following (as per http://drupal.org/node/1354):
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.
#14
I want the testbots to read my mind, please.
#15
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!
#16
Cool, I'll definitely check that queue out. See you around :)
#17
Thanks, committed, pushed.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.