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.

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

Support default values in Postgresql converter #417

Closed
mjperrone opened this issue Jan 19, 2024 · 3 comments
Closed

Support default values in Postgresql converter #417

mjperrone opened this issue Jan 19, 2024 · 3 comments
Assignees

Comments

@mjperrone
Copy link
Contributor

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 #414 (comment)

@criccomini
Copy link
Contributor

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

@criccomini
Copy link
Contributor

criccomini commented Jan 19, 2024

@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

@criccomini criccomini self-assigned this Jan 19, 2024
@mjperrone
Copy link
Contributor Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants