Skip to content

PostgreSQL: AlterTypeOperation owner/schema/attribute ops and Statement::AlterDefaultPrivileges#2322

Closed
fmguerreiro wants to merge 70 commits intoapache:mainfrom
fmguerreiro:feat/alter-type-default-privileges
Closed

PostgreSQL: AlterTypeOperation owner/schema/attribute ops and Statement::AlterDefaultPrivileges#2322
fmguerreiro wants to merge 70 commits intoapache:mainfrom
fmguerreiro:feat/alter-type-default-privileges

Conversation

@fmguerreiro
Copy link
Copy Markdown
Contributor

What wasn't working

Postgres rejects these statements at parse time because the AST has no shape for them — downstream consumers (e.g. pgmold) must strip them before calling Parser::parse_sql:

  • ALTER TYPE foo OWNER TO some_role
  • ALTER TYPE foo SET SCHEMA other_schema
  • ALTER TYPE foo { ADD | DROP | ALTER | RENAME } ATTRIBUTE ...
  • ALTER DEFAULT PRIVILEGES [FOR ROLE r] [IN SCHEMA s] { GRANT | REVOKE } ... ON { TABLES | SEQUENCES | FUNCTIONS | ROUTINES | TYPES | SCHEMAS } { TO | FROM } ...

AlterTypeOperation only had Rename | AddValue | RenameValue, and Statement::AlterDefaultPrivileges did not exist.

What changed

  • AlterTypeOperation gains OwnerTo, SetSchema, AddAttribute, DropAttribute, AlterAttribute, RenameAttribute. The Postgres grammar (IF [NOT] EXISTS, COLLATE, CASCADE | RESTRICT) is honoured. OwnerTo reuses Owner (same type as AlterTableOperation::OwnerTo / AlterCollationOperation::OwnerTo).
  • New Statement::AlterDefaultPrivileges(AlterDefaultPrivileges) carries for_roles: Vec<Ident>, in_schemas: Vec<Ident>, and an AlterDefaultPrivilegesAction { Grant { ... } | Revoke { ... } } body that reuses the existing Privileges and Grantee types. The ON <kind> target uses a new AlterDefaultPrivilegesObjectType enum.
  • New keywords: ATTRIBUTE, ROUTINES, TYPES. parse_alter accepts DEFAULT as a leading object token.

Examples

ALTER TYPE public.my_type OWNER TO some_role
ALTER TYPE public.my_type SET SCHEMA other_schema
ALTER TYPE foo ADD ATTRIBUTE IF NOT EXISTS new_attr TEXT CASCADE
ALTER TYPE foo ALTER ATTRIBUTE attr SET DATA TYPE INTEGER
ALTER TYPE foo RENAME ATTRIBUTE old_attr TO new_attr
ALTER DEFAULT PRIVILEGES FOR ROLE app_role IN SCHEMA public, audit
    GRANT SELECT, INSERT ON TABLES TO reader, writer WITH GRANT OPTION
ALTER DEFAULT PRIVILEGES IN SCHEMA public
    REVOKE GRANT OPTION FOR ALL PRIVILEGES ON FUNCTIONS FROM reader CASCADE

Breaking change disclosure

Both AlterTypeOperation and Statement are not #[non_exhaustive]. Adding variants is technically a breaking change for downstream consumers that match exhaustively without a _ arm. Recommend cutting the next release as 0.61.0.

Verified

cargo fmt --all
cargo clippy --all-targets --all-features -- -D warnings
cargo test --all-features
cargo check --no-default-features --target thumbv6m-none-eabi
(cd sqlparser_bench && cargo clippy --all-targets --all-features -- -D warnings)

