- Development - lists.drupal.org

[drupal-devel] [bug] Core help updates
by asimmonds 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: module system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: asimmonds Updated by: asimmonds Status: patch Attachment: http://drupal.org/files/issues/help_updates_1.patch (29.5 KB) Patch to fix a number of small issues with the help: - permissions menu link updates in a number of modules help - anchor link fix in distributed auth help - "my account" link fix in user help - spelling correction in tracker.module help asimmonds -- View: http://drupal.org/node/17208 Edit: http://drupal.org/project/comments/add/17208

1 0

[drupal-devel] [task] Pager Queries, Node Access and Performance
by JonBob 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: node.module Category: tasks Priority: normal Assigned to: Anonymous Reported by: pyromanfo Updated by: JonBob -Status: active +Status: patch Attachment: http://drupal.org/files/issues/node_access.patch (2.91 KB) Here's a patch that splits the nid=0 check off into a separate query. If the user has such a grant, things should be really fast since we don't need any join or even DISTINCT() check. If not, the join is done as before, but without the "OR nid=0" piece. This ideally needs performance checking on systems using node access with nid=0 rows, using it without them, and not using it at all. JonBob Previous comments: ------------------------------------------------------------------------ February 1, 2005 - 10:54 : pyromanfo I've got a Drupal 4.5.2 site running with Taxonomy Access Control. When you go to view a taxonomy and it tries to do an access check, the SQL query takes literally minutes to complete. This is on a dual Xeon box, it's rather beefy and runs two other sites (vBulletin and a hacked up Postnuke site) just fine. The problem comes from the taxonomy_select_nodes function $sql_count = 'SELECT COUNT(DISTINCT(n.nid)) FROM {node} n '. node_access_join_sql() .' INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE tn.tid IN ('. $str_tids .') AND n.status = 1 AND '. node_access_where_sql(); Which ends up looking like SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON (na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid = tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','term_access1') AND tn0.tid IN (9) This will take several minutes to complete. The query next to it that selected the nodes themselves took 3 minutes to complete. Where is the problem exactly? I've seen elsewhere that this is a problem for node_privacy_byrole as well, which makes sense because really all the various access control modules are contributing here is an extra entry in the concat search. Let me know if you need any details of how the MySQL server is configured. Here's some data from my drupal database mysql> select count(nid) from node; +------------+ | count(nid) | +------------+ | 10376 | +------------+ 1 row in set (0.00 sec) mysql> select count(nid) from node_access; +------------+ | count(nid) | +------------+ | 41508 | +------------+ 1 row in set (0.00 sec) Drupal performs very snappily with the node_access stuff disabled, and otherwise it performs very well. A three minute increase in the execute time of an SQL query is just strange, there's something odd going on here I think. ------------------------------------------------------------------------ February 3, 2005 - 16:02 : pyromanfo Reducing the size of the 'realm' field of node_access to something manageable like 12 instead of the current 255 speeds up the query tremendously, still it's rather slow. I also added a search index on the grant_view permission, since the view permission is the only permission you'd want to check for more than one node usually. It did speed it up, however it still take a minute or so on my little test box. I'm going to tweak with it some more and see if I can get it performing better. ------------------------------------------------------------------------ February 3, 2005 - 18:12 : killes(a)www.drop.org Could you give us an estimate with numbers? Or even better, make available the database for testing? ------------------------------------------------------------------------ February 4, 2005 - 14:59 : pyromanfo I can't really make the database available without going through and setting all the text and titles to something random and the uid's to 2, I may try that. I can give you some more numbers tonight though when I start messing with it again. You want execution times and table sizes? Any other specific numbers? Should I just dump show status? ------------------------------------------------------------------------ February 4, 2005 - 15:21 : killes(a)www.drop.org Averages of execution times before and after would be fine, I think. The db would be great of course You could simple set all text to "foo" and email addresses to "root@localhost". ------------------------------------------------------------------------ February 5, 2005 - 12:31 : pyromanfo I'm running these tests on a similar database I haven't been developing on. Created from the same source data with the same script though, I just haven't been hacking it up and it runs on faster hardware. SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON (na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid = tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','term_access1') AND tn0.tid IN (2); +------------------------+ | COUNT(DISTINCT(n.nid)) | +------------------------+ | 2993 | +------------------------+ 1 row in set (7 min 4.00 sec) Then I ran the following queries alter table node_access modify realm varchar(12) NOT NULL default ''; alter table node_access add index view_indx (nid,gid,realm,grant_view); Then I ran mysqlcheck --auto_repair -os Then the query again SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON (na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid = tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','term_access1') AND tn0.tid IN (2); +------------------------+ | COUNT(DISTINCT(n.nid)) | +------------------------+ | 2993 | +------------------------+ 1 row in set (6 min 18.62 sec) Not as big a speed boost as I thought I was seeing unfortunately. Here's 'show variables' mysql> show variables; +---------------------------------+---------------------------------------------------+ | Variable_name | Value | +---------------------------------+---------------------------------------------------+ | back_log | 50 | | basedir | /usr/local/mysql/ | | binlog_cache_size | 32768 | | bulk_insert_buffer_size | 8388608 | | character_set_client | latin1 | | character_set_connection | latin1 | | character_set_database | latin1 | | character_set_results | latin1 | | character_set_server | latin1 | | character_set_system | utf8 | | character_sets_dir | /usr/local/mysql/share/mysql/charsets/ | | collation_connection | latin1_swedish_ci | | collation_database | latin1_swedish_ci | | collation_server | latin1_swedish_ci | | concurrent_insert | ON | | connect_timeout | 5 | | datadir | /usr/local/mysql/var/ | | date_format | %Y-%m-%d | | datetime_format | %Y-%m-%d %H:%i:%s | | default_week_format | 0 | | delay_key_write | ON | | delayed_insert_limit | 100 | | delayed_insert_timeout | 300 | | delayed_queue_size | 1000 | | expire_logs_days | 0 | | flush | OFF | | flush_time | 0 | | ft_boolean_syntax | + -><()~*:""&| | | ft_max_word_len | 84 | | ft_min_word_len | 4 | | ft_query_expansion_limit | 20 | | ft_stopword_file | (built-in) | | group_concat_max_len | 1024 | | have_archive | NO | | have_bdb | NO | | have_compress | YES | | have_crypt | YES | | have_csv | NO | | have_example_engine | NO | | have_geometry | YES | | have_innodb | YES | | have_isam | NO | | have_ndbcluster | NO | | have_openssl | NO | | have_query_cache | YES | | have_raid | NO | | have_rtree_keys | YES | | have_symlink | YES | | init_connect | | | init_file | | | init_slave | | | innodb_additional_mem_pool_size | 1048576 | | innodb_autoextend_increment | 8 | | innodb_buffer_pool_awe_mem_mb | 0 | | innodb_buffer_pool_size | 8388608 | | innodb_data_file_path | ibdata1:10M:autoextend | | innodb_data_home_dir | | | innodb_fast_shutdown | ON | | innodb_file_io_threads | 4 | | innodb_file_per_table | OFF | | innodb_locks_unsafe_for_binlog | OFF | | innodb_flush_log_at_trx_commit | 1 | | innodb_flush_method | | | innodb_force_recovery | 0 | | innodb_lock_wait_timeout | 50 | | innodb_log_arch_dir | | | innodb_log_archive | OFF | | innodb_log_buffer_size | 1048576 | | innodb_log_file_size | 5242880 | | innodb_log_files_in_group | 2 | | innodb_log_group_home_dir | ./ | | innodb_max_dirty_pages_pct | 90 | | innodb_table_locks | ON | | innodb_max_purge_lag | 0 | | innodb_mirrored_log_groups | 1 | | innodb_open_files | 300 | | innodb_thread_concurrency | 8 | | interactive_timeout | 28800 | | join_buffer_size | 131072 | | key_buffer_size | 8388600 | | key_cache_age_threshold | 300 | | key_cache_block_size | 1024 | | key_cache_division_limit | 100 | | language | /usr/local/mysql/share/mysql/english/ | | large_files_support | ON | | license | GPL | | local_infile | ON | | locked_in_memory | OFF | | log | OFF | | log_bin | OFF | | log_error | | | log_slave_updates | OFF | | log_slow_queries | OFF | | log_update | OFF | | log_warnings | 1 | | long_query_time | 10 | | low_priority_updates | OFF | | lower_case_file_system | OFF | | lower_case_table_names | 0 | | max_allowed_packet | 1048576 | | max_binlog_cache_size | 4294967295 | | max_binlog_size | 1073741824 | | max_connect_errors | 10 | | max_connections | 100 | | max_delayed_threads | 20 | | max_error_count | 64 | | max_heap_table_size | 16777216 | | max_insert_delayed_threads | 20 | | max_join_size | 4294967295 | | max_length_for_sort_data | 1024 | | max_relay_log_size | 0 | | max_seeks_for_key | 4294967295 | | max_sort_length | 1024 | | max_tmp_tables | 32 | | max_user_connections | 0 | | max_write_lock_count | 4294967295 | | myisam_data_pointer_size | 4 | | myisam_max_extra_sort_file_size | 2147483648 | | myisam_max_sort_file_size | 2147483647 | | myisam_recover_options | OFF | | myisam_repair_threads | 1 | | myisam_sort_buffer_size | 8388608 | | net_buffer_length | 16384 | | net_read_timeout | 30 | | net_retry_count | 10 | | net_write_timeout | 60 | | new | OFF | | old_passwords | ON | | open_files_limit | 1024 | | pid_file | ********** | | port | 3306 | | preload_buffer_size | 32768 | | protocol_version | 10 | | query_alloc_block_size | 8192 | | query_cache_limit | 1048576 | | query_cache_min_res_unit | 4096 | | query_cache_size | 0 | | query_cache_type | ON | | query_cache_wlock_invalidate | OFF | | query_prealloc_size | 8192 | | range_alloc_block_size | 2048 | | read_buffer_size | 131072 | | read_only | OFF | | read_rnd_buffer_size | 262144 | | relay_log_purge | ON | | rpl_recovery_rank | 0 | | secure_auth | OFF | | server_id | 0 | | skip_external_locking | ON | | skip_networking | OFF | | skip_show_database | OFF | | slave_net_timeout | 3600 | | slow_launch_time | 2 | | socket | /tmp/mysql.sock | | sort_buffer_size | 2097144 | | sql_mode | | | storage_engine | MyISAM | | sync_binlog | 0 | | sync_frm | ON | | system_time_zone | EST | | table_cache | 64 | | table_type | MyISAM | | thread_cache_size | 0 | | thread_stack | 196608 | | time_format | %H:%i:%s | | time_zone | SYSTEM | | tmp_table_size | 33554432 | | tmpdir | | | transaction_alloc_block_size | 8192 | | transaction_prealloc_size | 4096 | | tx_isolation | REPEATABLE-READ | | version | 4.1.7 | | version_comment | Source distribution | | version_compile_machine | i686 | | version_compile_os | pc-linux | | wait_timeout | 28800 | +---------------------------------+---------------------------------------------------+ 176 rows in set (0.03 sec) I'm attaching my test database which takes several minutes to execute a similar query If you want me to run the tests on the test database I can do that, it's just on a much skimpier machine and it takes quite a bit longer (Athlon 600, 256MB Ram vs Dual Xeon). ------------------------------------------------------------------------ February 5, 2005 - 12:33 : pyromanfo Oops, preview removed the test db, here it is. ------------------------------------------------------------------------ February 5, 2005 - 12:38 : pyromanfo It's too big, it's 4.47 MB, I'm gonna put it up on GWJ and link to it. Get it here [1] [1] http://www.gamerswithjobs.com/testdb/drupal_test_db.tar.bz2 ------------------------------------------------------------------------ February 5, 2005 - 12:46 : pyromanfo Attachment: http://drupal.org/files/issues/drupal_test_db.tar.bz2 (734.78 KB) Nevermind, I left in some sensitive info and forgot to wipe the search index, now it's much smaller. ------------------------------------------------------------------------ February 7, 2005 - 17:58 : killes(a)www.drop.org Thanks, but I think you stripped the DB a bit too much. If I execute the query it is very fast because it does not return anything... ------------------------------------------------------------------------ February 7, 2005 - 18:12 : killes(a)www.drop.org When I use the query from your original post, I get some result. I guess I will write a script and run some tests over night... ------------------------------------------------------------------------ February 7, 2005 - 18:55 : killes(a)www.drop.org Ok, here are some preliminary results: - The unchanged query from the original post runs nearly 14 minutes on the hardware redLED lent to us. - If I shorten the varchar field to 18 from 255, it gets about halfed. - neither adding an index for the realm nor the grant_view field does make a change - Omitting the "na.nid = 0 OR" part makes the query very fast (quarter of a second) - SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON ( na.nid = n.nid) left jo in node_access nb ON nb.nid = 0 INNER JOIN term_node tn0 ON n.nid = tn0.nid WHERE n.status = 1 AND ((n a.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','term_access1')) OR (nb.grant_view = 1 AND CON CAT(nb.realm, nb.gid) IN ('all0','term_access1'))) AND tn0.tid IN (9); This query is still very much faster (about half a second) than the original query and gives the same result for your database. I don't know if it is fully equivalent, though. Note that it is a left join. ------------------------------------------------------------------------ February 8, 2005 - 08:06 : pyromanfo The na.nid = 0 stuff is basically a catchall, that's sort of a default permission that overrides anything else. This is good when you're not using node_access, however when you are the na.nid=0 stuff probably isn't there, I know Node Privacy By Role and Taxonomy Access Control both delete the entries for nid=0. Perhaps it's a good idea to control this another way, by setting a variable and using that to determine whether or not some sort of node_access is installed, as it seems using nid = 0 as a default simply kills performance. ------------------------------------------------------------------------ February 8, 2005 - 08:08 : pyromanfo Oh, and thanks for taking a good look at it killes, I should've explained better. The DB I provided, the test DB, has a large number of nodes in tid=9, the other DB I ran the tests on has alot in tid=2. ------------------------------------------------------------------------ February 8, 2005 - 11:17 : Anonymous The more I think about replacing nid=0 with a configuration setting the more it makes sense. The two major node access modules right now have to have a seperate "enable/disable" setting to deal with nid=0, since it overrides everything. In fact, it seems like it's only purpose it to override all other node access settings with a universal default. Why not instead have a global configuration setting under permissions that enables or disable node level permissions, then the various node access modules wouldn't need a seperate disable/enable setting. Furthermore, this performance problem would disappear, because either the node_access stuff would be there and you aren't looking for nid=0, or it isn't and you don't care what's in node_access. It seems it'd save queries to node access when you're not using node level permissions, and make it cleaner to implement a node access modifying module. ------------------------------------------------------------------------ February 8, 2005 - 11:17 : pyromanfo I hate the site randomly logging you out, that was me above. ------------------------------------------------------------------------ February 8, 2005 - 14:23 : JonBob "The more I think about replacing nid=0 with a configuration setting the more it makes sense. The two major node access modules right now have to have a seperate "enable/disable" setting to deal with nid=0, since it overrides everything. In fact, it seems like it's only purpose it to override all other node access settings with a universal default." This is not the case. I think you're confusing the case of nid=0 with the case of realm=all. As an example of the utility of nid=0, consider the scenario given in the example access module [2]. This module implements a simple system whereby anyone can view "public" nodes, and only users with a specific permission can view "private" nodes. This can be rephrased as follows: anonymous users can only view "public" nodes, while privileged users can view all nodes. A grant with nid=0 takes care of the second case, so that the module only needs to deal with granting permission to view the public nodes as necessary. Gerhard's query works in the case where every node has at least one node_access entry with its nid, but I'm afraid it fails in the above example. A node that is private would not have any corresponding entry, and so the inner join would fail for it, returning no record. The left join doesn't even have a chance to be matched. It would be worth benchmarking the scenario in which the nid=0 possibility is removed from the query, but the node_access table has an entry added for each node instead of a single nid=0 entry. If this is faster than the OR in the join, the extra DB storage is probably worth the change. [2] http://drupaldocs.org/api/head/file/contributions/docs/developer/examples/n… ------------------------------------------------------------------------ February 8, 2005 - 20:08 : pyromanfo "As an example of the utility of nid=0, consider the scenario given in the example access module. This module implements a simple system whereby anyone can view "public" nodes, and only users with a specific permission can view "private" nodes. This can be rephrased as follows: anonymous users can only view "public" nodes, while privileged users can view all nodes. A grant with nid=0 takes care of the second case, so that the module only needs to deal with granting permission to view the public nodes as necessary. " I think there's a bug in the module, in the docs in the top shouldn't the initial SQL statement be adding that permission to gid=1? Though you're right, I'd forgotten because I hadn't seen any of the current modules use it. As for that performance comparison mysql> SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN term_node tn0 ON n.nid = tn0.nid INNER JOIN node_access na ON (na.nid = n.nid) WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','term_access1') AND tn0.tid IN (2); +------------------------+ | COUNT(DISTINCT(n.nid)) | +------------------------+ | 2993 | +------------------------+ 1 row in set (0.23 sec) mysql> update node_access set realm='blam' where realm='term_access'; Query OK, 41508 rows affected (5.42 sec) Rows matched: 41508 Changed: 41508 Warnings: 0 mysql> insert into node_access (nid,gid,realm,grant_view) values (0,1,'term_access',1); Query OK, 1 row affected (0.00 sec) mysql> SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN term_node tn0 ON n.nid = tn0.nid INNER JOIN node_access na ON (na.nid = 0 OR na.nid=n.nid) WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','term_access1') AND tn0.tid IN (2); +------------------------+ | COUNT(DISTINCT(n.nid)) | +------------------------+ | 2993 | +------------------------+ 1 row in set (0.16 sec) So I don't think it improves performance for the nid=0 case, though it is suprisingly slow. ------------------------------------------------------------------------ February 8, 2005 - 20:57 : moshe weitzman perhaps we should split the query into 2 queries in order to avoid the 'na.nid = 0' part of the JOIN. Alternatively, perhaps we could inject that clause only when needed using the db_rewrite_sql hook. not sure. ------------------------------------------------------------------------ February 9, 2005 - 23:36 : pyromanfo Splitting it into two queries would make sense I think. Basically you could have it entirely in node_access_join_sql and node_access_where_sql. There we can check for the nid=0 case for any gids and realms. Then if it returns true, we know we don't have to check anything else, so we return null for the SQL to enter. We could even store this in a static variable to avoid calling this twice for join_sql and where_sql So in the nid=0 case, this should be similarly fast as we're removing a table from the JOIN in exchange for checking node_access for one nid. mysql> select count(nid) from node_access where nid=0 and gid=1 and realm in ('all','term_access','blah'); +------------+ | count(nid) | +------------+ | 0 | +------------+ 1 row in set (0.37 sec) SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN term_node tn0 ON n.nid = tn0.nid WHERE n.status = 1 AND tn0.tid IN (9); +------------------------+ | COUNT(DISTINCT(n.nid)) | +------------------------+ | 4202 | +------------------------+ 1 row in set (0.15 sec) SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON (na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid = tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid) IN ('all0','blah1') AND tn0.tid IN (9); +------------------------+ | COUNT(DISTINCT(n.nid)) | +------------------------+ | 4202 | +------------------------+ 1 row in set (1.97 sec) It looks to be faster, actually. Then, when we don't get a result, we check node access with the 'na.nid=0 OR' removed. This is certain to be faster as the test have shown, taking the query down from 7 minutes to a matter of seconds. In either case, this method would be faster. Plus, it wouldn't require any changes to other modules, simply change the node_access_where_sql and node_access_join_sql functions. JonBob, would you be okay with that? I can code it if you think you'll accept the patch. ------------------------------------------------------------------------ February 10, 2005 - 08:38 : Anonymous Splitting the queries into two is probably the only way to go. However, I'd like to avoid calling the nid = 0 query if possible. My Drupal installations that use node access do not use nid = 0 at all. I think we could store the available (nid = 0) => realm combinations somewhere and only execute the query if a matching condition is found. Gerhard ------------------------------------------------------------------------ February 10, 2005 - 10:14 : moshe weitzman that nid=0 is going to be very fast when done all by itself. no need to introduce any other mechanism IMO. testing will tell us for sure though ------------------------------------------------------------------------ February 10, 2005 - 13:31 : killes(a)www.drop.org The nid = 0 query would be executed for every node query on a page call even if your access module doesn't use nid = 0 at all. Most implementations do not use nid = 0. If we can avoid a query we should do so. -- View: http://drupal.org/node/16558 Edit: http://drupal.org/project/comments/add/16558

1 0

[drupal-devel] [bug] Twin problems with comments
by Junyor 11 Feb '05

11 Feb '05

Project: Drupal -Version: 4.5.2 +Version: cvs Component: comment.module Category: bug reports Priority: critical Assigned to: Junyor Reported by: pennywit Updated by: Junyor Status: patch Attachment: http://drupal.org/files/issues/node_comment_stats_head2.patch (11.92 KB) Synched patch with HEAD. Junyor Previous comments: ------------------------------------------------------------------------ October 7, 2004 - 15:23 : pennywit I have two problems with my upgrade at http://www.pennywit.com. First is that a number of comments have the body turn to the number 3. It looks like this happens when one of my users tries to edit his comments. The second is that in any nodes submitted before I upgraded, I don't see a count of the comments already submitted. I'm echoing this to the support forum ... ------------------------------------------------------------------------ October 7, 2004 - 16:01 : pennywit It says this has been fixed ... what do I need to do on my side? --|PW|-- ------------------------------------------------------------------------ October 8, 2004 - 09:11 : Bèr Kessels http://drupal.org/node/11316`was the bug. Update commment.module will fix this problem. ------------------------------------------------------------------------ October 17, 2004 - 13:04 : kps This problem remains (in 4.5.0-rc 1/2 hour before this post): " ... any nodes submitted before I upgraded, I don't see a count of the comments already submitted. " The {node_comment_statistics} table is evidently not initialized correctly. Justification for "critical": Since most people who install 4.5.0 will be users of older releases, upgrading really ought to work before the release. ------------------------------------------------------------------------ October 17, 2004 - 13:47 : kps Attachment: http://drupal.org/files/issues/updc.php (1.26 KB) Attached, raw and as-is, the script I used to initialize the {node_comment_statistics} table. ------------------------------------------------------------------------ October 20, 2004 - 18:12 : kps Attachment: http://drupal.org/files/issues/updatecommentcount.php (907 bytes) My above attachment (updc.php) is broken. This one is... better. Visiting this page should at least populate the node_comment_statistics table with something not entirely unreasonable. ------------------------------------------------------------------------ October 25, 2004 - 08:39 : Junyor kps is right, the update won't correctly populate the table. Here's (part of) update_105: "INSERT INTO {node_comment_statistics} (nid, cid, last_comment_timestamp, last_comment_name, last_comment_uid, comment_count) SELECT n.nid, 0, n.created, NULL, n.uid, 0 FROM {node} n" So, it's setting the comment_timestamp = node creation time, forgetting the comment_creator, using the userid that created the node as the last_comment_uid, and setting the comment count to 0. update_105 needs to be redone and I'd recommend a new update be created for 4.5.1 that will correctly initialize the node_comment_statistics table. ------------------------------------------------------------------------ November 12, 2004 - 10:02 : Junyor OK, here's two patches to solve the problem. Issues fixed: - Fixes node_comment_statistics table prefixing for PostGreSQL - Drops CID column from node_comment_statistics and supporting code in comment.module - Initializes comments table with usernames (needed by node_comment_statistics table) - Correctly initializing node_comments_statistics table - Removed duplicate query for last_comment_name in node_comment_statistics query in comment.module Needs checking: - I couldn't test this with PostGreSQL - I'm no database expert, so the updates.inc changes need to be gone over with a fine-toothed comb - Do we need any special handling for comments with no 'name', i.e. truly anonymous comments These patches are for the DRUPAL-4-5-0 branch. ------------------------------------------------------------------------ November 12, 2004 - 13:01 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats.patch (7.89 KB) ------------------------------------------------------------------------ November 12, 2004 - 13:01 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats2.patch (3.44 KB) Actually attaching patches. :) ------------------------------------------------------------------------ November 14, 2004 - 21:10 : Dries I'm not 100% sure but isn't the name field of the comments table supposed to be NULL, unless the comment is an /anonymous comment/? Is it really required to initialize the name field? What happens if you don't? I just checked drupal.org's database and only /post Drupal 4.5.0 comments/ have the registered user's name in the comment table. I'm a little nervous about committing database changes to stable branches (DRUPAL-4-5) so please make sure this patch is well-tested. The patches don't apply against HEAD but I can port them once we/you ironed out the last glitches. Otherwise these patches look fine. Good job. ------------------------------------------------------------------------ November 14, 2004 - 23:41 : Junyor I don't know if it's necessary, but I'm just making sure it's consistent. Maybe the author of the node_comment_statistics patch could comment? I'm also concerned about having the name information for registered users outside of the users table. I've tested this on a test site and I'll try testing on my production site once we have this worked out. ------------------------------------------------------------------------ November 15, 2004 - 10:45 : Dries I don't know whether the original author (ccourtne) is still around but it should be easy enough to test whether it is necessary or not. From what I've seen, it isn't necessary. In fact, your current patch might break things: like, what happens when somone changes his or her username? AFAIK, the old username would be shown in the comments. ------------------------------------------------------------------------ November 15, 2004 - 11:03 : Junyor OK, I'll edit that bit and resubmit. You'd like this for HEAD only? ------------------------------------------------------------------------ November 15, 2004 - 18:24 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats-2.patch (7.24 KB) New patch for database files. ------------------------------------------------------------------------ November 15, 2004 - 18:25 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats2-2.patch (3.46 KB) New patch for comment.module. ------------------------------------------------------------------------ November 22, 2004 - 11:31 : Junyor Attachment: http://drupal.org/files/issues/node-comment-statistics_head.patch (7.26 KB) Patches for head. ------------------------------------------------------------------------ November 22, 2004 - 11:31 : Junyor Attachment: http://drupal.org/files/issues/node-comment-statistics_head2.patch (3.45 KB) ------------------------------------------------------------------------ November 23, 2004 - 23:24 : Dries I'd like to move forward with this patch and include it in Drupal 4.5.1. I can't reproduce this problem (it seems) so it would be much appreciated if those who can, can test it. ------------------------------------------------------------------------ November 29, 2004 - 00:42 : Junyor It applied and works well on my 4.5.0 site. A lot of nodes were listed as not having comments before, but they're working now. Dries alerted me to a drupal-support message regarding the patch and I'll look into that tomorrow. ------------------------------------------------------------------------ December 8, 2004 - 06:12 : crw After applying the patch, approving new comments still does not update the node_comment_statistics table, therefore the comment_count field is off and the number of new comments does not show up on the main page of my site. I'm currently troubleshooting this and will report my findings here. ------------------------------------------------------------------------ December 8, 2004 - 06:13 : crw I should point out that I applied the following patch: node-comment-statistics_head2.patch And the problem described above still exists. ------------------------------------------------------------------------ December 8, 2004 - 06:25 : crw Ok, headway. Looks like submitting a comment triggers an updating of node_comment_statistics, but editing a comment to publish/unpublish does not. Both should trigger the update, so I just need to find out why it isn't working for the 'update' case. ------------------------------------------------------------------------ December 8, 2004 - 07:22 : crw Ok, I found the problem and came up with a solution. I've been manually approving comments from anonymous users via the admin interface. comment_admin_edit() calls comment_save(), but neither calls _comment_update_node_statistics. I inserted the following two lines around line 952 of comment.module after the call to comment_save(): $nid = db_result(db_query('SELECT nid FROM {comments} WHERE cid = %d',$edit['cid'])); _comment_update_node_statistics($nid); Ideally, there'd be tests for all these. comment_save should produce a return value, and comment_admin_edit() shouldn't go forward unless comment_save returns true. Please excuse my lack of diff/patch-fu. :) ------------------------------------------------------------------------ December 8, 2004 - 09:58 : Junyor Bah, that function should call comment_save(). All comment changes should go through comment_save(). It would make life so much easier. I'll try to come up with an updated patch soon, but probably not before this weekend. ------------------------------------------------------------------------ December 8, 2004 - 10:28 : Dries I agree that all comment editing should go through comment_save(), yet that will require a bit of refactoring. Also, there are two 'edit comment' forms (one for users, one for administrators) that should be merged, much like we merged 'node edit' forms. Anyone? ------------------------------------------------------------------------ December 11, 2004 - 17:19 : Junyor Dries: I see that you made a change to the updating of node_comment_statistics in CVS HEAD. Do I still need to add an updated patch? Are there plans for a 4.5.2 that could use this patch? ------------------------------------------------------------------------ December 11, 2004 - 18:14 : ax i tried upgrading a 4.4 site to 4.5 yesterday and fell into the same trap as pennywit, kps, junyor, and others: the update for the node_comment_statistics table only updates forum comments and inserts num_comments 0 for all other node types. quite cheaty, this. junyors 4.5 patches (node_comment_stats-2.patch, node_comment_stats2-2.patch) seem to solve the problem for me. at least, i see proper "replies" counts in tracker and node views now. i didn't try approving / publishing / unpublishing comments, though, nor did i check other issues mentioned in the thread (user names, postgresql, ...). i applied the 2 patches to a 4.4 database / 4.5 code both before running update.php and afterwards. in the first case, there is a small glitch in that update.php throws an error: user error: Unknown table 'node_comment_statistics' DROP TABLE {node_comment_statistics} because the patch removes "CREATE TABLE {node_comment_statistics}" from update_105. this could be catched by changing to "DROP TABLE IF EXISTS {node_comment_statistics}". this is definitely a critical bug, and these patches should be applied to 4.5 as soon as possible, regardless of the "all comment editing should go through comment_save()" issue. ------------------------------------------------------------------------ December 11, 2004 - 20:53 : Jeremy(a)kerneltrap.org I've just run into this same bug, trying to upgrade from 4.4 to 4.5. It's a show stopper for me. :( Ugh, so close. ------------------------------------------------------------------------ December 11, 2004 - 23:29 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats-3.patch (7.25 KB) Changed patch for updates.inc based on feedback from ax. For 4.5. ------------------------------------------------------------------------ December 11, 2004 - 23:53 : Junyor Attachment: http://drupal.org/files/issues/node_comment_patch2-3.patch (4.2 KB) And an update for comment.module with additions to comment_save to update the node_comment_statistics table. This should cover all edit cases. For 4.5. Untested. ------------------------------------------------------------------------ December 12, 2004 - 18:40 : Jeremy(a)kerneltrap.org I tested the upgrade process on KernelTrap -- worked perfectly nearest I could tell. I'm also running with your comment.module patch. ------------------------------------------------------------------------ December 15, 2004 - 14:37 : Junyor What needs to be done for this to be included in core? ------------------------------------------------------------------------ December 15, 2004 - 15:26 : Jeremy(a)kerneltrap.org FWIW: In the days following my upgrade, I've found numerous issues with comment statistics. Specifically, many old postings list "x new of x", but clicking "x new" shows that in fact none of them were new. I have not figured out what's different about the nodes for which this is a problem compared to the nodes for which this isn't a problem. ie, it doesn't seem to be a problem with a specific node type, etc. In other words: I retract my earlier comment that the upgrade went perfectly with this patch applied. Sorry I don't have more specifics to offer, perhaps I'll have more time at a future date to help dig into this more. In any case, the patch did improve the process noticeably. ------------------------------------------------------------------------ December 15, 2004 - 15:31 : Junyor So, none of the comments said they were new or you had read them previously despite them being marked new? ------------------------------------------------------------------------ December 15, 2004 - 16:13 : Jeremy(a)kerneltrap.org > or you had read them previously despite them being marked new? Right. Comments I had already read when running 4.4 were suddenly marked as new after the upgrade to 4.5. ------------------------------------------------------------------------ December 15, 2004 - 17:50 : Junyor That sounds like a problem with history more than node_comment_statistics. I'm pretty sure this code doesn't do anything related to marking comments as new or old. ------------------------------------------------------------------------ December 15, 2004 - 21:33 : Dries If possible provide a single patch against DRUPAL-4-5 and a second patch against HEAD. Looks like the patches are no longer in sync. ------------------------------------------------------------------------ December 16, 2004 - 10:57 : Junyor I will do so soon, probably this weekend. ------------------------------------------------------------------------ December 18, 2004 - 01:42 : Jonathan Furness Ah.... that's why I am struggling with my upgrade from 4.4.2 to 4.5.1..... thank goodness I found this page. Is there a case for building a script which looks for all the records in the comments table and updates the node_comment_statistics table... for those of us who are now working in 4.5.1 with inaccurate records in node_comment_statistics table? Looking forward to the fix.... Jonathan ========== Jonathan Furness www.jonathansblog.net ------------------------------------------------------------------------ December 18, 2004 - 10:48 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats_4-5.patch (11.45 KB) Updating patch to current 4.5.1. Jonathan: Just apply this patch to your installation (from the main Drupal directory, type 'patch -p0 -u < ../node_comment_stats_4-5.patch') and run update.php. ------------------------------------------------------------------------ December 18, 2004 - 10:59 : Junyor Dries: The comment.module in HEAD makes use of the cid column in node_comment_statistics. My patches were removing that column, as it wasn't being used. Should I leave it alone afterall? ------------------------------------------------------------------------ December 20, 2004 - 07:23 : Dries The column seems to get populated though (it's not empty). If it the data is not used, you can remove it. mysql> SELECT COUNT(cid) FROM node_comment_statistics WHERE cid != 0; +------------+ | COUNT(cid) | +------------+ | 3970 | +------------+ ------------------------------------------------------------------------ December 20, 2004 - 09:27 : Anonymous Well, cid wasn't used until the change you made in revision 1.314 of comment.module. It was never set correctly or used anywhere in the code, so I simply removed it. ------------------------------------------------------------------------ December 20, 2004 - 10:40 : Anonymous Attachment: http://drupal.org/files/issues/node_comment_stats_4-5_0.patch (11.64 KB) Slightly updated patch for 4.5.1. ------------------------------------------------------------------------ December 20, 2004 - 10:41 : Junyor Attachment: http://drupal.org/files/issues/node_comment_stats_head.patch (12.08 KB) And patch for HEAD (with all CID stuff removed). ------------------------------------------------------------------------ January 4, 2005 - 10:04 : Junyor Dries: Is there anything else you need here before its commitable? ------------------------------------------------------------------------ January 4, 2005 - 15:31 : ax i too would much appreciate if this *release* *critical* bug could be fixed soon. thanks a lot! ------------------------------------------------------------------------ January 4, 2005 - 23:59 : Dries I look into this patch tomorrow (err, later today). ------------------------------------------------------------------------ January 5, 2005 - 21:37 : Dries I committed the patch to DRUPAL-4-5. I'm putting the HEAD version on hold until the revisions patch landed. Please upgrade your Drupal 4.5 sites to DRUPAL-4-5 in preparation of Drupal 4.5.2. Thanks. ------------------------------------------------------------------------ January 18, 2005 - 14:47 : rooey I hate to be a total baboon - but i'm still seeing a problem after upgrading to 4.5.2 - the "poll" block has no comment options available (stats or otherwise). Have i missed something? (or are these patches not included in the 4.5.2 release?) Thanks! ------------------------------------------------------------------------ January 18, 2005 - 14:54 : Junyor They are included in 4.5.2. Did you run the update script (it has to be run despite what the Drupal 4.5.2 announcement says)? ------------------------------------------------------------------------ January 18, 2005 - 15:38 : rooey Yeah... I did :s (assuming you're talkin' about update.php) ------------------------------------------------------------------------ January 18, 2005 - 15:40 : Junyor I'm not entirely sure what this fix has to do with the poll block, as I don't use that. This fix is to display the correct number of comments for nodes. Could you go into some more details about the problem you're having? ------------------------------------------------------------------------ January 18, 2005 - 15:46 : Jeremy(a)kerneltrap.org I'm having a similar problem. Note that if you look at the poll in a node view, the comments show up: http://kerneltrap.org/node/4536 But if you look at the poll on the front page, it doesn't list any comments: http://kerneltrap.org/ I've not taken any time to try and track this down yet. ------------------------------------------------------------------------ January 18, 2005 - 15:46 : rooey Sure... Since the upgrade from 4.4 to 4.5 the poll block lost all ability to play with comments. For example, there used to be the standard "add new comment" links etc... Now there is not :( The poll module itself *seems* to be calling the various bits of code it's supposed to, but alas, nothing is happening. Take a look at the site if it helps: http://www.lineageone.com I have jury-rigged the "quote" module to at least get people able to comment on stuff for now. ------------------------------------------------------------------------ January 18, 2005 - 15:50 : Morbus Iff Regarding the poll/comment issue, see also: http://drupal.org/node/14936. Not sure if that bug should be closed and wrapped into this Issue, or if we should move over to there as a different issue. ------------------------------------------------------------------------ January 18, 2005 - 16:01 : rooey Thanks :) Dang - and they described it so much better than me too! How do you want to proceed? ------------------------------------------------------------------------ January 18, 2005 - 16:13 : Junyor Let's take discussions to that bug report. I'll take a quick look in a couple hours to see if there's anything glaringly wrong. -- View: http://drupal.org/node/11366 Edit: http://drupal.org/project/comments/add/11366

