Skip to content
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.

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

Ignore primary key, foreign key and unique constraints for DDL generation #541

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 39 additions & 0 deletions tests/unit/sqlalchemy/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# limitations under the License.
import pytest
from sqlalchemy import Column
from sqlalchemy import ForeignKey
from sqlalchemy import func
from sqlalchemy import insert
from sqlalchemy import Integer
Expand Down Expand Up @@ -40,6 +41,26 @@
trino_catalog='other'
)

table_with_pk = Table(
'table_with_pk',
metadata,
Column('id', String, primary_key=True)
)

table_with_fk = Table(
'table_with_fk',
metadata,
Column('id', String, primary_key=True),
Column('fk', String, ForeignKey('table_with_pk.id'))
)

table_with_unique = Table(
'table_with_constraint',
metadata,
Column('id', String, primary_key=True),
Column('uniq', String, unique=True)
)


@pytest.fixture
def dialect():
Expand Down Expand Up @@ -170,3 +191,21 @@ def test_try_cast(dialect):
statement = select(try_cast(table_without_catalog.c.id, String))
query = statement.compile(dialect=dialect)
assert str(query) == 'SELECT try_cast("table".id as VARCHAR) AS id \nFROM "table"'


def test_catalogs_create_table_with_pk(dialect):
statement = CreateTable(table_with_pk)
query = statement.compile(dialect=dialect)
assert 'primary key' not in str(query).lower()


def test_catalogs_create_table_with_fk(dialect):
statement = CreateTable(table_with_fk)
query = statement.compile(dialect=dialect)
assert 'foreign key' not in str(query).lower()


def test_catalogs_create_table_with_unique(dialect):
statement = CreateTable(table_with_unique)
query = statement.compile(dialect=dialect)
assert 'unique' not in str(query).lower()
9 changes: 8 additions & 1 deletion trino/sqlalchemy/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,14 @@ def visit_try_cast(self, element, **kw):


class TrinoDDLCompiler(compiler.DDLCompiler):
pass
def visit_foreign_key_constraint(self, constraint, **kw):
return None
Comment on lines +184 to +185
Copy link
Member

@damian3031 damian3031 Apr 11, 2025

Choose a reason for hiding this comment

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

Rather than silently ignoring constraints, maybe we should raise an explicit error indicating that these constraints are not supported in Trino:

Suggested change
def visit_foreign_key_constraint(self, constraint, **kw):
return None
from sqlalchemy.exc import CompileError
...
def visit_foreign_key_constraint(self, constraint, **kw):
raise CompileError("Trino does not support FOREIGN KEY constraints.")

Or at least we could add a warning to make it clearer that the constraint is being ignored:

Suggested change
def visit_foreign_key_constraint(self, constraint, **kw):
return None
import warnings
from sqlalchemy.exc import SAWarning
...
def visit_foreign_key_constraint(self, constraint, **kw):
warnings.warn("Trino does not support FOREIGN KEY constraints. Constraint will be ignored.", SAWarning)
return None

@willmostly @hashhar WDYT?


def visit_primary_key_constraint(self, constraint, **kw):
return None

def visit_unique_constraint(self, constraint, **kw):
return None


class TrinoTypeCompiler(compiler.GenericTypeCompiler):
Expand Down