Skip to content

feat(postgresql): add support for table creation DDL that contains a primary key alongside the INCLUDE keyword #5425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 21, 2025
Merged

feat(postgresql): add support for table creation DDL that contains a primary key alongside the INCLUDE keyword #5425

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9198db1
feat(postgresql): add support for table creation DDL that contains a …
amosbiras Jul 17, 2025
65f6622
feat(postgresql): modify the code according to the PR review
amosbiras Jul 18, 2025
555f4e0
feat(postgresql): annotate expression in primarykey_sql to be exp.Pri…
amosbiras Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sqlglot/expressions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2218,7 +2218,7 @@ class ColumnPrefix(Expression):


class PrimaryKey(Expression):
arg_types = {"expressions": True, "options": False}
arg_types = {"expressions": True, "options": False, "include": False}


# https://www.postgresql.org/docs/9.1/sql-selectinto.html
Expand Down
4 changes: 3 additions & 1 deletion sqlglot/generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2997,9 +2997,11 @@ def foreignkey_sql(self, expression: exp.ForeignKey) -> str:

def primarykey_sql(self, expression: exp.ForeignKey) -> str:
expressions = self.expressions(expression, flat=True)
include = self.expressions(expression, key="include", flat=True)
include = f" INCLUDE ({include})" if include else ""
Comment on lines +3000 to +3001
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto, if we have an exp.IndexParameters node here, all we'll have to do is:

include = self.sql(expression, "include")
include = f" {include}" if include else ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately here when I run this suggested code as is, I get the following error:
ValueError: Expected an Expression. Received <class 'list'>: [Identifier(this=a, quoted=False)]

Copy link
Collaborator

Choose a reason for hiding this comment

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

Your version is the correct one– you need to use self.expressions if the child is a list.

options = self.expressions(expression, key="options", flat=True, sep=" ")
options = f" {options}" if options else ""
return f"PRIMARY KEY ({expressions}){options}"
return f"PRIMARY KEY ({expressions}){include}{options}"

def if_sql(self, expression: exp.If) -> str:
return self.case_sql(exp.Case(ifs=[expression], default=expression.args.get("false")))
Expand Down
9 changes: 8 additions & 1 deletion sqlglot/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6296,8 +6296,15 @@ def _parse_primary_key(
expressions = self._parse_wrapped_csv(
self._parse_primary_key_part, optional=wrapped_optional
)

include = None
if self._match_text_seq("INCLUDE"):
include = self._parse_wrapped_id_vars()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should reuse parser::_parse_index_params() here, it contains the INCLUDE parsing & generation so you'll get them for free


options = self._parse_key_constraint_options()
return self.expression(exp.PrimaryKey, expressions=expressions, options=options)
return self.expression(
exp.PrimaryKey, expressions=expressions, include=include, options=options
)

def _parse_bracket_key_value(self, is_map: bool = False) -> t.Optional[exp.Expression]:
return self._parse_slice(self._parse_alias(self._parse_assignment(), explicit=True))
Expand Down
1 change: 1 addition & 0 deletions tests/dialects/test_postgres.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,7 @@ def test_ddl(self):
self.validate_identity(
"CREATE TABLE t (vid INT NOT NULL, CONSTRAINT ht_vid_nid_fid_idx EXCLUDE (INT4RANGE(vid, nid) WITH &&, INT4RANGE(fid, fid, '[]') WITH &&))"
)
self.validate_identity("CREATE TABLE t (i INT, a TEXT PRIMARY KEY (i) INCLUDE (a))")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried running this in PSQL and I'm getting a parse error there as well, this needs a comma between the column def list and the primary key afaict:

... a TEXT, PRIMARY KEY (i) ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, I must've missed it, thanks for thoroughly checking it.
I at first cloned the repository, checked out to a branch of mine and committed the changes but when I tried to push it, I received a 403 - so I went to see other PRs and I noticed that some of them came from forked repos, so I literally did it and copied my changes.
Maybe it's worth to add a description in the README for developers in the library for all the required steps, tho developing here was pretty much straightforward.

self.validate_identity(
"CREATE TABLE t (i INT, PRIMARY KEY (i), EXCLUDE USING gist(col varchar_pattern_ops DESC NULLS LAST WITH &&) WITH (sp1=1, sp2=2))"
)
Expand Down