Skip to content

Support AS MATERIALIZED in PG12 [RUN AS-IF-FOSS]

Adam Hegyi requested to merge 245323-arel-support-for-materialized-cte into master

What does this MR do?

Conditionally change all CTE query when PG12 is used.

From:

WITH cte AS (
  SELECT X
) SELECT Y

To:

WITH cte AS MATERIALIZED (
  SELECT X
) SELECT Y

Context (from the issue)

With PostgreSQL 12, CTE behavior changes. Before, CTEs used to be an optimization fence. We've used this sometimes to avoid a poor choice of plans. In PostgreSQL 12, CTEs are not being materialized or used as an optimization fence by default anymore.

In order to keep the same characteristics on PostgreSQL 12, we'll need to mark CTEs with MATERIALIZE option.

How

This MR introduces a new Arel node which generates the AS MATERIALIZED SQL string. The generation of the MATERIALIZED string is configurable so later we can fine-tune our queries. If the DB version is PG12, the MATERIALIZED keyword will be added automatically.

Additional changes:

  • Update raw CTE queries to optionally use MATERIALIZED.

Since adding MATERIALIZED is depending on the DB connection (check DB version) some String constants had to be moved to instance methods.

We can clean this up in 14.1.

Testing with GDK

Run a PG 12 docker container in a terminal window:

docker run --rm -it --name postgres12test -p 6000:5432 -e POSTGRES_USER=john -e POSTGRES_PASSWORD=snow -e POSTGRES_DB=gitlab postgres:12-alpine

Load the schema:

PGUSER=john PGPASSWORD=snow PGHOST=localhost PGPORT=6000 rake db:structure:load

Note: I got an error here, but the schema was actually loaded successfully.

Start a rails console:

rails c

Run:

g = FactoryBot.create(:group)
g2 = FactoryBot.create(:group, parent: g)

puts g.self_and_descendants.to_sql
cte = Gitlab::SQL::CTE.new(:test, User.active)

puts User.with(cte.to_arel).to_sql

You should see AS MATERIALIZED in the queries.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #245323 (closed)

Edited by Adam Hegyi

Merge request reports