Remove per-table prefixing [#3124382]

Problem/Motivation

In #3106531: Notify in Status Report that per-table database prefixes are no longer supported, and will throw errors in Drupal 10.0 we deprecated per-table prefixing and the 'prefix' connection array key in array form.

The per-table prefixing needs to be removed in D10.

Proposed resolution

Deprecated for removal in D11 the class variables (all protected): Drupal\Core\Database\Connection::$prefixes, Drupal\Core\Database\Connection::$prefixSearch, Drupal\Core\Database\Connection::$prefixReplace and Drupal\Core\Database\Connection::$unprefixedTablesMap.
Deprecated for removal in D11 the method Drupal\Core\Database\Connection::getUnprefixedTablesMap()
Added the class variables (private): Drupal\Core\Database\Connection::$prefix and Drupal\Core\Database\Connection::$tablePlaceholderReplacements.
::setPrefix() now accepts only strings.

Remaining tasks

User interface changes

API changes

See proposed solution.

Data model changes

Release notes snippet

Drupal previously supported per-table prefixing for complex multisite setups. This functionality has been deprecated since Drupal 8.2, and warnings are displayed on the status reports of sites stil using this functionality since Drupal 9.3.0. The functionality has been removed from Drupal 10. See the change record for alternatives to per-table prefixing.

Comment	File	Size	Author
#6	3124382-6.patch	14.37 KB	mondrake
#5	3124382-5.patch	15.35 KB	mondrake
#4	3124382-4.patch	7.98 KB	mondrake
#3	3124382-3.patch	2.99 KB	mondrake

Issue fork drupal-3124382

Show commands

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

Add & fetch this issue fork’s repository

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

Add & fetch this issue fork’s repository

1 hidden branch

3124382-pp-1-remove-per-table

changes, plain diff MR !1607

Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

2 April 2020 at 09:01

mondrake created an issue. See original summary.

Comment #2

daffie commented 3 April 2020 at 07:50

Issue summary:

View changes

Comment #3

mondrake

🇮🇹

commented 7 September 2021 at 20:37

Status	File	Size
new	3124382-3.patch	2.99 KB

Just a curiosity to see what breaks.

Comment #4

mondrake

🇮🇹

commented 20 October 2021 at 17:43

Status	File	Size
new	3124382-4.patch	7.98 KB

Comment #5

mondrake

🇮🇹

commented 20 October 2021 at 19:29

Status	File	Size
new	3124382-5.patch	15.35 KB

Comment #6

mondrake

🇮🇹

commented 20 October 2021 at 19:36

Status	File	Size
new	3124382-6.patch	14.37 KB

3 files were hidden/shown/deleted

Status	File	Size
hidden	3124382-3.patch	2.99 KB
hidden	3124382-4.patch	7.98 KB
hidden	3124382-5.patch	15.35 KB

Comment #7

mondrake

🇮🇹

commented 20 October 2021 at 19:43

Status:	Active	» Postponed
Related issues:		+#3210310: Adjust Database API to remove deprecated Drupal 9 code in Drupal 10

Need to do #3210310: Adjust Database API to remove deprecated Drupal 9 code in Drupal 10 first.

Comment #8

mondrake

🇮🇹

commented 30 December 2021 at 13:45

Title:

Remove per-table prefixing

» [PP-1] Remove per-table prefixing

Comment #9

4 January 2022 at 15:20

mondrake opened merge request !1607

Comment #10

mondrake

🇮🇹

commented 4 January 2022 at 15:21

Title:	[PP-1] Remove per-table prefixing	» Remove per-table prefixing
Status:	Postponed	» Needs review

1 file was hidden/shown/deleted

Status	File	Size
hidden	3124382-6.patch	14.37 KB

Comment #11

mondrake

🇮🇹

commented 5 January 2022 at 08:18

Issue summary:

View changes

Comment #12

daffie commented 5 January 2022 at 11:02

Status:

Needs review

» Needs work

In the method Drupal\pgsql\Driver\Database\pgsql\Schema::findTables() is the deprecated method getUnprefixedTablesMap() called.

Patch looks good.

Comment #13

mondrake

🇮🇹

commented 5 January 2022 at 15:17

Status:

Needs work

» Needs review

Thanks @daffie

Comment #14

daffie commented 5 January 2022 at 20:49

Status:	Needs review	» Needs work
Issue tags:		+Needs change record

The MR is for me RTBC.

I will work on the CR tomorrow.

Comment #15

daffie commented 6 January 2022 at 07:42

Issue summary:	View changes
Status:	Needs work	» Reviewed & tested by the community
Issue tags:	-Needs change record

All code changes look good to me.
The IS and the CR are in order.
For me it is RTBC.

Comment #16

mondrake

🇮🇹

commented 6 January 2022 at 09:37

Status:

Reviewed & tested by the community

» Needs work

we need to update the MR with the actual number of the CR

Comment #17

mondrake

🇮🇹

commented 6 January 2022 at 11:43

Status:

Needs work

» Needs review

Comment #18

daffie commented 6 January 2022 at 12:38

Status:

Needs review

» Reviewed & tested by the community

we need to update the MR with the actual number of the CR

Oeps!

It is fixed.
Back to RTBC.

Comment #19

alexpott

he/they

English

🇪🇺🌍

commented 7 January 2022 at 02:26

Status:

Reviewed & tested by the community

» Needs work

I think the change as it current is results in an change in the output of function unexpectedly.

Comment #20

mondrake

🇮🇹

commented 7 January 2022 at 10:29

Status:

Needs work

» Reviewed & tested by the community

That code comes straight from D8 https://git.drupalcode.org/project/drupal/-/blob/8.9.x/core/lib/Drupal/C...

Doing some archeocodololgy, AFAICS the idea was to convert the 'prefix' element (that was an array there) to a list of tables that had a different prefix than the default -- $table_name as a variable name is not a very happy one, it's the key of the 'prefix' array.

In other words, a 'prefix' array like

[
  'default' => 'bar_',
  'users' => 'foo_',
  'node => 'qux_',
]

would result in ::getUnprefixedTablesMap() returning

[
  'foo_users' => 'users',
  'qux_node => 'node',
]

Since we no longer have per-table prefixes as a result of this patch, ::getUnprefixedTablesMap() would return an empty array which is correct IMHO.

Comment #21

catch

he/him

English

commented 7 February 2022 at 16:52

Status:

Reviewed & tested by the community

» Needs review

Couple of questions on the MR.

Comment #22

mondrake

🇮🇹

commented 7 February 2022 at 18:16

Thanks @catch, addressed your points.

Comment #23

daffie commented 7 February 2022 at 21:11

Status:

Needs review

» Reviewed & tested by the community

Both points of @catch have been addressed.

The "extra_prefix" stuff from the SQLite database driver makes it for me that we can only do this in D10 and remove it in D11. Just a lot cleaner and it is not in the way.

Comment #24

7 February 2022 at 21:26

catch committed e1c6ba3 on 10.0.x

Issue #3124382 by mondrake, daffie, alexpott, catch: Remove per-table...

Comment #25

catch

he/him

English

commented 7 February 2022 at 21:26

Status:

Reviewed & tested by the community

» Fixed

Committed e1c6ba3 and pushed to 10.0.x. Thanks!

Comment #26

mondrake

🇮🇹

commented 7 February 2022 at 21:31

Published CR

Comment #27

7 February 2022 at 21:32

mondrake closed merge request !1607

Comment #28

xjm

she/her

English

commented 16 February 2022 at 20:27

I went ahead and attached the original CR to this issue (from all the way back in Drupal 8.2): https://www.drupal.org/node/2768219

I'm confused though. The IS says:

The functionality needs to be removed in D10.

But then it goes onto say a bunch of stuff is deprecated for removal in D11, and the patch is adding deprecations for D11 removal as well, not removing any existing deprecations.

I would've expected this issue to wait for 10.1.x. Do we need to mention it in the release notes? Or does the followup to actually remove the feature get mentioned in the 11.0.0 release notes?