Problem/Motivation

The Database Schema API supports defining and managing database tables, but it has no equivalent support for SQL views.

Modules that need an SQL view must currently execute DDL directly and manage database portability, lifecycle, dependencies, and test cleanup themselves.

Core would benefit from this with at least a couple of use cases:

  • Deprecating/renaming database tables. For example, Workspaces would like to rename the workspace_association table to workspace_tracker. BC could be provided for this rename by creating a virtual workspace_association table that matches the data of the new one.
  • After #3343634: Add "json" as core data type to Schema and Database API is done, we can extract data from JSON columns into table structures, for example to match the structure of dedicated entity field tables.

Proposed resolution

Extend the Database Schema API so modules can define, create, discover, replace, and remove SQL views in a database-independent way.

Modules can declare views alongside tables in hook_schema(). Tables are created before their views and views are removed before their underlying tables.

Example:

$views[] = new View(
  name: 'example_active_items',
  query: new ViewQuery(
    'SELECT [id], [label] FROM {example_item} WHERE [active] = 1'
  ),
  description: 'Active example items.',
);

return new Schema(
  type: SchemaDefinitionType::Module,
  name: 'example',
  tables: $tables,
  views: $views,
);

API changes

New schema definition classes:

  • Drupal\Core\Database\SchemaDefinition\View
  • Drupal\Core\Database\SchemaDefinition\ViewQuery

New methods on Drupal\Core\Database\Schema:

public function createView(
  string $name,
  string $query,
  bool $replace = FALSE,
): void;

public function createViewFromDefinition(View $view): void;

public function dropView(string $name): bool;

public function dropViews(array $views): void;

public function viewExists(
  string $name,
  bool $add_prefix = TRUE,
): bool;

public function findViews(string $view_expression): array;

SchemaDefinition\Schema receives a new optional views constructor argument and provides:

  • viewNames()
  • getViewDefinition()

Schema definitions containing views cannot be converted to the legacy array representation because that format has no representation for SQL views.

Data model changes

Modules can create SQL views alongside their tables. No existing table schemas or stored data are changed.

User interface changes

None.

Remaining tasks

  • Review the new API and database-driver implementations.

Release notes snippet

TBD.

CommentFileSizeAuthor
#21 3618770-nr-bot_93b8mc23.txt91 bytesneeds-review-queue-bot
#8 3618770-nr-bot_gjq4ettj.txt98 bytesneeds-review-queue-bot

Issue fork drupal-3618770

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

amateescu created an issue. See original summary.

amateescu opened merge request !16823

amateescu’s picture

amateescu commented
Issue summary: View changes
Status: Active » Needs review

Here we go :) A LLM was used to assist with this work.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Needs work
Related issues: +#3567335: (experiment) Introduce a registry to store the abstract definitions of the tables created, +#3557481: Convert hook_schema() implementations to SchemaDefinition - regular modules

Nice to see the new Schema Definition API getting traction!

#3557481: Convert hook_schema() implementations to SchemaDefinition - regular modules would probably have to be done first.
#3567335: (experiment) Introduce a registry to store the abstract definitions of the tables created might help in the process of re-creating views without having to look into each db definition tables.

Added some comments inline, NW for some of them

amateescu’s picture

amateescu commented
Status: Needs work » Needs review

Thanks for the review, @mondrake! I've addressed most points, and kept only 3 open with my opinion on them.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +Needs subsystem maintainer review, +Needs framework manager review

This is large enough to require subsystem and FM reviews.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Category: Task » Feature request
Related issues: +#3620121: Introduce SchemaDefinition-based alternatives to Schema::addField and ::changeField

What will happen on a created view when we drop/rename a table or a column via schema operations? Do we need to conceive a process so that the view is dropped and rebuilt? I’ve been doing something as such in #3620121: Introduce SchemaDefinition-based alternatives to Schema::addField and ::changeField.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3618770-nr-bot_gjq4ettj.txt98 bytes

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. 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.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +Database schema definition
amateescu’s picture

amateescu commented
Status: Needs work » Needs review

@mondrake, that's a very good question! The MR was not handling that, and now it is :)

  • A view that still works after the change is kept, with the same column names: renaming a table, changing a column type, adding a column.
  • A view that reads a dropped or renamed column is dropped, together with any view reading it. Dropping a table drops the views as well.

Renamed columns are treated like dropped ones on purpose: MySQL and SQLite do not rename the column inside view definitions (PgSQL does), and rewriting the stored SQL to follow it turned into a partial SQL parser with no reliable semantics, so I stopped chasing that goal.

mondrake’s picture

mondrake
Location 🇮🇹
commented

This looks great. I am biased by SchemaDefinition work so I would prefer to see more use of it than new API methods on Schema, but yeah, we're not there yet. Still it looks like the methods introduced here would be exploited by to-be methods doing SchemaDefinition based work (see #3620121: Introduce SchemaDefinition-based alternatives to Schema::addField and ::changeField).

Added a couple of comments/questions inline.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Needs work
amateescu’s picture

amateescu commented
Status: Needs work » Needs review

Replied to all review items.

mondrake’s picture

mondrake
Location 🇮🇹
commented

reopened a thread (sorry)

amateescu’s picture

amateescu commented

The pipeline failure was coming from #3621016: text_update_12001 fails to load text_with_summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented

@amateescu sorry for coming back in bits and pieces. This is a biggie and I found myself mumbling again and again about it and then ideas pop up...

I think we need to consider a bit about the responsibility of the new methods about dependency management (dependency of database objects here). Let me explain. In the current Schema API, changing or adding a column is just doing that bit, not dealing with dropping/recreating the indexes/keys that are affected by the change. To that extent, this is even documented for example in ::changeField():

   * IMPORTANT NOTE: To maintain database portability, you have to explicitly
   * recreate all indices and primary keys that are using the changed field.
   *
   * That means that you have to drop all affected keys and indexes with
   * Schema::dropPrimaryKey(), Schema::dropUniqueKey(), or Schema::dropIndex()
   * before calling ::changeField().
   * To recreate the keys and indices, pass the key definitions as the
   * optional $keys_new argument directly to ::changeField().

Now, if we drop/recreate views as part of the same methods, we will be mixing up responsibilities between the primitive methods of the Schema API, and of calling code (typically update functions): some things need to be managed in the calling code, other by the primitive itself.

IMHO, we should take away the dependency management from the primitive methods, and leave everything to the calling code. Actually, in #3620121: Introduce SchemaDefinition-based alternatives to Schema::addField and ::changeField I am trying to introduce new methods that would encapsulate dependency management and the call to the primitive:

find dependencies (find...) -> remove dependencies (drop...) -> apply the primitive (change/add...) -> restore dependencies according to the relevant schema definition (create...)

in other words, introduce a set of methods 'one level up' from the primitives, that use the primitives based on the input from the SchemaDefinition.

TBH I think if we could do #3620121: Introduce SchemaDefinition-based alternatives to Schema::addField and ::changeField first, this will all make probably more sense, as dropping/recreating views would fit nicely to come along the dropping/recreating of indexes keys in the new methods.

amateescu’s picture

amateescu commented

No worries at all, I also do that on many issues :)

I looked into the git history of the changeField() documentation, and it's super old and no longer accurate. It dates from 2007 (Drupal 6 days), when the PostgreSQL driver changed a column by adding a new one and dropping the old one. The current drivers are much more "resilient" to those types of schema changes, and we should probably investigate that more in depth in a followup.

I agree with your point though, so I removed the dependent-view handling from the primitives. The MR no longer drops, rewrites or recreates views in any schema operation, and the dependency lookup code is gone. The rule is documented in database.api.php: the module that declares a view needs to drop it manually before changing a table or column the view depends on, then recreate it afterwards.

If a module is adding views for another module's (or core) schema, it will have to use hook_update_dependencies() to position their update functions before and after the one that changes the schema. A bit more work for them, but it does make this MR a lot simpler, and we can improve things later.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Thanks. I have no more input, LGTM.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3622611: Introduce Select and Condition classes that abstract from the Connection

Filed #3622611: Introduce Select and Condition classes that abstract from the Connection for following up on https://git.drupalcode.org/project/drupal/-/merge_requests/16823#note_23...

amateescu’s picture

amateescu commented
Issue tags: +Contributed project blocker
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3618770-nr-bot_93b8mc23.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. 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.

amateescu’s picture

amateescu commented
Status: Needs work » Reviewed & tested by the community

Rebased.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Needs review
Related issues: +#3566893: Convert DbDumpCommand to use SchemaDefinition objects

Just restarted looking at #3566893: Convert DbDumpCommand to use SchemaDefinition objects and realized: do we need to include the views in the db-dumps?