API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Database%...

The $options param just says:

> array $options: (optional) An array of options on the query.

This is not very useful, and requires a developer to go digging in the source to figure this out.

The query() method explains more:

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Database%...

The various methods such as insert(), update(), delete() need to do the same.

Though note that the link to the other method isn't properly being made:

> See the documentation for self::defaultOptions() for details

CommentFileSizeAuthor
#15 interdiff-12-15.txt1.95 KBmsankhala
#15 updated-doc-2772321-15.patch2.42 KBmsankhala
#12 removedreturn-2772321-12.patch2.74 KBvimal_nadar
#9 2772321-9.patch3.09 KBdhruveshdtripathi
#4 2772321-4.patch3.11 KBshashikant_chauhan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Assigned: Unassigned » shashikant_chauhan

working on the issue.

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Status: Active » Needs review
FileSize
2772321-4.patch3.11 KB

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ commented
Status: Needs review » Needs work

I really like this, this is a good improvement. This patch doesn't apply anymore.

dhruveshdtripathi’s picture

dhruveshdtripathi CreditAttribution: dhruveshdtripathi as a volunteer and at DevsAdda commented
Assigned: shashikant_chauhan » Unassigned
Status: Needs work » Needs review
FileSize
2772321-9.patch3.09 KB

Made fresh patch for 8.6.x

joachim’s picture

joachim CreditAttribution: joachim commented

This looks good.

+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -862,12 +868,18 @@ public function upsert($table, array $options = []) {
+   *   details. Typically, $options['return'] will be set by a default or by a
+   *   querybuilder, and should not be set by a user.

But do we need to mention the 'return' option in each method? The docs for that are pretty clean on the method we link to.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ commented
Status: Needs review » Needs work

I agree with #10, we shouldn't copy that information in every time.

vimal_nadar’s picture

vimal_nadar CreditAttribution: vimal_nadar commented
Status: Needs work » Needs review
FileSize
removedreturn-2772321-12.patch2.74 KB

Updated the patch as per comment #10.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs review » Reviewed & tested by the community

I can confirm the patch #12 applies cleanly and fixes the docblock as per issue description. Also, removes the $option['return'] mention as per #10.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -808,12 +808,17 @@ public function select($table, $alias = NULL, array $options = []) {
+   *   (optional) An associative array of options to control how the query is
+   *   run. The given options will be merged with
+   *   \Drupal\Core\Database\Connection::defaultOptions(). See the
+   *   documentation for \Drupal\Core\Database\Connection::defaultOptions() for
+   *   details.

    We don't need the See the documentation for \Drupal\Core\Database\Connection::defaultOptions() for details.. That's what the @see is for. 

  2. +++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -879,13 +889,19 @@ public function update($table, array $options = []) {
    *   The table to use for the delete statement.
+   *
    * @param array $options

    The new line added here is incorrect.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
updated-doc-2772321-15.patch2.42 KB
interdiff-12-15.txt1.95 KB

Here is updated patch.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Reviewed & tested by the community

  • catch committed 4ba4c09 on 8.7.x 
    Issue #2772321 by msankhala, shashikant_chauhan, dhruveshdtripathi,...

  • catch committed a94c8d5 on 8.6.x 
    Issue #2772321 by msankhala, shashikant_chauhan, dhruveshdtripathi,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.7.x-dev » 8.6.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.7.x and cherry-picked to 8.6.x. Thanks!

Status: Fixed » Closed (fixed)

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