dlt-hub / dlt

data load tool (dlt) is an open source Python library that makes data loading easy 🛠️
https://dlthub.com/docs
Apache License 2.0
2.65k stars 175 forks source link

When ingesting a sql_database with a UUID column, getting a flood of warnings #1862

Closed neuromantik33 closed 1 month ago

neuromantik33 commented 1 month ago

dlt version

1.0.0

Describe the problem

Exactly what the title says.

Expected behavior

No annoying warnings log messages.

Steps to reproduce

Operating system

Linux

Runtime environment

Local

Python version

3.11

dlt data source

built-in sql_database

dlt destination

Google BigQuery

Other deployment details

No response

Additional information

The bug can be easily corrected by changing https://github.com/dlt-hub/dlt/blob/devel/dlt/sources/sql_database/schema_types.py#L89

Before

    if hasattr(sqltypes, "Uuid") and isinstance(sql_t, sqltypes.Uuid):
        # we represent UUID as text by default, see default_table_adapter
        col["data_type"] = "text"
    if isinstance(sql_t, sqltypes.Numeric):
        # check for Numeric type first and integer later, some numeric types (ie. Oracle)
        # derive from both
        # all Numeric types that are returned as floats will assume "double" type
        # and returned as decimals will assume "decimal" type
        if sql_t.asdecimal is False:
            col["data_type"] = "double"

After

    if hasattr(sqltypes, "Uuid") and isinstance(sql_t, sqltypes.Uuid):
        # we represent UUID as text by default, see default_table_adapter
        col["data_type"] = "text"
    elif isinstance(sql_t, sqltypes.Numeric):
        # check for Numeric type first and integer later, some numeric types (ie. Oracle)
        # derive from both
        # all Numeric types that are returned as floats will assume "double" type
        # and returned as decimals will assume "decimal" type
        if sql_t.asdecimal is False:
            col["data_type"] = "double"
rudolfix commented 1 month ago

this is actually a bug! do you maybe want to push this fix as PR? we'll release it early next week. thanks!