Skip to content

feat: add support for PG18#3375

Merged
mdashti merged 48 commits into
mainfrom
phil/pg18
Nov 29, 2025
Merged

feat: add support for PG18#3375
mdashti merged 48 commits into
mainfrom
phil/pg18

Conversation

@philippemnoel

@philippemnoel philippemnoel commented Oct 18, 2025

Copy link
Copy Markdown
Member

Ticket(s) Closed

What

Add support for Postgres 18 to pg_search. I don't yet default to PG18 for our builds, though. That can come later. We'll also want to add PG18 to the prebuilt CI binaries. This can come in a separate PR

I also upgraded pgvector to 0.8.1, which is necessary for PG18 support.

Why

Lots of community users are waiting for this, and I had an hour to spare.

How

Mostly reading Postgres source and compare REL_17_STABLE and REL_18_STABLE.

Tests

I can compile and run the full quickstart locally, and CI should pass for the full test suite.

Comment thread pg_search/src/postgres/customscan/builders/custom_path.rs Outdated
Comment thread pg_search/src/postgres/options.rs
Comment thread pg_search/src/postgres/customscan/builders/custom_path.rs Outdated
@mithuncy mithuncy self-assigned this Oct 30, 2025
@mithuncy mithuncy force-pushed the phil/pg18 branch 2 times, most recently from 3b1c26c to 471129f Compare November 10, 2025 06:32
PostgreSQL 18 introduces self-join elimination optimization that breaks
ParadeDB's context-sensitive score() functions by eliminating duplicate
table scans, causing score() to return the same value for both instances.

This implementation adds self-join detection and score() detection to the
existing planner hook (which handles window function replacement) and
uses a RAII guard to temporarily disable enable_self_join_elimination
when both conditions are met.

Changes:
- Add BoolGucGuard RAII pattern for safe GUC management
- Add has_self_join() to detect same table appearing multiple times with recursive subquery support
- Add has_score_function() to detect score() usage in queries
- Integrate PG18 self-join protection into existing paradedb_planner_hook
- Preserve all existing window function replacement functionality

The guard automatically restores the original GUC value when it goes out
of scope, ensuring configuration changes don't leak beyond their intended
scope.
@mithuncy mithuncy marked this pull request as draft November 10, 2025 07:13
This commit fixes two issues caused by rebase conflicts:

1. Restored pgvector upgrade to v0.8.1 for system Postgres installations
   - Line 127 in test-pg_search.yml was reverted to v0.8.0 during rebase
   - pgvector v0.8.1 is required for PostgreSQL 18 compatibility

2. Added missing pgrx CI configuration for PostgreSQL 18
   - Previously only had pgrx tests for PG 17
   - PG 18 tests were only running with system Postgres
   - Now includes pgrx-managed Postgres tests for PG 18
Comment thread .github/workflows/test-pg_search.yml
@jippi jippi mentioned this pull request Nov 12, 2025
2 tasks
… hooks

PostgreSQL 18 introduced automatic rewriting of OR clauses to ScalarArrayOpExpr
(ANY) during match_clause_to_indexcol. This caused crashes in pg_search's planner
hooks which expected only scalar datums.

Changes:
- Enhanced from_datum_resilient() to detect and handle array datums
- Arrays are converted to Boolean OR queries: Boolean{should: [q1, q2, ...]}
- Added coerce_scalar_array() to safely detect array vs scalar datums
- Updated planner hooks (query_input_restrict, query_input_support) to use
  universal datum decoder
- Enhanced custom scan path with explicit scalar_array_use_or handling
- Maintains backward compatibility with PG14-17 scalar queries

This fix allows both user-written ANY() and planner-generated ANY() from
OR rewrites to work correctly on PostgreSQL 18.
@mithuncy mithuncy added the Do Not Cherry Pick PR should not be cherry-picked to other branches label Nov 14, 2025
mithuncy added a commit that referenced this pull request Nov 14, 2025
PostgreSQL 18 added self-join elimination code that calls score_funcoids()
on every query through the planner hook, even before CREATE EXTENSION runs.
The old implementation used regprocedurein which panics when functions don't
exist yet.

This fix:
- Creates try_score_funcoids() using LookupFuncName with missing_ok=true
- Returns None gracefully when functions don't exist (before CREATE EXTENSION)
- Follows the ParadeDB pattern from utils.rs:lookup_pdb_function()
- Fixes test_logical_replication test failure on PG18

PG17 and below don't have this issue because the self-join elimination
check code is wrapped in #[cfg(feature = "pg18")].

Fixes #3375
PostgreSQL 18 added self-join elimination code that calls score_funcoids()
on every query through the planner hook, even before CREATE EXTENSION runs.
The old implementation used regprocedurein which panics when functions don't
exist yet.