1 0

[drupal-devel] [bug] "TYPES" is not a legal field name table "BLOCKS"
by andremolnar 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: Philippe Updated by: andremolnar Status: patch Attachment: http://drupal.org/files/issues/blocks_db_change_5.patch (1.79 KB) And the db files and upgrade path. andremolnar Previous comments: ------------------------------------------------------------------------ February 5, 2005 - 13:31 : Philippe There was a "TYPES" field added to the BLOCKS table. MySQL does not like this; "types" is considered a reserved word. (MySQL-server-4.1.0-0) The field name needs to be changed to something else. ------------------------------------------------------------------------ February 5, 2005 - 13:37 : Philippe Attachment: http://drupal.org/files/issues/types.diff (3.51 KB) here's a patch ------------------------------------------------------------------------ February 9, 2005 - 21:35 : andremolnar Changing Status to patch. I guess we didn't catch this - the field wasn't orginally called types. Good eye. andre ------------------------------------------------------------------------ February 9, 2005 - 21:39 : Dries I suggest we name this field 'nodes'. Also, we'll need an upgrade path for those who are running CVS HEAD. ------------------------------------------------------------------------ February 9, 2005 - 21:54 : JonBob I'm a bit confused by this. The word "types" doesn't show up here: http://dev.mysql.com/doc/mysql/en/reserved-words.html ------------------------------------------------------------------------ February 9, 2005 - 22:12 : andremolnar I remembered seeing it here: http://drupal.org/node/2497 If this is incorrect, then the documentation needs to be changed. But all in all, nodes isn't a bad idea - I can submit a patch along with the upgrade path if 'types' is indeed a reserved word in either mySQL or pgSQL. andre ------------------------------------------------------------------------ February 11, 2005 - 01:22 : ax "TYPES" /is/ a reserved word in mysql 4.1.0 [1] and 4.1.1 [2]. it doesn't show up at http://dev.mysql.com/doc/mysql/en/reserved-words.html because it isn't reserved anymore since some point release later. it isn't even documented in the mysql changelog that it has ever been a reserved word ... once again the suggestion to get rid of sql reserved words issues once and for all [3] [1] http://drupal.org/node/1612 [2] http://info.ccone.at/INFO/MySQL-Doc/manual_Reference.html#Reserved_words [3] http://drupal.org/node/4907#sql-reserved-word-fix ------------------------------------------------------------------------ February 11, 2005 - 14:27 : andremolnar I'll submit a patch later today to use the word 'nodes' in block - along with the database upgrade path. andre ------------------------------------------------------------------------ February 11, 2005 - 14:40 : andremolnar Attachment: http://drupal.org/files/issues/block_module_7.patch (1.69 KB) Here is the patch to block.module -- View: http://drupal.org/node/16817 Edit: http://drupal.org/project/comments/add/16817

