Per-gallery image count in the database

I would, but let's define what would need addressed:

1) views integration. This would actually be easier with the count stored in the db, but we'd have to pull the current implementation out.
2) hook_update_N() implementation to pre-populate the db
3) Define what the count represents - is it the number of Published images, or total images? Currently we do the former by default, but it's user-configurable by editing the sort view.

I don't think it would be too hard, because we already have extrapolated the code for counting into atomic helper functions.

All that being said, I think it may be easier to simply extend the current implementation of node_gallery_views_handler_image_count to be used in sorts and filters. There, we're using drupal's db cache and static caches to store the lists of images, and no matter how fast the db is, you can't outrun a count($array) :)

Thoughts?

This looks really solid -- nice work. Just a few points:

+++ b/node_gallery.incundefined
@@ -686,6 +686,31 @@ function node_gallery_get_image_count($gid, $reset = FALSE) {
+  $all = (int) db_result(db_query("SELECT COUNT(*) FROM {node_gallery_images} WHERE gid = %d", $gid));

We should do one query here that gets the nid and the status. Then we can iterate over the results incrementing both counters if it's published, but only $all if it's not.

This won't save a lot on one image, but if I update a large set of images, it could save a lot of queries.

+++ b/node_gallery.installundefined
@@ -860,3 +880,72 @@ function node_gallery_update_6304() {
+  $fields = array(

To save changing schema definitions in two spots later, you should call node_gallery_schema() and pull your field definitions from that.

+++ b/node_gallery.installundefined
@@ -860,3 +880,72 @@ function node_gallery_update_6304() {
+  $indexes = array(

Same here - use node_gallery_schema to get your index definitions.

+++ b/node_gallery.installundefined
@@ -860,3 +880,72 @@ function node_gallery_update_6304() {
+  $res = db_query("SELECT gid, count(*) AS c FROM {node_gallery_images} GROUP BY gid");

Same here as above, let's query once, and iterate over the results and increment the counters. Some users have a lot of galleries on crappy webhosts with slow MySQL db's.

+++ b/views2inc/node_gallery.views.incundefined
@@ -105,13 +105,46 @@ function node_gallery_views_data() {
+    'help' => t('The number of images in the gallery, including unpublished.'),

t('The total number of published and unpublished images in the gallery.')

If we could get these fixes in, I'd love for one community member to test this before I commit, but if everyone's busy I'll see if I can get some time. I'm headed out of town for some family time and won't be back until middle of the week next week.

Also, why waste time on removing lines if I already spent it on writing them ?
Umm, because of DRY? However, thanks for the reference, I didn't know that about hook_schema().

Also, if you could run that patch through coder module, you'll see a bunch of whitespace issues. If you can fix those now it'll save me time fixing them before the 3.0 release.

That's up to you, but I think this does belong in "core" NG. I just didn't want to drop in an untested feature right at the 3.0 release, and I didn't have the time to test it myself. With 3.0 out of the way, if you can re-roll the patch against HEAD (doesn't apply cleanly anymore), I'll commit it in so we can get some testers that way.

I agree that this should go into core. Good milestone for the next dev cicle for the D6 branch.

Committed to 6.x-3.x-dev.