Spin off from #3399150: [PP-1] Enable dynamic queries to produce SQL with positional placeholders.

Problem/Motivation

\Drupal\Core\Database\Query\Insert::execute() is overriden in all core implementations, and therefore the base implementation is missing test coverage.

Proposed resolution

Convert \Drupal\Core\Database\Query\Insert to abstract class and make its ::execute() method abstract too.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3608493

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes

mondrake opened merge request !16219

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
daffie’s picture

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

All code changes look good to me.
The change is right.
The sqlsrv database driver has its own implementation of Insert::Execute().
I do not see BC breaking.
For me it is RTBC.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs change record

This looks fine but I think we need a change record.

Also, should there be an 11.x version of this where the method triggers a deprecation error?

mondrake opened merge request !16387

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: -Needs change record

Added a 11.x MR that only triggers a deprecation in case the methods are called, and a draft CR.

daffie’s picture

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

Both remarks of @catch have been addressed.
Back to RTBC.

catch closed merge request !16219

  • catch committed 5f496970 on 11.x 
    fix: #3608493 \Drupal\Core\Database\Query\Insert::execute() is overriden...

  • catch committed f6012fd4 on main 
    fix: #3608493 \Drupal\Core\Database\Query\Insert::execute() is overriden...

catch closed merge request !16387

catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and the respective MR to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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