1 0

[drupal-devel] [bug] Clicking on Home after login brings up browser-cached non-logged-in version of home page
by moshe weitzman 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: user system Category: bug reports Priority: critical Assigned to: mathias Reported by: christianlong(a)christianlong.com Updated by: moshe weitzman Status: patch "Isn't this change saying that all pages must always be redownloaded if mod_expires is enabled?" Correct. And that is desired behavior. A dynamic application like ours has no choice but to redownload every page (excluding RSS feeds). moshe weitzman Previous comments: ------------------------------------------------------------------------ February 16, 2004 - 15:44 : christianlong(a)christianlong.com Attachment: http://drupal.org/files/issues/f_040214_01.txt (6.33 KB) This bug has been discussed before, but not resolved. http://drupal.org/node/view/1740 http://drupal.org/node/view/5686 One suggestion was to change the browser settings. That means I have to tell all my users to change their browser cache settings. Not a great way to run a site. Instead, the login should just work with default browser settings. Currently, login does not work, as described below. I log in to my drupal site, feingold.christianlong.com, and the logged-in home page appears, with my user name. OK. The address bar shows http://feingold.christianlong.com/node?PHPSESSID=867c4e2d57d290a3f59e138524… In the site menu, the "Home" link links to http://feingold.christianlong.com/ Here's the problem: when I click on the "Home" link, the browser takes me to the un-logged-in version of http://feingold.christianlong.com/, which was cached when I was not logged in. When I hit refresh, I do get the logged-in version of the page. So, to restate Start at home page of site, not logged in Log in - this works, and brings up a logged-in version of the page Click "Home" - this is where the problem is. I get the cached version of the home page (from when I was not logged in) Refresh browser - now I see the correct, logged-in version of the home page. It looks like I am logging in OK, but that Drupal is not telling my browser that there is a new version of the home page that it needs to check for Maybe the original (non-logged-in) version of the page is not marked for no-cache, and so when I click on the "Home" link, I get the cached (non-logged-in) version Browsers: MSIE 6 (happens a lot) and Firebird (happens sometimes). Also happens with christianlong.com Attached, find an annotated record of the HTTP header traffic. Thanks Christian ------------------------------------------------------------------------ February 16, 2004 - 15:48 : christianlong(a)christianlong.com Forum discussion is here http://drupal.org/node/view/5669 ------------------------------------------------------------------------ March 4, 2004 - 12:49 : moshe weitzman I am seing this same behavior at http://www.nshp.org [1]. To see for yourself, login using the box on the home page while using IE (Firefox doesn't have a problem). user: mwtest8 pass: testpass Here is the php config of the server [2]. Note that IE is set to "Check for newer pages: automatically" in Options -> Temporary Internet Files -> Settings Thoughts on how to resolve this are welcome. [1] http://www.nshp.org/phpinfo.php [2] http://www.nshp.org/phpinfo.php ------------------------------------------------------------------------ June 15, 2004 - 21:38 : duztin I had this problem, fix the date on your pc, mine was a month ahead and my cookies were expiring too soon. ------------------------------------------------------------------------ July 27, 2004 - 15:18 : killes(a)www.drop.org Setting to cvs. Is the fix proposed by duztin a real fix? if not: what else could we do? ------------------------------------------------------------------------ October 16, 2004 - 16:45 : mathias Attachment: http://drupal.org/files/issues/browser_caching.patch (644 bytes) I'm moving this to critical because in some cases this bug causes the hidden $edit['destination'] value of the login form to be set to 'logout', and to the user it appears they can't login since they are immediately logged out again. Very frustrating. The other side effect of this browser caching bug are, as stated above, the authenticated user will receive stale pages and if you have the login block enabled it will look as though they're were unexplicably logged out. It all depends on your browser and its settings, but to attempt to reproduce: 1. Login to your site. 2. Next, Click the homepage link. If you are served a stale copy of the page, you've hit the bug. This seems to happen more with IE and Safari than Firefox. A potential patch is to have Drupal issue the following header: header('Cache-Control: no-store, no-cache, must-revalidate'); It works, but I don't know the implications this has on other Drupal components such as RSS conditional GETS and gzip page serving. ------------------------------------------------------------------------ October 21, 2004 - 13:30 : mathias Attachment: http://drupal.org/files/issues/browser_caching_0.patch (593 bytes) Thanks to Ethereal [3] and LiveHTTPHeaders [4], I was able to trace this problem to the Cache-Control header being sent by Mac OS X server. On this OS, mod_expires [5] is enabled by default for Apache which sets the Cache-Control time to 60 seconds for dynamically rendered pages. The implications of this action are that once you login, re-visiting any page on the site will result in stale, locally-cached versions if you viewed that page within 60 seconds of logging in. Since you weren't logged in on those pages, the system will have appeared to logged you out. It will keep doing this unless you wait 60 seconds to login. Thus, users perceive this as a failure to successfully log in. This new patch doesn't tweak bootstrap.inc. Instead it uses .htaccess to test if mod_expires is enabled and resets the caching time to 1 second for dynamically rendered content. The benefit of this approach is that it doesn't interfere with other types of caching that may be in place for images, pdf files, etc. # Overload mod_expires variables. <IfModule mod_expires.c> # Reduce the time dynamically generated HTML pages are cache-able. ExpiresDefault A1 </IfModule> Down the road it may also be wise to consider sending our own caching headers to maintain control of our caching environment. I reviewed the following pieces of software, all of which intentionally disable caching by the client browser or proxy caches. I just grabbed the ones I thought were most popular. Plone (Cache-Control parameter is configurable) Wordpress (except for RSS feeds) eZpublish Mambo phpBB And if you're so inclined, here's the relevent code snippets [6] for each piece of software. [3] http://www.ethereal.com/ [4] http://livehttpheaders.mozdev.org/ [5] http://httpd.apache.org/docs/mod/mod_expires.html [6] http://asitis.org/tmp/cms_cache_review.txt ------------------------------------------------------------------------ October 21, 2004 - 13:39 : moshe weitzman I'v seen this bug on drupal.org, so this impacts more than OSX server ... Looks like a nice clean patch to me. ------------------------------------------------------------------------ October 26, 2004 - 11:21 : jvandyk +1 from me. This reduces confusion among end users while retaining caching ability for other mime types. ------------------------------------------------------------------------ October 27, 2004 - 03:05 : wazdog Does this end up having any effect on the RSS conditional GETS? If not, then +1 from me. (The WordPress code does check for RSS, so I'm thinking this may set them to expire too?) ------------------------------------------------------------------------ October 27, 2004 - 20:58 : jvandyk Attachment: http://drupal.org/files/issues/htaccess_0.patch (792 bytes) Regarding the concern about RSS caching: this patch is identical to the previous one except instead of targetting the default we target the text/html MIME type. <IfModule mod_expires.c> # Reduce the time dynamically generated HTML pages are cache-able. ExpiresByType text/html A1 </IfModule> This means that RSS feeds, which are MIME type text/xml, are not affected. ------------------------------------------------------------------------ February 10, 2005 - 20:47 : moshe weitzman i think this is a small, worthwhile patch. this bug plagues drupal.org as well. ------------------------------------------------------------------------ February 11, 2005 - 08:45 : Junyor @jvandyk: Is it just me or is there some junk before the meat of that patch? Better cache header handling would be a welcome addition here[1], but I'm not sure if the suggested patch will solve the problem efficiently. Isn't this change saying that all pages must always be redownloaded if mod_expires is enabled? Won't that cause unnecessary bandwidth and performance overhead? [1] I'm constantly seeing stale pages throughout all authenticated pages with Opera. -- View: http://drupal.org/node/5900 Edit: http://drupal.org/project/comments/add/5900

