Support AS MATERIALIZED in PG12 [RUN AS-IF-FOSS]
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
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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)