…R TABLE operations (#1)

PostgreSQL supports FORCE ROW LEVEL SECURITY and NO FORCE ROW LEVEL SECURITY
as ALTER TABLE operations. Add parsing support for both variants.
feat: parse EXCLUDE constraints for PostgreSQL
…m-main

# Conflicts:
#	Cargo.toml
#	src/ast/ddl.rs
#	src/ast/helpers/stmt_create_table.rs
#	src/ast/mod.rs
#	src/ast/spans.rs
#	src/ast/table_constraints.rs
#	src/parser/mod.rs
#	tests/sqlparser_postgres.rs
chore: merge upstream apache/datafusion-sqlparser-rs main
- Add HANDLER and VALIDATOR keywords to the keyword list
- Add FdwRoutineClause enum for HANDLER/NO HANDLER and VALIDATOR/NO VALIDATOR clauses
- Add CreateForeignDataWrapper struct for CREATE FOREIGN DATA WRAPPER
- Add CreateForeignTable struct for CREATE FOREIGN TABLE
- Export new types from ast::mod and add Statement variants
- Add spans.rs coverage returning Span::empty() for the new variants
- Dispatch on FOREIGN keyword in parse_create, branching on DATA WRAPPER or TABLE
- parse_create_foreign_data_wrapper: parses optional HANDLER/NO HANDLER,
  VALIDATOR/NO VALIDATOR, and OPTIONS clauses
- parse_create_foreign_table: parses IF NOT EXISTS, column list via parse_columns,
  required SERVER name, and optional OPTIONS clause
Round-trip tests via pg().verified_stmt for:
- FDW: name-only, HANDLER, NO HANDLER, NO VALIDATOR, combined HANDLER+VALIDATOR+OPTIONS
- FOREIGN TABLE: basic columns+SERVER, IF NOT EXISTS, table-level OPTIONS
Add Statement::CreateAggregate, CreateAggregate struct, CreateAggregateOption
enum, and AggregateModifyKind enum to represent PostgreSQL CREATE AGGREGATE
DDL. Options are stored as a typed enum covering all documented parameters
(SFUNC, STYPE, FINALFUNC, PARALLEL, moving-aggregate variants, etc.).
Wire AGGREGATE into the CREATE dispatch (before the or_replace error branch
so CREATE OR REPLACE AGGREGATE is accepted). parse_create_aggregate parses
the name, argument-type list, and the options block. Each recognised option
keyword dispatches to parse_create_aggregate_option which produces the typed
CreateAggregateOption variant.
Three tests covering: basic old-style aggregate (SFUNC/STYPE/FINALFUNC/INITCOND),
CREATE OR REPLACE with PARALLEL = SAFE, and moving-aggregate options
(MSFUNC/MINVFUNC/MSTYPE/MFINALFUNC_EXTRA/MFINALFUNC_MODIFY). All use
pg().verified_stmt() to assert parse-then-display round-trips identically.
feat: parse CREATE TEXT SEARCH CONFIGURATION/DICTIONARY/PARSER/TEMPLATE
# Conflicts:
#	src/ast/ddl.rs
#	src/ast/mod.rs
# Conflicts:
#	src/ast/ddl.rs
#	src/ast/mod.rs
#	src/parser/mod.rs
# Conflicts:
#	src/ast/ddl.rs
#	src/ast/mod.rs
#	tests/sqlparser_postgres.rs
… nodes

Add SecurityLabelObjectKind enum (TABLE/COLUMN/DATABASE/DOMAIN/FUNCTION/
ROLE/SCHEMA/SEQUENCE/TYPE/VIEW/MATERIALIZED VIEW) and SecurityLabel struct,
CreateUserMapping struct with UserMappingUser enum, and CreateTablespace struct
to ddl.rs with standard derives, Display impls, and From<X> for Statement.
Add Statement variants, Display arms, re-exports in mod.rs.
Add Span::empty() arms in spans.rs.
Add LABEL and MAPPING to keywords.rs.

Refs: pgmold-115, pgmold-119, pgmold-120
…SPACE

Add parse_security_label to parse_statement dispatch on Keyword::SECURITY.
Add parse_create_user_mapping dispatched from CREATE USER MAPPING branch
(checked before existing CREATE USER fallthrough).
Add parse_create_tablespace dispatched from parse_create on Keyword::TABLESPACE.

Refs: pgmold-115, pgmold-119, pgmold-120
…REATE TABLESPACE

Add 4 tests for SECURITY LABEL covering TABLE, NULL label, ROLE, and SCHEMA targets.
Add 4 tests for CREATE USER MAPPING covering basic, IF NOT EXISTS with OPTIONS,
CURRENT_USER, and PUBLIC user specifiers.
Add 3 tests for CREATE TABLESPACE covering basic, OWNER, and WITH options.

Refs: pgmold-115, pgmold-119, pgmold-120
…er, CreateTransform AST nodes

Add structs and enums for four PostgreSQL-specific DDL statements:
- CreateStatistics with StatisticsKind (NDistinct, Dependencies, Mcv)
- CreateAccessMethod with AccessMethodType (Index, Table)
- CreateEventTrigger with EventTriggerEvent (DdlCommandStart, DdlCommandEnd, TableRewrite, SqlDrop)
- CreateTransform / TransformElement with OR REPLACE support

Adds TRANSFORM keyword to keywords.rs.

Closes pgmold-103, pgmold-104, pgmold-105, pgmold-106.
…TRANSFORM

Add parse_create_statistics, parse_create_access_method,
parse_create_event_trigger, and parse_create_transform to the parser.

- STATISTICS: name, optional IF NOT EXISTS, optional (kind, ...) list, ON expr-list, FROM table
- ACCESS METHOD: name TYPE INDEX|TABLE HANDLER function
- EVENT TRIGGER: name ON event [WHEN TAG IN (...)] EXECUTE FUNCTION|PROCEDURE name()
- TRANSFORM: [OR REPLACE] FOR type LANGUAGE lang (element, ...)

TRANSFORM is dispatched before the or_replace error guard.
…le AST nodes

Add AST structs and Display impls for four PostgreSQL catalog DDL
statements. Each follows the established fork pattern: struct in ddl.rs
with Debug/Clone/PartialEq/etc derives, Statement enum variant, Display
arm, From impl, and Span::empty() arm in spans.rs.

New keywords: ALSO, ASSIGNMENT, CONVERSION, IMPLICIT, INLINE,
PROCEDURAL, TRUSTED.

Closes pgmold-89, pgmold-91, pgmold-92, pgmold-93.
feat: parse SECURITY LABEL, CREATE USER MAPPING, CREATE TABLESPACE
# Conflicts:
#	src/ast/ddl.rs
#	src/ast/mod.rs
#	src/ast/spans.rs
#	src/parser/mod.rs
#	tests/sqlparser_postgres.rs
feat: parse CREATE STATISTICS, CREATE ACCESS METHOD, CREATE EVENT TRIGGER, CREATE TRANSFORM
# Conflicts:
#	src/ast/ddl.rs
#	src/ast/mod.rs
#	src/ast/spans.rs
#	src/parser/mod.rs
#	tests/sqlparser_postgres.rs
feat: parse CREATE CAST, CREATE CONVERSION, CREATE LANGUAGE, CREATE RULE
PRs #14, #15, and #16 each added new AST types that collide in the
same places in src/ast/ddl.rs, src/ast/mod.rs, src/parser/mod.rs,
and tests/sqlparser_postgres.rs. Several mid-match-arm and
mid-function-body concat resolutions during the three rebase-and-merge
passes left fn / impl / match blocks missing their closing braces
and left the ddl re-export list with duplicate entries.

This restores compile:
- Adds missing closing braces for CastFunctionKind, StatisticsKind,
  CreateRule / CreateStatistics / CreateEventTrigger / CreateTransform /
  CreateLanguage Display impls and parse fns, plus the CreateTransform
  From-to-Statement impl.
- Dedupes and alphabetizes the `pub use self::ddl::{ ... }` list in
  src/ast/mod.rs.
- Adds missing doc comments on CastFunctionKind fields and RuleEvent
  variants that #[warn(missing_docs)] requires.

Unblocks 0.60.10 publish.
fix: close delimiter gaps left by concat merges
PostgreSQL's pg_dump emits WITH NO DATA on unpopulated materialized views.
Add a with_data: Option<bool> field to CreateView and parse the clause
after the query body. Bump to 0.60.11.
This was an accidentally-committed gitlink (160000 mode) with no
corresponding entry in .gitmodules. It does not break git itself
but causes any cargo git-dep consumer to fail cloning the repo:

  failed to update submodule `.worktrees/fix-pr-2307-ci`
  no URL configured for submodule '.worktrees/fix-pr-2307-ci'

Same class of residue that 837b5a0 on feat/exclude-constraint-upstream
cleaned up, but that commit never reached main so the gitlink still
ships on every descendant branch.
feat(parser): parse WITH [NO] DATA on CREATE MATERIALIZED VIEW
Add AlterTableOperation::AttachPartitionOf and DetachPartitionOf variants
for PostgreSQL declarative partition DDL:

  ALTER TABLE parent ATTACH PARTITION child { FOR VALUES <spec> | DEFAULT }
  ALTER TABLE parent DETACH PARTITION child [ CONCURRENTLY | FINALIZE ]

These are distinct from the existing ClickHouse AttachPartition/DetachPartition
variants which use the PART|PARTITION <expr> syntax.

- Add FINALIZE keyword to the keyword list
- Add span impls for the new variants
- Add parser tests covering range, list, hash, default attach and
  plain, concurrently, finalize detach
- Bump version to 0.60.12
…h-partition

feat: parse ALTER TABLE ATTACH/DETACH PARTITION for PostgreSQL
FULLTEXT and SPATIAL are MySQL-specific table constraint keywords. When
parse_optional_table_constraint encountered them in a PostgreSQL context
it would consume the token, fall through to the _ arm, and call
prev_token() to restore it — but the consume-then-backtrack round-trip
left room for a subtle parser-state bug that caused the next call to
parse_data_type to see 'fulltext' instead of the actual column type.

Fix: add an explicit peek-based guard at the top of
parse_optional_table_constraint so that for dialects other than MySQL /
Generic the FULLTEXT / SPATIAL tokens are never consumed at all. The
caller (parse_columns) then correctly falls through to parse_column_def
and treats the word as an ordinary column identifier.

Also add a parser unit test covering a tsvector column literally named
'fulltext' (as in the pagila DVD-rental schema) and a GiST index that
references the same column by name.

Bump version to 0.60.13.
fix(parser): guard MySQL FULLTEXT/SPATIAL table constraint on dialect
…TE FUNCTION

TriggerExecBody previously reused FunctionDesc (and parse_function_desc),
which parsed arguments as CREATE-FUNCTION-style parameter declarations
(argname argtype DEFAULT expr). Any trigger that passes literal arguments
such as tsvector_update_trigger('fulltext', 'pg_catalog.english', ...) would
fail with "Expected: a data type name, found: 'fulltext'".

Replace the func_desc: FunctionDesc field with func_name: ObjectName and
args: Option<Vec<Expr>>, and replace the parse_function_desc() call in
parse_trigger_exec_body with parse_object_name() + parse_comma_separated
(Parser::parse_expr). This treats args as call-site expressions, matching
PostgreSQL semantics. The None/Some(vec![]) distinction preserves whether
parentheses were written at all.

Bump version to 0.60.14.
fix(parser): accept call-site expression args in CREATE TRIGGER EXECUTE FUNCTION
…est) (#23)

* chore(tests): restore green main; fix clippy and keyword sort regressions

`cargo test --all-features` and `cargo clippy --all-targets --all-features
-- -D warnings` were both failing on `main`; CI never ran on the fork to
catch it. This commit makes them green so subsequent PRs can be
verified.

Test compile errors:
- `Statement::AlterTable` was refactored from struct to tuple form
  carrying `AlterTable`. Update 10 ATTACH/DETACH PARTITION test sites
  in `sqlparser_postgres.rs` to match the new shape.
- `CreateView` gained a `with_data` field (added with the
  `WITH [NO] DATA` parser support). Add `with_data: _,` to 6 destructure
  sites in `sqlparser_common.rs`.

Test runtime failures:
- `all_keywords_sorted`: `CONFIGURATION`, `INLINE`, and `TRANSFORM` were
  inserted out of ASCII order in `keywords.rs`; move each to its
  alphabetical slot.
- `alter_procedure_rename` / `alter_procedure_set_search_path` /
  `parse_fulltext_column_and_index_in_postgres`: `verified_stmt` round-trip
  fails because Display canonicalises type names and access methods to
  uppercase. Update the input SQL to match.

Lib clippy regressions (forbidden via `#![forbid(clippy::unreachable)]`
and `-D warnings`):
- `parse_create_aggregate_options` PARALLEL arm: replace `_ => unreachable!()`
  with an explicit `Internal parser error` so `clippy::unreachable` is
  satisfied.
- `parse_create_aggregate_args`: drop the let-and-return.
- `parse_create_rule` INSTEAD/ALSO: collapse the always-false else-if
  branches into a single optional-keyword consume.

Doctest:
- `Statement::CreateStatistics` doc block was missing its opening
  ```sql fence.

Spans:
- Drop the duplicate `ForceRowLevelSecurity` / `NoForceRowLevelSecurity`
  arms in `AlterTableOperation::span()` (warnings, but `-D warnings`
  blocks clippy).

* chore(ci): fix lint, no-std, RAT, and benchmark-lint regressions

CI on `main` was failing five jobs (test/lint/compile-no-std/RAT/
benchmark-lint). The first commit on this branch fixed test compile +
runtime + a few clippy and doctest issues found locally. CI surfaced
the rest. This commit covers them.

`lint` (clippy 1.95 — 8 collapsible_match errors)
- Fold inner `if` blocks into match guards in `parse_sql` (END token
  delimiter), `parse_compound_field` (period-qualified identifier), the
  DO/loop sequence terminator, and the five `HiveRowFormat::DELIMITED`
  arms (FIELDS / COLLECTION / MAP / LINES / NULL DEFINED AS).

`compile-no-std` (`thumbv6m-none-eabi`)
- `src/ast/table_constraints.rs` introduced an EXCLUDE-constraint type
  with `pub operator: String` but did not import `String` in the
  `#[cfg(not(feature = "std"))]` alloc block. Add `string::String`.

`Release Audit Tool (RAT)`
- Add `CLAUDE.md` to `dev/release/rat_exclude_files.txt` next to the
  existing `AGENTS.md` exclusion. Both are agent-guideline files that
  do not need an Apache header.

`benchmark-lint`
- `sqlparser_bench/Cargo.toml` referenced the path-dep as `sqlparser`
  but the fork's package was renamed to `pgmold-sqlparser`. Add the
  `package = "pgmold-sqlparser"` rename so the bench resolves.

Verified locally on rustc 1.95.0:
  cargo fmt --all -- --check
  cargo clippy --all-targets --all-features -- -D warnings
  cargo test --all-features
  cargo check --no-default-features --target thumbv6m-none-eabi
  (cd sqlparser_bench && cargo clippy --all-targets --all-features -- -D warnings)
…AIN (#24)

`GRANT USAGE ON TYPE foo TO role` and `GRANT ON DOMAIN d TO role` are
valid PostgreSQL grammar but failed to parse — `parse_grant_target`
had no `TYPE` or `DOMAIN` keyword, and `GrantObjects` lacked variants
to carry them.

Add `GrantObjects::Types(Vec<ObjectName>)` and
`GrantObjects::Domains(Vec<ObjectName>)`, along with their `Display`
arms, the `Keyword::TYPE` / `Keyword::DOMAIN` arms in
`parse_grant_deny_revoke_privileges_objects`, and round-trip tests
in `parse_grant`.

Verified:
  cargo fmt --all -- --check
  cargo clippy --all-targets --all-features -- -D warnings
  cargo test --all-features parse_grant
…nt::AlterDefaultPrivileges

Postgres rejects `ALTER TYPE ... OWNER TO ...`, `ALTER TYPE ... SET SCHEMA ...`,
`ALTER TYPE ... { ADD | DROP | ALTER | RENAME } ATTRIBUTE ...`, and every
`ALTER DEFAULT PRIVILEGES ...` form because the AST has no shape for them. This
forces downstreams (e.g. pgmold) to strip those statements before parsing.

`AlterTypeOperation` gains six variants (`OwnerTo`, `SetSchema`,
`AddAttribute`, `DropAttribute`, `AlterAttribute`, `RenameAttribute`). The
parser branches accept the matching grammar including optional `IF [NOT]
EXISTS`, `COLLATE`, and `CASCADE | RESTRICT`.

A new `Statement::AlterDefaultPrivileges` carries `for_roles`, `in_schemas`,
and an `AlterDefaultPrivilegesAction { Grant | Revoke }` body that reuses
`Privileges` and `Grantee`. The `ON <kind>` target uses a new
`AlterDefaultPrivilegesObjectType` enum (`TABLES | SEQUENCES | FUNCTIONS |
ROUTINES | TYPES | SCHEMAS`). New keywords: `ATTRIBUTE`, `ROUTINES`, `TYPES`.
The `parse_alter` dispatch now accepts `DEFAULT` as the leading word.

Note: both `AlterTypeOperation` and `Statement` are not `non_exhaustive`, so
adding variants is technically a breaking change for downstream consumers
that match exhaustively without `_`. Recommend bumping the next release to
0.61.0.

Verified:
  cargo fmt --all
  cargo clippy --all-targets --all-features -- -D warnings
  cargo test --all-features
  cargo check --no-default-features --target thumbv6m-none-eabi
  (cd sqlparser_bench && cargo clippy --all-targets --all-features -- -D warnings)
@fmguerreiro
Copy link
Copy Markdown
Contributor Author

Wrong target: this should target the fmguerreiro/datafusion-sqlparser-rs fork. Reopening there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant