Add references table hint
#1925
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
| `columns` corresponds to the `referenced_columns` in the referenced table and their order should match. | ||
| """ | ||
|
|
||
| columns: Sequence[str] |
There was a problem hiding this comment.
@sh-rp this is correct? corresponds to the one used in dbt-gen?
dlt/common/schema/utils.py
Outdated
| @@ -559,6 +560,18 @@ def normalize_table_identifiers(table: TTableSchema, naming: NamingConvention) - | |||
| else: | |||
| new_columns[new_col_name] = c | |||
| table["columns"] = new_columns | |||
| reference = table.get("references") | |||
There was a problem hiding this comment.
this is good. but we need to change the table diff functions above. the questions is: should we replace references or merge them ie by making sure duplicates are not endlessly inserted. could you check it out and test?
| if reflection_level == "minimal": | ||
| return None | ||
| result: List[TTableReference] = [] | ||
| for fk_constraint in table.foreign_key_constraints: |
There was a problem hiding this comment.
if table is reflected, we have foreign keys available, or here a call to database is made to populate the constraints?
| return None | ||
| result: List[TTableReference] = [] | ||
| for fk_constraint in table.foreign_key_constraints: | ||
| referenced_table = fk_constraint.referred_table.name |
There was a problem hiding this comment.
what happens if the tables is in a separate schema in the database? is the other schema reflected lazily? should we add reference to tables in schemas different from the table? WDYT?
There was a problem hiding this comment.
Separate schemas should work the same. But I didn't consider it. The reference hint only has table name so a reference to a different schema doesn't make sense, unless we add a dataset_name property?
Probably best to just filter out different schemas.
We have always been using metadata.reflect(resolve_fks=True) (default) so there are no extra db calls from before, referenced tables are always reflected whether they're synced or not so we have the referred_table property.
But should handle the case when referred_table is not available, may be when using custom metadata.
+ reflect references from foreign keys in sqlalchemy source
steinitzu
force-pushed
the
table-reference-hint
branch
2 times, most recently
from
33dbea8
to
195f88a
Compare
steinitzu
force-pushed
the
table-reference-hint
branch
from
ecf3ef1
to
bb49c49
Compare
There was a problem hiding this comment.
the code is good. from your comment is pretty clear that we should get the references in sql alchemy only when requested by the user so
- a new flag to
sql_databaseandsql_table - do not reflect foreign keys if flag disabled (should be default)
- do not create references if the above
Description
Adds new
referencestable hint. Can be added via@resourcedecorator andapply_hintsTakes a list of "foreign key" references.
SQLAlchemy database source automatically generates the hint from tabel foreign keys
Related Issues
#1713