Skip to content

PostgreSQL: Support CREATE FOREIGN DATA WRAPPER - #2317

Open
fmguerreiro wants to merge 16 commits into
apache:mainfrom
fmguerreiro:upstream/foreign-table-fdw
Open

fmguerreiro wants to merge 16 commits into
apache:mainfrom
fmguerreiro:upstream/foreign-table-fdw

Conversation

@fmguerreiro

@fmguerreiro fmguerreiro commented Apr 18, 2026 •

Copy link
Copy Markdown
Contributor

Parses CREATE FOREIGN DATA WRAPPER name [HANDLER fn | NO HANDLER] [VALIDATOR fn | NO VALIDATOR] [OPTIONS (...)] (docs).

Split out of this PR at @LucaCappelletti94's request. CREATE FOREIGN TABLE is now #2515.

HANDLER and VALIDATOR are accepted in either order, because the PostgreSQL grammar takes them as an unordered list even though the doc synopsis shows one order.

ALTER FOREIGN DATA WRAPPER and CREATE USER MAPPING are out of scope.

Ported from the pgmold-sqlparser fork.

@LucaCappelletti94

Copy link
Copy Markdown
Contributor

I suggest to split the PR into two, once for each statement

@fmguerreiro
fmguerreiro force-pushed the upstream/foreign-table-fdw branch from 538c418 to 54177bd Compare September 16, 2026 01:35
@fmguerreiro fmguerreiro changed the title PostgreSQL: add support for CREATE FOREIGN TABLE and CREATE FOREIGN DATA WRAPPER PostgreSQL: add support for CREATE FOREIGN DATA WRAPPER Sep 16, 2026
@fmguerreiro

Copy link
Copy Markdown
Contributor Author

split. CREATE FOREIGN TABLE is now #2515.

@fmguerreiro fmguerreiro changed the title PostgreSQL: add support for CREATE FOREIGN DATA WRAPPER PostgreSQL: Support CREATE FOREIGN DATA WRAPPER Sep 16, 2026
@codecov-commenter

codecov-commenter commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.27778% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.32%. Comparing base (3330dc9) to head (b6a5877).

Files with missing lines Patch % Lines
src/parser/mod.rs 89.18% 0 Missing and 4 partials ⚠️
src/ast/ddl.rs 90.90% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2317      +/-   ##
==========================================
+ Coverage   81.31%   81.32%   +0.01%     
==========================================
  Files          42       42              
  Lines       33977    34040      +63     
  Branches    33977    34040      +63     
==========================================
+ Hits        27628    27684      +56     
  Misses       2785     2785              
- Partials     3564     3571       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/ast/spans.rs
Comment on lines +3168 to +3181
#[test]
fn test_create_foreign_data_wrapper_span_includes_option_keys() {
let dialect = &crate::dialect::PostgreSqlDialect {};
let sql = "CREATE FOREIGN DATA WRAPPER myfdw HANDLER myhandler OPTIONS (debug 'true')";
let mut test = SpanTest::new(dialect, sql);

// Ends at the option key, not the statement: a quoted option value is an
// Ident with an empty span, so it contributes nothing to the union.
let stmt = test.0.parse_statement().unwrap();
assert_eq!(
test.get_source(stmt.span()),
"myfdw HANDLER myhandler OPTIONS (debug"
);
}

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.