1 0

[drupal-devel] [bug] "TYPES" is not a legal field name table "BLOCKS"
by andremolnar 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: Philippe Updated by: andremolnar Status: patch Attachment: http://drupal.org/files/issues/block_module_7.patch (1.69 KB) Here is the patch to block.module andremolnar Previous comments: ------------------------------------------------------------------------ February 5, 2005 - 13:31 : Philippe There was a "TYPES" field added to the BLOCKS table. MySQL does not like this; "types" is considered a reserved word. (MySQL-server-4.1.0-0) The field name needs to be changed to something else. ------------------------------------------------------------------------ February 5, 2005 - 13:37 : Philippe Attachment: http://drupal.org/files/issues/types.diff (3.51 KB) here's a patch ------------------------------------------------------------------------ February 9, 2005 - 21:35 : andremolnar Changing Status to patch. I guess we didn't catch this - the field wasn't orginally called types. Good eye. andre ------------------------------------------------------------------------ February 9, 2005 - 21:39 : Dries I suggest we name this field 'nodes'. Also, we'll need an upgrade path for those who are running CVS HEAD. ------------------------------------------------------------------------ February 9, 2005 - 21:54 : JonBob I'm a bit confused by this. The word "types" doesn't show up here: http://dev.mysql.com/doc/mysql/en/reserved-words.html ------------------------------------------------------------------------ February 9, 2005 - 22:12 : andremolnar I remembered seeing it here: http://drupal.org/node/2497 If this is incorrect, then the documentation needs to be changed. But all in all, nodes isn't a bad idea - I can submit a patch along with the upgrade path if 'types' is indeed a reserved word in either mySQL or pgSQL. andre ------------------------------------------------------------------------ February 11, 2005 - 01:22 : ax "TYPES" /is/ a reserved word in mysql 4.1.0 [1] and 4.1.1 [2]. it doesn't show up at http://dev.mysql.com/doc/mysql/en/reserved-words.html because it isn't reserved anymore since some point release later. it isn't even documented in the mysql changelog that it has ever been a reserved word ... once again the suggestion to get rid of sql reserved words issues once and for all [3] [1] http://drupal.org/node/1612 [2] http://info.ccone.at/INFO/MySQL-Doc/manual_Reference.html#Reserved_words [3] http://drupal.org/node/4907#sql-reserved-word-fix ------------------------------------------------------------------------ February 11, 2005 - 14:27 : andremolnar I'll submit a patch later today to use the word 'nodes' in block - along with the database upgrade path. andre -- View: http://drupal.org/node/16817 Edit: http://drupal.org/project/comments/add/16817