This fix:
- Creates try_score_funcoids() using LookupFuncName with missing_ok=true
- Returns None gracefully when functions don't exist (before CREATE EXTENSION)
- Follows the ParadeDB pattern from utils.rs:lookup_pdb_function()
- Fixes test_logical_replication test failure on PG18

PG17 and below don't have this issue because the self-join elimination
check code is wrapped in #[cfg(feature = "pg18")].

Fixes #3375
Comment thread pg_search/src/query/mod.rs Outdated
Comment thread pg_search/src/postgres/scan.rs Outdated
Comment thread pg_search/src/postgres/scan.rs Outdated
Comment thread pg_search/src/postgres/scan.rs Outdated
Comment thread pg_search/src/query/mod.rs Outdated

@mdashti mdashti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@workingjubilee Thanks for the comments.

Comment thread pg_search/src/query/mod.rs Outdated
Comment thread pg_search/src/postgres/scan.rs Outdated
Comment thread pg_search/src/postgres/scan.rs Outdated
Comment thread pg_search/src/postgres/scan.rs Outdated
Comment thread pg_search/src/query/mod.rs Outdated
@mdashti mdashti changed the base branch from main to moe/fix-expr-context-lifetime November 29, 2025 09:50
Base automatically changed from moe/fix-expr-context-lifetime to main November 29, 2025 09:55
Comment thread pg_search/src/postgres/customscan/qual_inspect.rs
Comment thread pg_search/src/query/mod.rs
@mdashti mdashti merged commit 583a10a into main Nov 29, 2025
21 checks passed
@mdashti mdashti deleted the phil/pg18 branch November 29, 2025 18:58
@jippi

jippi commented Nov 29, 2025

Copy link
Copy Markdown

Thanks for the weekend merge ❤️

@philippemnoel

Copy link
Copy Markdown
Member Author

Thanks for the weekend merge ❤️

Thank you for your patience on this issue! :)

Comment on lines +572 to +577
// Check if this is a SearchQueryInput array (ScalarArrayOpExpr case).
// Without this check, FromDatum would panic with "cache lookup failed" on non-array datums.
let array = datum.cast_mut_ptr::<pg_sys::ArrayType>();
let is_sqi_array = (*array).ndim >= 1
&& (*array).ndim <= pg_sys::MAXDIM as i32
&& (*array).elemtype == searchqueryinput_typoid();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, that makes sense.

Comment on lines +112 to +121
if is_array {
// ScalarArrayOpExpr: decode as array of SearchQueryInput
let should = unsafe {
<Vec<SearchQueryInput> as FromDatum>::from_polymorphic_datum(
key.sk_argument,
false,
searchqueryinput_typoid(),
)
.expect("SearchQueryInput array should not be NULL")
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That also works, yep.

}

impl SearchQueryInput {
pub unsafe fn from_datum(datum: pg_sys::Datum, is_null: bool) -> Option<SearchQueryInput> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has one interesting little quirk in that this function can now shadow certain calls that might seem like they would hit FromDatum::from_datum, even if that trait is in scope: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=dec11478802dbf79879c77e69bc9de64

This can be desirable, so I am not saying this to warn you off it, just noting that. It's especially probably-what-you-want as this function does exactly what you want it to when from_datum is called.

}
}

// Detoast if needed (PostgreSQL memory context handles cleanup)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also valid, as long as this isn't called a ton of times in a single transaction. But if it does turn out that's the case, you can fix this later easily.

jamessewell pushed a commit that referenced this pull request Dec 2, 2025
# Ticket(s) Closed

- Closes #2723 

## What
Add support for Postgres 18 to `pg_search`. I don't yet default to PG18
for our builds, though. That can come later. We'll also want to add PG18
to the prebuilt CI binaries. This can come in a separate PR

I also upgraded pgvector to 0.8.1, which is necessary for PG18 support.

## Why
Lots of community users are waiting for this, and I had an hour to
spare.

## How
Mostly reading Postgres source and compare REL_17_STABLE and
REL_18_STABLE.

## Tests
I can compile and run the full quickstart locally, and CI should pass
for the full test suite.

---------

Signed-off-by: Philippe Noël <21990816+philippemnoel@users.noreply.github.com>
Co-authored-by: Stu Hood <stuhood@gmail.com>
Co-authored-by: Jubilee <workingjubilee@gmail.com>
Co-authored-by: Mithun Chicklore Yogendra <mithun.cy@gmail.com>
Co-authored-by: Mohammad Dashti <mdashti@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Do Not Cherry Pick PR should not be cherry-picked to other branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for PG 18

7 participants