Skip to content
This repository was archived by the owner on May 7, 2026. It is now read-only.

fix(ibis): added primary key management on postgres and updated mssql and mysql to manage composite keys #1106

Closed
yalexbadan wants to merge 16 commits into from
Closed

fix(ibis): added primary key management on postgres and updated mssql and mysql to manage composite keys #1106

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e159f5a
fix(ibis): added primary key management on postgres and updated mssql…
yalexbadan Mar 25, 2025
0b5eb0a
fix(ibis): runned just for code formatting
yalexbadan Mar 26, 2025
bfcd4ed
fix(ibis): fixed tests
yalexbadan Mar 28, 2025
8076da8
fix(ibis): fmt code
goldmedal Mar 31, 2025
7bcce3b
fix(ibis): fixed clickhouse and trino metadata classes
yalexbadan Mar 31, 2025
dbff7d8
fix(ibis): fixed test models
yalexbadan Mar 31, 2025
592ed28
fix(ibis): updated java servers with new models
yalexbadan Mar 31, 2025
3238116
fix(ibis): reverted to initial proposal
yalexbadan Apr 23, 2025
2337028
fix(ibis): chagend composed primary key logic
yalexbadan Apr 23, 2025
feb19ba
Merge branch 'main' into issues/1105
yalexbadan Apr 23, 2025
10fc6f6
fix(ibis): minor fix on postgres foreign key extraction query
yalexbadan Apr 24, 2025
a119b55
fix(ibis): fixed rever issues
yalexbadan Apr 24, 2025
43cef18
fix(ibis): fixed revert issues
yalexbadan Apr 24, 2025
7c8ed81
fix(ibis): fixed revert regressions
yalexbadan Apr 29, 2025
721d978
fix(ibis): fixed revert regressions
yalexbadan Apr 30, 2025
e6e7c02
fix(ibis): fixed other revert regressions
yalexbadan May 6, 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 ibis-server/app/model/metadata/dto.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class Table(BaseModel):
columns: list[Column]
description: str | None = None
properties: TableProperties = None
primaryKey: str | None = None
primaryKey: list[str] | None = None


class ConstraintType(Enum):
Expand Down
5 changes: 3 additions & 2 deletions ibis-server/app/model/metadata/mssql.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def get_table_list(self) -> list[Table]:
catalog=row["catalog"],
table=row["table_name"],
),
primaryKey="",
primaryKey=[],
)

# table exists, and add column to the table
Expand All @@ -96,7 +96,8 @@ def get_table_list(self) -> list[Table]:
)
# if column is primary key
if row["is_pk"] == "YES":
unique_tables[schema_table].primaryKey = row["column_name"]
unique_tables[schema_table].primaryKey.append(row["column_name"])

return list(unique_tables.values())

def get_constraints(self) -> list[Constraint]:
Expand Down
4 changes: 2 additions & 2 deletions ibis-server/app/model/metadata/mysql.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def get_table_list(self) -> list[Table]:
catalog="",
table=row["table_name"],
),
primaryKey="",
primaryKey=[],
)

# table exists, and add column to the table
Expand All @@ -71,7 +71,7 @@ def get_table_list(self) -> list[Table]:
)
# if column is primary key
if row["column_key"] == "PRI":
unique_tables[schema_table].primaryKey = row["column_name"]
unique_tables[schema_table].primaryKey.append(row["column_name"])
return list(unique_tables.values())

def get_constraints(self) -> list[Constraint]:
Expand Down
4 changes: 3 additions & 1 deletion ibis-server/app/model/metadata/oracle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def get_table_list(self) -> list[Table]:
catalog="", # Oracle doesn't use catalogs.
table=row["TABLE_NAME"],
),
primaryKey="",
primaryKey=[],
)

unique_tables[schema_table].columns.append(
Expand All @@ -100,6 +100,8 @@ def get_table_list(self) -> list[Table]:
properties=None,
)
)

# TODO: manage primary key

@coderabbitai coderabbitai Bot Mar 25, 2025

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.

💡 Verification agent

❓ Verification inconclusive

Implementation needed for primary key collection.

The TODO comment indicates that the implementation for collecting primary key information is still pending. Unlike the MySQL and MSSQL implementations, there's no code to populate the primaryKey list yet.

The Oracle metadata class needs to be updated to collect primary key information similar to the MySQL and MSSQL implementations. Without this, composite primary keys won't be properly represented for Oracle tables.

Check if there is a related SQL query that can be added to identify primary keys:

🏁 Script executed:

#!/bin/bash
# Search the codebase for examples of Oracle primary key queries
rg -A 5 -B 5 "PRIMARY KEY" --glob "*.py" | grep -i oracle

Length of output: 59

Update Oracle Metadata to Properly Handle Primary Keys [Critical]

The Oracle metadata implementation in ibis-server/app/model/metadata/oracle.py (lines 103–104) still contains a TODO for primary key handling. Unlike the MySQL and MSSQL implementations, this class does not currently populate the primaryKey list. This omission means that composite primary keys for Oracle tables may not be properly represented.

Suggestions:

  • Investigate Oracle system views (e.g., ALL_CONSTRAINTS and ALL_CONS_COLUMNS) to construct an SQL query that retrieves primary key columns.
  • Update the Oracle metadata extraction logic to populate the primaryKey list in a manner consistent with the other implementations.
  • Manually verify that the proposed SQL query and logic integration correctly capture both single and composite primary keys for Oracle tables.


Comment on lines +104 to +105

@goldmedal goldmedal May 12, 2025

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.

What's the comment propose? Could you explain more?

return list(unique_tables.values())

Expand Down
13 changes: 12 additions & 1 deletion ibis-server/app/model/metadata/postgres.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def get_table_list(self) -> list[Table]:
c.data_type,
c.is_nullable,
c.ordinal_position,
tc.constraint_type,
obj_description(cls.oid) AS table_comment,
col_description(cls.oid, a.attnum) AS column_comment
FROM
Expand All @@ -44,6 +45,12 @@ def get_table_list(self) -> list[Table]:
pg_attribute a
ON a.attrelid = cls.oid
AND a.attname = c.column_name
LEFT JOIN information_schema.key_column_usage kcu
ON c.table_name = kcu.table_name
AND c.column_name = kcu.column_name
AND c.table_schema = kcu.table_schema
LEFT JOIN information_schema.table_constraints tc
ON kcu.constraint_name = tc.constraint_name
WHERE
t.table_type IN ('BASE TABLE', 'VIEW')
AND t.table_schema NOT IN ('information_schema', 'pg_catalog');
Expand All @@ -67,7 +74,7 @@ def get_table_list(self) -> list[Table]:
catalog=row["table_catalog"],
table=row["table_name"],
),
primaryKey="",
primaryKey=[],
)

# table exists, and add column to the table
Expand All @@ -80,6 +87,10 @@ def get_table_list(self) -> list[Table]:
properties=None,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +104 to 112

@goldmedal goldmedal May 12, 2025

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.

Is it possible to have multiple primary keys in the same table? IMO, the postgres only allows the table has only one primary key. Given a table customer:

test=# alter table customer add constraint cky primary key (c_custkey);
ALTER TABLE
test=# alter table customer add constraint cky primary key (c_name);
ERROR:  multiple primary keys for table "customer" are not allowed

)

if row["constraint_type"] == 'PRIMARY KEY':
unique_tables[schema_table].primaryKey.append(row["column_name"])

return list(unique_tables.values())

def get_constraints(self) -> list[Constraint]:
Expand Down