1 0

[drupal-devel] [bug] "TYPES" is not a legal field name table "BLOCKS"
by andremolnar 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: Philippe Updated by: andremolnar Status: patch I'll submit a patch later today to use the word 'nodes' in block - along with the database upgrade path. andre andremolnar Previous comments: ------------------------------------------------------------------------ February 5, 2005 - 13:31 : Philippe There was a "TYPES" field added to the BLOCKS table. MySQL does not like this; "types" is considered a reserved word. (MySQL-server-4.1.0-0) The field name needs to be changed to something else. ------------------------------------------------------------------------ February 5, 2005 - 13:37 : Philippe Attachment: http://drupal.org/files/issues/types.diff (3.51 KB) here's a patch ------------------------------------------------------------------------ February 9, 2005 - 21:35 : andremolnar Changing Status to patch. I guess we didn't catch this - the field wasn't orginally called types. Good eye. andre ------------------------------------------------------------------------ February 9, 2005 - 21:39 : Dries I suggest we name this field 'nodes'. Also, we'll need an upgrade path for those who are running CVS HEAD. ------------------------------------------------------------------------ February 9, 2005 - 21:54 : JonBob I'm a bit confused by this. The word "types" doesn't show up here: http://dev.mysql.com/doc/mysql/en/reserved-words.html ------------------------------------------------------------------------ February 9, 2005 - 22:12 : andremolnar I remembered seeing it here: http://drupal.org/node/2497 If this is incorrect, then the documentation needs to be changed. But all in all, nodes isn't a bad idea - I can submit a patch along with the upgrade path if 'types' is indeed a reserved word in either mySQL or pgSQL. andre ------------------------------------------------------------------------ February 11, 2005 - 01:22 : ax "TYPES" /is/ a reserved word in mysql 4.1.0 [1] and 4.1.1 [2]. it doesn't show up at http://dev.mysql.com/doc/mysql/en/reserved-words.html because it isn't reserved anymore since some point release later. it isn't even documented in the mysql changelog that it has ever been a reserved word ... once again the suggestion to get rid of sql reserved words issues once and for all [3] [1] http://drupal.org/node/1612 [2] http://info.ccone.at/INFO/MySQL-Doc/manual_Reference.html#Reserved_words [3] http://drupal.org/node/4907#sql-reserved-word-fix -- View: http://drupal.org/node/16817 Edit: http://drupal.org/project/comments/add/16817