You should drop this test. It asserts the truncated span ... OPTIONS (debug, which comes from parse_identifier returning Span::empty() for single-quoted strings (src/parser/mod.rs:13877). The test turns that defect into a contract, so a later fix of the value span would break it.

Suggested change
#[test]
fn test_create_foreign_data_wrapper_span_includes_option_keys() {
let dialect = &crate::dialect::PostgreSqlDialect {};
let sql = "CREATE FOREIGN DATA WRAPPER myfdw HANDLER myhandler OPTIONS (debug 'true')";
let mut test = SpanTest::new(dialect, sql);
// Ends at the option key, not the statement: a quoted option value is an
// Ident with an empty span, so it contributes nothing to the union.
let stmt = test.0.parse_statement().unwrap();
assert_eq!(
test.get_source(stmt.span()),
"myfdw HANDLER myhandler OPTIONS (debug"
);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

dropped in 59608c6.

Comment thread tests/sqlparser_postgres.rs Outdated
Comment on lines +9739 to +9740
// Pinning the message keeps this from passing on the old positional parser,
// which also errored here but on the trailing tokens instead.

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.

Differential comment with previous code.

Suggested change
// Pinning the message keeps this from passing on the old positional parser,
// which also errored here but on the trailing tokens instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed in 59608c6.

Comment thread tests/sqlparser_postgres.rs Outdated
}])
);

// A single hardcoded Display label would render this as `NO HANDLER NO HANDLER`.

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.

Do not include comments describing the PR change, comments are for describing the code.

Suggested change
// A single hardcoded Display label would render this as `NO HANDLER NO HANDLER`.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed in 59608c6.

@LucaCappelletti94 LucaCappelletti94 added waiting on contributor The review needs further refinements by its author waiting on reviewer PR needs to be reviewed needs rebasing The PR has collisions with main and requires rebasing and removed waiting on contributor The review needs further refinements by its author waiting on reviewer PR needs to be reviewed labels Sep 24, 2026
@LucaCappelletti94
LucaCappelletti94 force-pushed the upstream/foreign-table-fdw branch from 59608c6 to 4d03db2 Compare September 26, 2026 12:18
@LucaCappelletti94 LucaCappelletti94 added waiting on reviewer PR needs to be reviewed and removed needs rebasing The PR has collisions with main and requires rebasing labels Sep 26, 2026

@LucaCappelletti94 LucaCappelletti94 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.

Do please also add some seed entries for the fuzzer corpus, so it can better search bugs that are relevant to this syntax.

Comment thread src/parser/mod.rs
Comment thread src/parser/mod.rs
Comment thread src/parser/mod.rs Outdated
Comment thread tests/sqlparser_postgres.rs Outdated
@LucaCappelletti94 LucaCappelletti94 added waiting on contributor The review needs further refinements by its author and removed waiting on reviewer PR needs to be reviewed labels Sep 26, 2026
Share one has_modifier check between the FOREIGN TABLE and FOREIGN
DATA WRAPPER arms so temporary/or-alter/global no longer parse and
render as a bare CREATE FOREIGN DATA WRAPPER. Add fuzz seeds.
@LucaCappelletti94 LucaCappelletti94 added waiting on reviewer PR needs to be reviewed and removed waiting on contributor The review needs further refinements by its author labels Sep 27, 2026

@LucaCappelletti94 LucaCappelletti94 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.

Most likely "A repeated clause errors." should be dropped from the description once the parser keeps repeats, given the pg_query reference.

Comment thread src/parser/mod.rs Outdated
Comment thread src/ast/ddl.rs Outdated
Comment thread src/ast/ddl.rs Outdated
Comment thread src/ast/ddl.rs Outdated
Comment thread src/ast/ddl.rs Outdated
Comment thread tests/sqlparser_postgres.rs Outdated
Comment thread tests/sqlparser_postgres.rs Outdated
Comment thread tests/sqlparser_postgres.rs
@LucaCappelletti94 LucaCappelletti94 added waiting on contributor The review needs further refinements by its author needs rebasing The PR has collisions with main and requires rebasing enhancement New feature or request and removed waiting on reviewer PR needs to be reviewed labels Oct 17, 2026
fmguerreiro and others added 13 commits October 19, 2026 15:58
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
Co-authored-by: Luca Cappelletti <cappelletti.luca94@gmail.com>
# Conflicts:
#	tests/sqlparser_postgres.rs
# Conflicts:
#	tests/sqlparser_postgres.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs rebasing The PR has collisions with main and requires rebasing PostgreSQL waiting on contributor The review needs further refinements by its author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants