This PR replaces the IS NOT NULL clause with > 0 in both methods. > 0 is correct for nullable columns too because NULL > 0 evaluates to NULL, which WHERE treats as false.

@genr8r Can we be sure that it works this way on all supported databases (MySQL, MariaDB and PostgreSQL)?

And even if it does, it is hardly understandable for a code reader who is not an SQL expert.

For better readability and to make sure it always works like we want, I would suggest:

$query->where($db->quoteName('checked_out') . ' IS NOT NULL');
$query->where($db->quoteName('checked_out') . ' > 0');

That will result in

WHERE `checked_out` IS NOT NULL AND `checked_out` > 0

That works the same way, and I don't really expect a performance issue with the additional IS NOT NULL condition.

That's just an idea or suggestion, not a change request by me.

What do you think about it?

@richard67 Thanks for the careful read.

On portability: this is ANSI SQL three-valued logic. NULL > 0 evaluates to UNKNOWN, and WHERE treats UNKNOWN as not-true, so the row is filtered. MySQL, MariaDB, and PostgreSQL all behave this way consistently (SQL-92 behavior, predates all three).

On the underlying convention in core: the "both NULL and 0 mean released" semantic is set by libraries/src/Table/Table.php on 5.4-dev @ 83d4bf7afe:

• Table::checkIn() writes either NULL or 0 depending on $_supportNullValue (Table.php#L1286-L1291):

  $nullID = $this->_supportNullValue ? 'NULL' : '0';
  ...
  ->set($db->quoteName($checkedOutField) . ' = ' . $nullID)

• Table::isCheckedOut() treats both as "not locked" via PHP truthy test (Table.php#L1450):

  if (!$against || ($against == $with)) {
      return false;
  }

  !0 and !null both short-circuit to "not checked out".

So both states (NULL and 0) genuinely occur on this column after a clean release, and need to be filtered out either way. checked_out > 0 does that in one predicate; IS NOT NULL AND > 0 does it in two with identical SQL semantics.

On the readability of the two-condition form: my hesitation is that the redundant predicate reads as authorial uncertainty. A reader who doesn't know the NULL semantics sees both conditions and asks "why both?", which is the same knowledge gap the suggestion was meant to close. It moves the confusion rather than removing it.

If reader comprehension is the underlying concern, I'd happily add a short inline comment instead:

// checked_out > 0 also excludes NULL rows (NULL > 0 is UNKNOWN, treated as false by WHERE),
// matching Table::checkIn() which writes either NULL or 0 to release a lock.
$query->where($db->quoteName('checked_out') . ' > 0');

That keeps the predicate clean while making both the NULL behavior and the cross-reference explicit. Let me know which way you'd prefer.

@genr8r A comment would also be ok for me … but if it is more than one line we use a certain code style for that and not multiple // one line comments.

@richard67 Good point on the style. Switched to the block form and pushed it:

/*
 * checked_out > 0 excludes both release states Table::checkIn() can write:
 * NULL (when $_supportNullValue) and 0 (otherwise).
 * NULL > 0 evaluates to UNKNOWN, which WHERE treats as false.
 */
$query->where($db->quoteName('checked_out') . ' > 0');

I have tested this item ✅ successfully on 6714a20

Tested successfully but did not encounter the Text::plural('COM_CHECKIN_N_ITEMS_CHECKED_IN', 0) issue before the PR.

I have tested this item ✅ successfully on 6714a20

Tested successfully but did not encounter the Text::plural('COM_CHECKIN_N_ITEMS_CHECKED_IN', 0) issue before the PR.

Please reduce the comment to a one-liner: // Both NULL and 0 represent records that are not checked out.

I have tested this item ✅ successfully on 6714a20

I changed the admin module data in phpMyAdmin with a checked_out of '0' and checked_out_time 'null' (by flag)
Maintenance: Global Check-in showed the module for checkin before apply of PR and did not display it after
Additional tested article state checked out and it appeared without the "admin module" checkout state

I have tested this item ✅ successfully on 6714a20

I changed the admin module data in phpMyAdmin with a checked_out of '0' and checked_out_time 'null' (by flag)
Maintenance: Global Check-in showed the module for checkin before apply of PR and did not display it after
Additional tested article state checked out and it appeared without the "admin module" checkout state

@MacJoom Done, pushed in 9c956c1. Both occurrences now use your wording verbatim:

// Both NULL and 0 represent records that are not checked out.
$query->where($db->quoteName('checked_out') . ' > 0');

Comment-only change, no logic difference from the commit @QuyTon and @ThomasFinnern tested.

RTC

RTC

RTC

then the same change should be made here

and also i'm afraid that we need a better explanation i don't know where is better in code comments or in some manuals
something like :

// Both NULL and 0 represent records that are not checked out.
// NULL > 0 evaluates to NULL/false in SQL; 0 > 0 is false.
// Only user IDs (positive integers) indicate active locks.

@alikon Good catch on the task plugin. plugins/task/globalcheckin/src/Extension/Globalcheckin.php has the identical if/else on column nullability, so the same predicate mismatch is there. Happy to fold that into this PR so both call sites stay consistent. (Impact there is milder in practice: with the default delay > 0, the checked_out_time < $delayTime clause already filters checked_out = 0 rows because their time is NULL, so it only bites at delay = 0 or on rows with a stale non-NULL checked_out_time. Same defect regardless.)

On the comment, I need a steer before I touch it. The three-line version you've quoted is essentially what was in 6714a20; @MacJoom asked me on 07-10 to reduce it to the single line, I did that in 9c956c1, and the RTC followed. I don't want to revert one maintainer's request on another's say-so and start a ping-pong.

@MacJoom @richard67 @alikon, could you settle on one form? I'll implement whichever you agree on and apply it identically at all three sites (both CheckinModel methods plus the task plugin).

If the fuller explanation wins, note that @richard67 flagged earlier that multi-line inline comments should use the block style rather than stacked // lines, which matches core (ApplicationModel, InstallModel, et al.). So it would look like:

/*
 * Both NULL and 0 represent records that are not checked out.
 * NULL > 0 evaluates to UNKNOWN, which WHERE treats as false, and 0 > 0 is false.
 * Only a positive user ID indicates an active lock.
 */
$query->where($db->quoteName('checked_out') . ' > 0');

I've used "UNKNOWN, which WHERE treats as false" rather than "NULL/false" since that's the precise three-valued-logic behavior, but I'm not attached to the wording.

Heads up that adding the plugin fix will drop the RTC label and invalidate the two successful tests from @QuyTon and @ThomasFinnern, so it goes back through the test queue. I think that's the right trade rather than shipping the fix at only two of the three call sites.

On the comment, I need a steer before I touch it. The three-line version you've quoted is essentially what was in 6714a20; @MacJoom asked me on 07-10 to reduce it to the single line, I did that in 9c956c1, and the RTC followed. I don't want to revert one maintainer's request on another's say-so and start a ping-pong.

@MacJoom @richard67 @alikon, could you settle on one form? I'll implement whichever you agree on and apply it identically at all three sites (both CheckinModel methods plus the task plugin).

I am ok with the short comment, but I can also live with the long comment. So @MacJoom and @alikon have to fight it out, or we get more opinions.

Heads up that adding the plugin fix will drop the RTC label and invalidate the two successful tests from @QuyTon and @ThomasFinnern, so it goes back through the test queue. I think that's the right trade rather than shipping the fix at only two of the three call sites.

I agree. Would be good to fix it all at once in the same way. It would also need to add a test case for the global checking task plugin to the testing instructions.

If the field is well documented in the manual (which i can not confirm now) then we don't need a comment in the code at all. so one line should be enough. Imagine all decisions in the code be documented with a block...

How about the following one-liner?

// The following expression evaluates to false for values <= 0 as well as for NULL values

@genr8r @MacJoom @alikon Opinions please.

for the comment i'm ok with the one-liner
it took me sometimes to understand when reading the code but probably that's me 🥲

@richard67 @MacJoom @alikon Sounds like consensus, so I've pushed it in 7d8fe7f.

richard67's one-liner is now at all three call sites:

// The following expression evaluates to false for values <= 0 as well as for NULL values
$query->where($db->quoteName('checked_out') . ' > 0');

I also folded in the task plugin fix @alikon spotted. plugins/task/globalcheckin/src/Extension/Globalcheckin.php had the identical if/else on column nullability and now uses the same predicate.

As flagged, adding the plugin drops RTC and invalidates the successful tests from @QuyTon and @ThomasFinnern, since this is no longer a comment-only change. Apologies for the extra round. Testing instructions in the description now include a task plugin case, per @richard67's request. Note that on the plugin side the difference is a no-op write either way, so it reads as a correctness fix rather than a visible behavior change; the steps spell that out so nobody burns time hunting for a symptom that isn't there.

I've also retitled the PR, since "CheckinModel" no longer covers the full scope.

I have tested this item ✅ successfully on 7d8fe7f

I have tested this item ✅ successfully on 7d8fe7f

I have tested this item ✅ successfully on 7d8fe7f

First tests like above,
Scheduled Tasks:
checked_out = 1, checked_out_time = '2026-07-16 15:51:22' ==> null, null
checked_out = 0, checked_out_time = '2026-07-16 15:59:36' ==> 0, 2026-07-16 15:59:36

I have tested this item ✅ successfully on 7d8fe7f

First tests like above,
Scheduled Tasks:
checked_out = 1, checked_out_time = '2026-07-16 15:51:22' ==> null, null
checked_out = 0, checked_out_time = '2026-07-16 15:59:36' ==> 0, 2026-07-16 15:59:36

RTC

RTC

✅ Final test before merge with JBT and MariaDB

• Seen content table is listed in Global Check-in after hacking article's checked_out with 0 before PR
• Applied PR with Patch Tester
  • content table is no more listed in Global Check-in for an article with checked_out = 0
  • content table is listed in Global Check-in for an article being edited
  • Tested banners and guidedtours as two more tables:
    • Before PR with checked_out = 0 they are listed in Global Check-in
    • After PR they are no more listed in Global Check-in
    • If a banner or a guided tours is being edited, they are listed Global Check-in
  • New installed Joomla with PostgreSQL
    • It looks like PostgreSQL does not have the problem, even with checked_out = 0 for an article entry, the content table is not listed in Global Check-in
    • Applied PR with Patch Tester
      • content table is not listed in Global Check-in for an article with checked_out = 0
      • content table is listed in Global Check-in for an article being edited
      • Using Global Check-in removes an edited article check-out
• Not tested task as it is already tested and is the identical source code change

Just something to consider regarding this global change: the following tables contain a checked_out column:

• banners
• banner_clients
• categories
• contact_details
• content
• extensions
• fields
• fields_groups
• finder_filters
• guidedtours
• guidedtour_steps
• menu
• modules
• newsfeeds
• scheduler_tasks
• tags
• update_sites
• user_notes
• workflows
• workflow_stages

⚠️ Question: There are other places where checked_out is compared against NULL. Should these places also be changed?

• Searched with find . -name \*.php | xargs grep -n "'checked_out'"| grep NULL
  • libraries/src/Table/MenuType.php:222
  • administrator/components/com_banners/src/Helper/BannersHelper.php:59
  • administrator/components/com_templates/src/Model/StyleModel.php:473
  • administrator/components/com_templates/src/Model/StyleModel.php:493

⚠️ Question: There are other places where checked_out is compared against NULL. Should these places also be changed?

• Searched with find . -name \*.php | xargs grep -n "'checked_out'"| grep NULL
  • libraries/src/Table/MenuType.php:222
  • administrator/components/com_banners/src/Helper/BannersHelper.php:59
  • administrator/components/com_templates/src/Model/StyleModel.php:473
  • administrator/components/com_templates/src/Model/StyleModel.php:493

⚠️ Question: There are other places where checked_out is compared against NULL. Should these places also be changed?

* Searched with `find . -name \*.php | xargs grep -n "'checked_out'"| grep NULL`
  
  * libraries/src/Table/MenuType.php:222
  * administrator/components/com_banners/src/Helper/BannersHelper.php:59
  * administrator/components/com_templates/src/Model/StyleModel.php:473
  * administrator/components/com_templates/src/Model/StyleModel.php:493

These other places handle core table written by core only, so checking for NULL only is sufficient. For the global checkout handled in this PR here we have to handle also tables of 3rd party extensions, that's why we cannot rely on NULLbeing used.

Thank you very much @genr8r for your contribution. Thanks to @richard67, @alikon and @MacJoom for supporting. Thank you to @ThomasFinnern, @QuyTon and @alikon for testing.