Deprecated function: array_slice(): Passing null to parameter #2 ($offset) of type int is deprecated in Drupal\Core\Config\Entity\Query\Query->execute()

Created PR with the solution from the description

This is also an issue when trying to use JSON:API to load field storage configs /jsonapi/field_storage_config/field_storage_config

The code from MR fixes the problem

Uploading patch for 9.5.x made out of MR because it is needed for my project. I want to mention, that this is done not for credit hunting. Please do not give credits and do not use the commit message with me as author.

Patch #9 can be applied to drupal/core (10.0.9) as well. I will test it on 10.1.0 shortly.

It can be applied to 10.1.0 as well.

The patch #9 works as expected in Drupal 10.1, this is affect custom entity that use EntityConfig, thanks for the contribution @_tarik_ → .

Interestingly this causes a regression. The fix in the MR will return the full result set, which is the expected behavior according to the documentation:

  /**
   * Defines the range of the query.
   *
   * @param int|null $start
   *   (optional) The first record from the result set to return. If NULL,
   *   removes any range directives that are set.
   * @param int|null $length
   *   (optional) The maximum number of rows to return. If $start and $length
   *   are NULL, then a complete result set will be generated. If $start is
   *   not NULL and $length is NULL, then an empty result set will be
   *   generated.
   *
   * @return $this
   */
  public function range($start = NULL, $length = NULL);

However, in the case when $start is NULL, but $length is set, the current implementation is NOT removing the range directives but is returning the number of results passed in $length.

In this issue we should only fix the deprecation warning but not change the existing behavior (even though it might be wrong). Let's open a separate issue to address the discrepancy between the documentation and the observed behavior.

I will start a new MR with a fix that preserves the existing behavior.

Created issue to address the discrepancy between the documentation and the existing behavior: 🐛 Omitting $start in entityQuery->range() does not return full result set Active .

pfrenssen → changed the visibility of the branch 3358581-deprecated-function-arrayslice to hidden.

Tests have run, they correctly detect the deprecation warning (full test output at https://git.drupalcode.org/issue/drupal-3358581/-/jobs/609008#L5940):

---- Drupal\KernelTests\Core\Entity\ConfigEntityQueryTest ----
Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Fail      Other      phpunit-370.xml      0 Drupal\KernelTests\Core\Entity\Conf
    PHPUnit Test failed to complete; Error: PHPUnit 9.6.15 by Sebastian
    Bergmann and contributors.
    
    Testing Drupal\KernelTests\Core\Entity\ConfigEntityQueryTest
    ........                                                            8 / 8
    (100%)
    
    Time: 00:07.274, Memory: 4.00 MB
    
    OK (8 tests, 228 assertions)
    
    Unsilenced deprecation notices (2)
    
      2x: array_slice(): Passing null to parameter #2 ($offset) of type int is
    deprecated
        2x in ConfigEntityQueryTest::testSortRange from
    Drupal\KernelTests\Core\Entity

So the proposed solution mentions doing what Select::range() does which is

$this->range = $start !== NULL ? ['start' => $start, 'length' => $length] : [];

But a different solution here. Could it be documented in the issue summary why an alternative was needed.

Updated issue summary. There is a followup issue to address this difference with Select::range(): 🐛 Omitting $start in entityQuery->range() does not return full result set Active .

Thanks for linking that.

Test-only feature shows the coverage and the solution appears correct.

This appears to be a small enough change that I feel comfortable RTBC'ing it.

Good idea to fix this deprecation here and then decide if we can should fix the method or the docs over in the linked issue.

Backported as a valid bug fix to 10.2.x.

Committed and pushed 8226fe885a to 11.x and 2deae09a5f to 10.2.x. Thanks!

FYI this introduced a behaviour change to the ::range method.

Previously calling ->range(0, 0) would return no results, now it returns all results.

We found this with a failing test in aggregator module. We suspsect the test is redundant and will likely remove it but wanted to point it out in case someone else came across something similar.

Hmm, should we undo this and add test coverage for that? $length of 0 probably should return 0 items?

If its not in a tagged release perhaps?

Reverted, let's add a test for #28/29 and rework this a bit.

pfrenssen → changed the visibility of the branch 3358581-entity-query-omit-range-start to hidden.

I updated the patch with a test + fix for the regression reported in #28/29 but I'm having some trouble with the issue fork. The spellcheck job cannot find the 10.2.x branch?

pfrenssen → changed the visibility of the branch 10.2.x to hidden.

pfrenssen → changed the visibility of the branch 11.x to hidden.

Manually pushing the 10.2.x and 11.x branches to the issue fork has done the trick. Tests are passing!

Not sure the scenario but could we add a test scenario for NULL, NULL since we doing that check.

@smustgrave the first test case is doing a (NULL, NULL) check implicitly: https://git.drupalcode.org/project/drupal/-/merge_requests/6374/diffs#95...

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Not sure what the bot is picking up but the changes in the MR seem fine.

I'm triaging RTBC issues → . I read the IS, the comments and the MR. I didn't find any unanswered questions or other work to do.

Leaving at RTBC.

Committed and pushed 6227621d38 to 11.x and 418d0cb65c to 10.3.x and 7afb8b2824 to 10.2.x. Thanks!

Backported to 10.2.x as a bug fix. This time without a behaviour change.

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