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.

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

fix(postgres): join on schema and table name in get_model TCTC-6814 #1285

Merged
merged 1 commit into from
Sep 18, 2023
Merged

fix(postgres): join on schema and table name in get_model TCTC-6814 #1285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

## Unreleased

### Fixed

- Postgres: In case two tables in different schemas have the same name, `get_model`
and `get_model_with_info` now return the correct information.

### [4.8.0] 2023-09-13

## Changed
### Changed

- S3: Add a new AWS S3 connector using the Security Token Service (STS) API Assume Role.

Expand Down
14 changes: 14 additions & 0 deletions tests/postgres/fixtures/world_postgres.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,20 @@ CREATE TABLE city (
population integer NOT NULL
);

CREATE SCHEMA other_schema;

CREATE TABLE other_schema.city (
id integer NOT NULL,
nom text NOT NULL,
code_pays character(3) NOT NULL,
districteuh text NOT NULL,
populationg integer NOT NULL
);

COPY other_schema.city (id, nom, code_pays, districteuh, populationg) FROM stdin;
1 Kabul AFG Kabol 1780000
\.

CREATE TABLE country (
code character(3) NOT NULL,
name text NOT NULL,
Expand Down
182 changes: 74 additions & 108 deletions tests/postgres/test_postgres.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def check(host_port):


@pytest.fixture
def postgres_connector(postgres_server):
def postgres_connector(postgres_server) -> PostgresConnector:
return PostgresConnector(
name='test',
host='localhost',
Expand All @@ -43,6 +43,73 @@ def postgres_connector(postgres_server):
)


@pytest.fixture
def postgres_db_model() -> list[dict]:
return [
{
'schema': 'other_schema',
'database': 'postgres_db',
'type': 'table',
'name': 'city',
'columns': [
{'name': 'id', 'type': 'integer'},
{'name': 'nom', 'type': 'text'},
{'name': 'code_pays', 'type': 'character'},
{'name': 'districteuh', 'type': 'text'},
{'name': 'populationg', 'type': 'integer'},
],
},
{
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'name': 'city',
'columns': [
{'name': 'name', 'type': 'text'},
{'name': 'countrycode', 'type': 'character'},
{'name': 'district', 'type': 'text'},
{'name': 'population', 'type': 'integer'},
{'name': 'id', 'type': 'integer'},
],
},
{
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'name': 'country',
'columns': [
{'name': 'localname', 'type': 'text'},
{'name': 'governmentform', 'type': 'text'},
{'name': 'headofstate', 'type': 'text'},
{'name': 'capital', 'type': 'integer'},
{'name': 'code2', 'type': 'character'},
{'name': 'surfacearea', 'type': 'real'},
{'name': 'code', 'type': 'character'},
{'name': 'name', 'type': 'text'},
{'name': 'continent', 'type': 'text'},
{'name': 'region', 'type': 'text'},
{'name': 'indepyear', 'type': 'smallint'},
{'name': 'population', 'type': 'integer'},
{'name': 'lifeexpectancy', 'type': 'real'},
{'name': 'gnp', 'type': 'numeric'},
{'name': 'gnpold', 'type': 'numeric'},
],
},
{
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'name': 'countrylanguage',
'columns': [
{'name': 'countrycode', 'type': 'character'},
{'name': 'language', 'type': 'text'},
{'name': 'isofficial', 'type': 'boolean'},
{'name': 'percentage', 'type': 'real'},
],
},
]


def test_get_status_all_good(postgres_connector):
assert postgres_connector.get_status() == ConnectorStatus(
status=True,
Expand Down Expand Up @@ -380,58 +447,8 @@ def execute(q):
postgres_connector.describe(ds)


def test_get_model(postgres_connector: PostgresConnector):
def test_get_model(postgres_connector: PostgresConnector, postgres_db_model: list[dict]) -> None:
"""Check that it returns the db tree structure"""
postgres_db_model = [
{
'name': 'city',
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'columns': [
{'name': 'id', 'type': 'integer'},
{'name': 'name', 'type': 'text'},
{'name': 'countrycode', 'type': 'character'},
{'name': 'district', 'type': 'text'},
{'name': 'population', 'type': 'integer'},
],
},
{
'name': 'country',
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'columns': [
{'name': 'code', 'type': 'character'},
{'name': 'name', 'type': 'text'},
{'name': 'continent', 'type': 'text'},
{'name': 'region', 'type': 'text'},
{'name': 'surfacearea', 'type': 'real'},
{'name': 'indepyear', 'type': 'smallint'},
{'name': 'population', 'type': 'integer'},
{'name': 'lifeexpectancy', 'type': 'real'},
{'name': 'gnp', 'type': 'numeric'},
{'name': 'gnpold', 'type': 'numeric'},
{'name': 'localname', 'type': 'text'},
{'name': 'governmentform', 'type': 'text'},
{'name': 'headofstate', 'type': 'text'},
{'name': 'capital', 'type': 'integer'},
{'name': 'code2', 'type': 'character'},
],
},
{
'name': 'countrylanguage',
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'columns': [
{'name': 'countrycode', 'type': 'character'},
{'name': 'language', 'type': 'text'},
{'name': 'isofficial', 'type': 'boolean'},
{'name': 'percentage', 'type': 'real'},
],
},
]
assert postgres_connector.get_model() == postgres_db_model
assert postgres_connector.get_model(db_name='postgres_db') == postgres_db_model
assert postgres_connector.get_model(db_name='another_db') == []
Expand All @@ -445,63 +462,12 @@ def test_raised_error_for_get_model(mocker, postgres_connector):
assert postgres_connector.get_model() == []


def test_get_model_with_info(postgres_connector):
def test_get_model_with_info(
postgres_connector: PostgresConnector, postgres_db_model: list[dict]
) -> None:
"""Check that it returns the db tree structure"""
postgres_db_model = (
[
{
'name': 'city',
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'columns': [
{'name': 'id', 'type': 'integer'},
{'name': 'name', 'type': 'text'},
{'name': 'countrycode', 'type': 'character'},
{'name': 'district', 'type': 'text'},
{'name': 'population', 'type': 'integer'},
],
},
{
'name': 'country',
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'columns': [
{'name': 'code', 'type': 'character'},
{'name': 'name', 'type': 'text'},
{'name': 'continent', 'type': 'text'},
{'name': 'region', 'type': 'text'},
{'name': 'surfacearea', 'type': 'real'},
{'name': 'indepyear', 'type': 'smallint'},
{'name': 'population', 'type': 'integer'},
{'name': 'lifeexpectancy', 'type': 'real'},
{'name': 'gnp', 'type': 'numeric'},
{'name': 'gnpold', 'type': 'numeric'},
{'name': 'localname', 'type': 'text'},
{'name': 'governmentform', 'type': 'text'},
{'name': 'headofstate', 'type': 'text'},
{'name': 'capital', 'type': 'integer'},
{'name': 'code2', 'type': 'character'},
],
},
{
'name': 'countrylanguage',
'schema': 'public',
'database': 'postgres_db',
'type': 'table',
'columns': [
{'name': 'countrycode', 'type': 'character'},
{'name': 'language', 'type': 'text'},
{'name': 'isofficial', 'type': 'boolean'},
{'name': 'percentage', 'type': 'real'},
],
},
],
{},
)
assert postgres_connector.get_model_with_info() == postgres_db_model
assert postgres_connector.get_model_with_info(db_name='postgres_db') == postgres_db_model
assert postgres_connector.get_model_with_info() == (postgres_db_model, {})
assert postgres_connector.get_model_with_info(db_name='postgres_db') == (postgres_db_model, {})
assert postgres_connector.get_model_with_info(db_name='another_db') == (
[],
{'info': {'Could not reach databases': ['another_db']}},
Expand Down
22 changes: 11 additions & 11 deletions toucan_connectors/postgres/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,16 +604,16 @@


def build_database_model_extraction_query() -> str:
return """select t.table_catalog as database,
t.table_schema as schema,
CASE WHEN t.table_type = 'BASE TABLE' THEN 'table' ELSE lower(t.table_type) END as type,
t.table_name as name,
json_agg(json_build_object('name', c.column_name, 'type', c.data_type)) as columns
from
return """SELECT t.table_catalog AS database,
t.table_schema AS schema,
CASE WHEN t.table_type = 'BASE TABLE' THEN 'table' ELSE lower(t.table_type) END AS type,
t.table_name AS name,
JSON_AGG(JSON_BUILD_OBJECT('name', c.column_name, 'type', c.data_type)) AS columns
FROM
information_schema.tables t
inner join information_schema.columns c on
t.table_name = c.table_name
where t.table_type in ('BASE TABLE', 'VIEW')
and t.table_schema not in ('pg_catalog', 'information_schema', 'pg_internal')
group by t.table_schema, t.table_catalog, t.table_name, t.table_type;
INNER JOIN information_schema.columns c ON
t.table_name = c.table_name AND t.table_schema = c.table_schema
WHERE t.table_type IN ('BASE TABLE', 'VIEW')
AND t.table_schema NOT IN ('pg_catalog', 'information_schema', 'pg_internal')
GROUP BY t.table_schema, t.table_catalog, t.table_name, t.table_type;
"""