1 0

[drupal-devel] [bug] Clicking on Home after login brings up browser-cached non-logged-in version of home page
by Junyor 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: user system Category: bug reports Priority: critical Assigned to: mathias Reported by: christianlong(a)christianlong.com Updated by: Junyor Status: patch @jvandyk: Is it just me or is there some junk before the meat of that patch? Better cache header handling would be a welcome addition here[1], but I'm not sure if the suggested patch will solve the problem efficiently. Isn't this change saying that all pages must always be redownloaded if mod_expires is enabled? Won't that cause unnecessary bandwidth and performance overhead? [1] I'm constantly seeing stale pages throughout all authenticated pages with Opera. Junyor Previous comments: ------------------------------------------------------------------------ February 16, 2004 - 21:44 : christianlong(a)christianlong.com Attachment: http://drupal.org/files/issues/f_040214_01.txt (6.33 KB) This bug has been discussed before, but not resolved. http://drupal.org/node/view/1740 http://drupal.org/node/view/5686 One suggestion was to change the browser settings. That means I have to tell all my users to change their browser cache settings. Not a great way to run a site. Instead, the login should just work with default browser settings. Currently, login does not work, as described below. I log in to my drupal site, feingold.christianlong.com, and the logged-in home page appears, with my user name. OK. The address bar shows http://feingold.christianlong.com/node?PHPSESSID=867c4e2d57d290a3f59e138524… In the site menu, the "Home" link links to http://feingold.christianlong.com/ Here's the problem: when I click on the "Home" link, the browser takes me to the un-logged-in version of http://feingold.christianlong.com/, which was cached when I was not logged in. When I hit refresh, I do get the logged-in version of the page. So, to restate Start at home page of site, not logged in Log in - this works, and brings up a logged-in version of the page Click "Home" - this is where the problem is. I get the cached version of the home page (from when I was not logged in) Refresh browser - now I see the correct, logged-in version of the home page. It looks like I am logging in OK, but that Drupal is not telling my browser that there is a new version of the home page that it needs to check for Maybe the original (non-logged-in) version of the page is not marked for no-cache, and so when I click on the "Home" link, I get the cached (non-logged-in) version Browsers: MSIE 6 (happens a lot) and Firebird (happens sometimes). Also happens with christianlong.com Attached, find an annotated record of the HTTP header traffic. Thanks Christian ------------------------------------------------------------------------ February 16, 2004 - 21:48 : christianlong(a)christianlong.com Forum discussion is here http://drupal.org/node/view/5669 ------------------------------------------------------------------------ March 4, 2004 - 18:49 : moshe weitzman I am seing this same behavior at http://www.nshp.org [1]. To see for yourself, login using the box on the home page while using IE (Firefox doesn't have a problem). user: mwtest8 pass: testpass Here is the php config of the server [2]. Note that IE is set to "Check for newer pages: automatically" in Options -> Temporary Internet Files -> Settings Thoughts on how to resolve this are welcome. [1] http://www.nshp.org/phpinfo.php [2] http://www.nshp.org/phpinfo.php ------------------------------------------------------------------------ June 16, 2004 - 03:38 : duztin I had this problem, fix the date on your pc, mine was a month ahead and my cookies were expiring too soon. ------------------------------------------------------------------------ July 27, 2004 - 21:18 : killes(a)www.drop.org Setting to cvs. Is the fix proposed by duztin a real fix? if not: what else could we do? ------------------------------------------------------------------------ October 16, 2004 - 22:45 : mathias Attachment: http://drupal.org/files/issues/browser_caching.patch (644 bytes) I'm moving this to critical because in some cases this bug causes the hidden $edit['destination'] value of the login form to be set to 'logout', and to the user it appears they can't login since they are immediately logged out again. Very frustrating. The other side effect of this browser caching bug are, as stated above, the authenticated user will receive stale pages and if you have the login block enabled it will look as though they're were unexplicably logged out. It all depends on your browser and its settings, but to attempt to reproduce: 1. Login to your site. 2. Next, Click the homepage link. If you are served a stale copy of the page, you've hit the bug. This seems to happen more with IE and Safari than Firefox. A potential patch is to have Drupal issue the following header: header('Cache-Control: no-store, no-cache, must-revalidate'); It works, but I don't know the implications this has on other Drupal components such as RSS conditional GETS and gzip page serving. ------------------------------------------------------------------------ October 21, 2004 - 19:30 : mathias Attachment: http://drupal.org/files/issues/browser_caching_0.patch (593 bytes) Thanks to Ethereal [3] and LiveHTTPHeaders [4], I was able to trace this problem to the Cache-Control header being sent by Mac OS X server. On this OS, mod_expires [5] is enabled by default for Apache which sets the Cache-Control time to 60 seconds for dynamically rendered pages. The implications of this action are that once you login, re-visiting any page on the site will result in stale, locally-cached versions if you viewed that page within 60 seconds of logging in. Since you weren't logged in on those pages, the system will have appeared to logged you out. It will keep doing this unless you wait 60 seconds to login. Thus, users perceive this as a failure to successfully log in. This new patch doesn't tweak bootstrap.inc. Instead it uses .htaccess to test if mod_expires is enabled and resets the caching time to 1 second for dynamically rendered content. The benefit of this approach is that it doesn't interfere with other types of caching that may be in place for images, pdf files, etc. # Overload mod_expires variables. <IfModule mod_expires.c> # Reduce the time dynamically generated HTML pages are cache-able. ExpiresDefault A1 </IfModule> Down the road it may also be wise to consider sending our own caching headers to maintain control of our caching environment. I reviewed the following pieces of software, all of which intentionally disable caching by the client browser or proxy caches. I just grabbed the ones I thought were most popular. Plone (Cache-Control parameter is configurable) Wordpress (except for RSS feeds) eZpublish Mambo phpBB And if you're so inclined, here's the relevent code snippets [6] for each piece of software. [3] http://www.ethereal.com/ [4] http://livehttpheaders.mozdev.org/ [5] http://httpd.apache.org/docs/mod/mod_expires.html [6] http://asitis.org/tmp/cms_cache_review.txt ------------------------------------------------------------------------ October 21, 2004 - 19:39 : moshe weitzman I'v seen this bug on drupal.org, so this impacts more than OSX server ... Looks like a nice clean patch to me. ------------------------------------------------------------------------ October 26, 2004 - 17:21 : jvandyk +1 from me. This reduces confusion among end users while retaining caching ability for other mime types. ------------------------------------------------------------------------ October 27, 2004 - 09:05 : wazdog Does this end up having any effect on the RSS conditional GETS? If not, then +1 from me. (The WordPress code does check for RSS, so I'm thinking this may set them to expire too?) ------------------------------------------------------------------------ October 28, 2004 - 02:58 : jvandyk Attachment: http://drupal.org/files/issues/htaccess_0.patch (792 bytes) Regarding the concern about RSS caching: this patch is identical to the previous one except instead of targetting the default we target the text/html MIME type. <IfModule mod_expires.c> # Reduce the time dynamically generated HTML pages are cache-able. ExpiresByType text/html A1 </IfModule> This means that RSS feeds, which are MIME type text/xml, are not affected. ------------------------------------------------------------------------ February 11, 2005 - 02:47 : moshe weitzman i think this is a small, worthwhile patch. this bug plagues drupal.org as well. -- View: http://drupal.org/node/5900 Edit: http://drupal.org/project/comments/add/5900

1 0

