gabledata / recap

Work with your web service, database, and streaming schemas in a single format.
https://recap.build
MIT License
334 stars 24 forks source link

Support default values in Postgresql converter #417

Closed mjperrone closed 10 months ago

mjperrone commented 10 months ago

Postgres supports setting default values for column values. Recap also supports default values.

Postgres reveals the default value in the information schema, column_default column. This should already be grabbed in the client query, so it could be used in the postgresql converter with column_props["COLUMN_DEFAULT"] to set the right value in the RecapType instantiation.

This came to my attention in https://github.com/recap-build/recap/pull/414#issuecomment-1899182470

criccomini commented 10 months ago

@mjperrone I'll take a look at this one. Thanks for raising it.

criccomini commented 10 months ago

@mjperrone I'm not 100% sure what you're seeing broken. The tests/integration/clients/test_postgresql.py test validates that both default=null and default=1 works:

                test_bigint BIGINT,
...
                test_not_null_default INTEGER NOT NULL DEFAULT 1,

These are validated here:

            UnionType(
                default=None,
                name="test_bigint",
                types=[NullType(), IntType(bits=64, signed=True)],
            ),
...
            IntType(bits=32, signed=True, name="test_not_null_default", default="1"),

And these validations are run for both:

test_struct_method_arrays_no_enforce_dimensions
test_struct_method_arrays_enforce_dimensions

The code that sets the default is here:

https://github.com/recap-build/recap/blob/main/recap/converters/dbapi.py#L20

mjperrone commented 10 months ago

That convinces me. I saw a related error when I was implementing the enums, and then went looking in the postgresql client (not realizing it was a subclass of dbapi) and the postgres converter, and saw no references to default.

That's what I get for not also making a failing test to prove the issue :D

I'll close this, my fault!