[drupal-devel] [bug] Clicking on Home after login brings up browser-cached non-logged-in version of home page
by moshe weitzman 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: user system Category: bug reports Priority: critical Assigned to: mathias Reported by: christianlong(a)christianlong.com Updated by: moshe weitzman Status: patch i think this is a small, worthwhile patch. this bug plagues drupal.org as well. moshe weitzman Previous comments: ------------------------------------------------------------------------ February 16, 2004 - 15:44 : christianlong(a)christianlong.com Attachment: http://drupal.org/files/issues/f_040214_01.txt (6.33 KB) This bug has been discussed before, but not resolved. http://drupal.org/node/view/1740 http://drupal.org/node/view/5686 One suggestion was to change the browser settings. That means I have to tell all my users to change their browser cache settings. Not a great way to run a site. Instead, the login should just work with default browser settings. Currently, login does not work, as described below. I log in to my drupal site, feingold.christianlong.com, and the logged-in home page appears, with my user name. OK. The address bar shows http://feingold.christianlong.com/node?PHPSESSID=867c4e2d57d290a3f59e138524… In the site menu, the "Home" link links to http://feingold.christianlong.com/ Here's the problem: when I click on the "Home" link, the browser takes me to the un-logged-in version of http://feingold.christianlong.com/, which was cached when I was not logged in. When I hit refresh, I do get the logged-in version of the page. So, to restate Start at home page of site, not logged in Log in - this works, and brings up a logged-in version of the page Click "Home" - this is where the problem is. I get the cached version of the home page (from when I was not logged in) Refresh browser - now I see the correct, logged-in version of the home page. It looks like I am logging in OK, but that Drupal is not telling my browser that there is a new version of the home page that it needs to check for Maybe the original (non-logged-in) version of the page is not marked for no-cache, and so when I click on the "Home" link, I get the cached (non-logged-in) version Browsers: MSIE 6 (happens a lot) and Firebird (happens sometimes). Also happens with christianlong.com Attached, find an annotated record of the HTTP header traffic. Thanks Christian ------------------------------------------------------------------------ February 16, 2004 - 15:48 : christianlong(a)christianlong.com Forum discussion is here http://drupal.org/node/view/5669 ------------------------------------------------------------------------ March 4, 2004 - 12:49 : moshe weitzman I am seing this same behavior at http://www.nshp.org [1]. To see for yourself, login using the box on the home page while using IE (Firefox doesn't have a problem). user: mwtest8 pass: testpass Here is the php config of the server [2]. Note that IE is set to "Check for newer pages: automatically" in Options -> Temporary Internet Files -> Settings Thoughts on how to resolve this are welcome. [1] http://www.nshp.org/phpinfo.php [2] http://www.nshp.org/phpinfo.php ------------------------------------------------------------------------ June 15, 2004 - 21:38 : duztin I had this problem, fix the date on your pc, mine was a month ahead and my cookies were expiring too soon. ------------------------------------------------------------------------ July 27, 2004 - 15:18 : killes(a)www.drop.org Setting to cvs. Is the fix proposed by duztin a real fix? if not: what else could we do? ------------------------------------------------------------------------ October 16, 2004 - 16:45 : mathias Attachment: http://drupal.org/files/issues/browser_caching.patch (644 bytes) I'm moving this to critical because in some cases this bug causes the hidden $edit['destination'] value of the login form to be set to 'logout', and to the user it appears they can't login since they are immediately logged out again. Very frustrating. The other side effect of this browser caching bug are, as stated above, the authenticated user will receive stale pages and if you have the login block enabled it will look as though they're were unexplicably logged out. It all depends on your browser and its settings, but to attempt to reproduce: 1. Login to your site. 2. Next, Click the homepage link. If you are served a stale copy of the page, you've hit the bug. This seems to happen more with IE and Safari than Firefox. A potential patch is to have Drupal issue the following header: header('Cache-Control: no-store, no-cache, must-revalidate'); It works, but I don't know the implications this has on other Drupal components such as RSS conditional GETS and gzip page serving. ------------------------------------------------------------------------ October 21, 2004 - 13:30 : mathias Attachment: http://drupal.org/files/issues/browser_caching_0.patch (593 bytes) Thanks to Ethereal [3] and LiveHTTPHeaders [4], I was able to trace this problem to the Cache-Control header being sent by Mac OS X server. On this OS, mod_expires [5] is enabled by default for Apache which sets the Cache-Control time to 60 seconds for dynamically rendered pages. The implications of this action are that once you login, re-visiting any page on the site will result in stale, locally-cached versions if you viewed that page within 60 seconds of logging in. Since you weren't logged in on those pages, the system will have appeared to logged you out. It will keep doing this unless you wait 60 seconds to login. Thus, users perceive this as a failure to successfully log in. This new patch doesn't tweak bootstrap.inc. Instead it uses .htaccess to test if mod_expires is enabled and resets the caching time to 1 second for dynamically rendered content. The benefit of this approach is that it doesn't interfere with other types of caching that may be in place for images, pdf files, etc. # Overload mod_expires variables. <IfModule mod_expires.c> # Reduce the time dynamically generated HTML pages are cache-able. ExpiresDefault A1 </IfModule> Down the road it may also be wise to consider sending our own caching headers to maintain control of our caching environment. I reviewed the following pieces of software, all of which intentionally disable caching by the client browser or proxy caches. I just grabbed the ones I thought were most popular. Plone (Cache-Control parameter is configurable) Wordpress (except for RSS feeds) eZpublish Mambo phpBB And if you're so inclined, here's the relevent code snippets [6] for each piece of software. [3] http://www.ethereal.com/ [4] http://livehttpheaders.mozdev.org/ [5] http://httpd.apache.org/docs/mod/mod_expires.html [6] http://asitis.org/tmp/cms_cache_review.txt ------------------------------------------------------------------------ October 21, 2004 - 13:39 : moshe weitzman I'v seen this bug on drupal.org, so this impacts more than OSX server ... Looks like a nice clean patch to me. ------------------------------------------------------------------------ October 26, 2004 - 11:21 : jvandyk +1 from me. This reduces confusion among end users while retaining caching ability for other mime types. ------------------------------------------------------------------------ October 27, 2004 - 03:05 : wazdog Does this end up having any effect on the RSS conditional GETS? If not, then +1 from me. (The WordPress code does check for RSS, so I'm thinking this may set them to expire too?) ------------------------------------------------------------------------ October 27, 2004 - 20:58 : jvandyk Attachment: http://drupal.org/files/issues/htaccess_0.patch (792 bytes) Regarding the concern about RSS caching: this patch is identical to the previous one except instead of targetting the default we target the text/html MIME type. <IfModule mod_expires.c> # Reduce the time dynamically generated HTML pages are cache-able. ExpiresByType text/html A1 </IfModule> This means that RSS feeds, which are MIME type text/xml, are not affected. -- View: http://drupal.org/node/5900 Edit: http://drupal.org/project/comments/add/5900

1 0

[drupal-devel] [bug] "TYPES" is not a legal field name table "BLOCKS"
by ax 11 Feb '05

11 Feb '05

Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: Philippe Updated by: ax Status: patch "TYPES" /is/ a reserved word in mysql 4.1.0 [1] and 4.1.1 [2]. it doesn't show up at http://dev.mysql.com/doc/mysql/en/reserved-words.html because it isn't reserved anymore since some point release later. it isn't even documented in the mysql changelog that it has ever been a reserved word ... once again the suggestion to get rid of sql reserved words issues once and for all [3] [1] http://drupal.org/node/1612 [2] http://info.ccone.at/INFO/MySQL-Doc/manual_Reference.html#Reserved_words [3] http://drupal.org/node/4907#sql-reserved-word-fix ax Previous comments: ------------------------------------------------------------------------ February 5, 2005 - 14:31 : Philippe There was a "TYPES" field added to the BLOCKS table. MySQL does not like this; "types" is considered a reserved word. (MySQL-server-4.1.0-0) The field name needs to be changed to something else. ------------------------------------------------------------------------ February 5, 2005 - 14:37 : Philippe Attachment: http://drupal.org/files/issues/types.diff (3.51 KB) here's a patch ------------------------------------------------------------------------ February 9, 2005 - 22:35 : andremolnar Changing Status to patch. I guess we didn't catch this - the field wasn't orginally called types. Good eye. andre ------------------------------------------------------------------------ February 9, 2005 - 22:39 : Dries I suggest we name this field 'nodes'. Also, we'll need an upgrade path for those who are running CVS HEAD. ------------------------------------------------------------------------ February 9, 2005 - 22:54 : JonBob I'm a bit confused by this. The word "types" doesn't show up here: http://dev.mysql.com/doc/mysql/en/reserved-words.html ------------------------------------------------------------------------ February 9, 2005 - 23:12 : andremolnar I remembered seeing it here: http://drupal.org/node/2497 If this is incorrect, then the documentation needs to be changed. But all in all, nodes isn't a bad idea - I can submit a patch along with the upgrade path if 'types' is indeed a reserved word in either mySQL or pgSQL. andre -- View: http://drupal.org/node/16817 Edit: http://drupal.org/project/comments/add/16817

1 0