3 ms·

This is a great example of why SQLAlchemy is a terrible ORM, and chatGPT is not alone in making the same mistake as millions of engineers, in fact likely where

This is a great example of why SQLAlchemy is a terrible ORM, and chatGPT is not alone in making the same mistake as millions of engineers, in fact likely where it learned the mistake. default = python code evaluates the default value as necessary for each new record server_default = the initial CREATE TABLE uses this computed (from python) value, thus the hardcoded UUID. They also could have done server_default=text("uuid_generate_v4()") if they had that corresponding module installed on postgres.

viraptor 2y ago
I'm not sure what you think SQLalchemy can fix here? There has to be an option to pass a static default value and the library does not have a visibility into the parse tree. What's the proposed solution?
- dragonwriter 2y ago
  One solution would be to have a separate default (static value) and default_factory (callable that returns a value) keyword argument, and make a static default on a PK (or other unique) column, by default, be an error.
- hipadev23 2y ago
  > I'm not sure what you think SQLalchemy can fix here Separate table definition (DDL) from row-insertion (DML) definitions along with their corresponding defaults.
- Nullabillity 2y ago
  > There has to be an option to pass a static default value Does there, though? You can always set `Column(default = lambda: 1234)`
  - viraptor 2y ago
    Yes, because you're creating a table and the value needs to be encoded as string in the CRATE TABLE query. You could in theory pass a lambda which is disassembled in SQLalchemy, then checked against the pattern returning a constant, then the constant itself gets used... But I'm not sure SQLalchemy would go that way. It's not as crazy as Linq. To be explicit: You're not seeing a function called later in python. You're setting an attribute on a database table.
    - Nullabillity 2y ago
      No[0], default= is inserted into the DML queries by the client. A DDL DEFAULT clause is only generated if you set server_default= instead. [0]: https://docs.sqlalchemy.org/en/20/core/defaults.html https://docs.sqlalchemy.org/en/20/core/defaults.html
Lunrtick 2y ago
They could also have done default=uuid.uuid4 to have a new id each time, or default=lambda : str(uuid.uuid4()). It's not really related to whether or not it's a database default. It seems quite unfair to place the blame on SQLAlchemy here, or even Python. Even a statically typed language wouldn't prevent this kind of issue - the author of the code is the only person who can decide when they mean "use this exact string each time" or "use this function to give me a new string each time". I suppose a column description API could follow the dataclass style definition, with different argument names for default and default_func. That would (I think) prevent this from happening.
- jowea 2y ago
  I could imagine some sort of static analysis saying that using a constant value instead of a function is a mistake for columns with unique set.
  - throw156754228 2y ago
    Exactly, this is an error that should've been picked up statically, before unit even.
  - Lunrtick 2y ago
    That's a good point! I suppose in Python that could be a guard in the column definition function.
hn_throwaway_99 2y ago
> This is a great example of why SQLAlchemy is a terrible ORM I feel like "terrible ORM" is a tautology. This issue highlighted in one respect a fundamental problem with all ORMs and why I despise them so much - they're the absolute worst of a leaky abstraction, and as I like to say "they make the easiest stuff a bit easier, and they make the harder stuff way harder". E.g. in this specific case they don't prevent you from needing to know the details of default column value initialization, but apparently they must have also obscured a simple duplicate key constraint to some level that it took 5 days to find the root cause of this bug. Just learn SQL. You'll need to know it anyway even if you do use an ORM, the basics aren't hard, the skill is much more transferable than the esoteric details of some ORM, and there are lots of good libraries that make dealing directly with SQL easy and safe (slonik for postgres is a favorite of mine, but there are other similar ones).
hfusdvdkchfbfb 2y ago
[flagged]

viraptor 2y ago

I'm not sure what you think SQLalchemy can fix here? There has to be an option to pass a static default value and the library does not have a visibility into the parse tree. What's the proposed solution?

dragonwriter 2y ago
One solution would be to have a separate default (static value) and default_factory (callable that returns a value) keyword argument, and make a static default on a PK (or other unique) column, by default, be an error.
hipadev23 2y ago
> I'm not sure what you think SQLalchemy can fix here Separate table definition (DDL) from row-insertion (DML) definitions along with their corresponding defaults.
Nullabillity 2y ago
> There has to be an option to pass a static default value Does there, though? You can always set `Column(default = lambda: 1234)`
- viraptor 2y ago
  Yes, because you're creating a table and the value needs to be encoded as string in the CRATE TABLE query. You could in theory pass a lambda which is disassembled in SQLalchemy, then checked against the pattern returning a constant, then the constant itself gets used... But I'm not sure SQLalchemy would go that way. It's not as crazy as Linq. To be explicit: You're not seeing a function called later in python. You're setting an attribute on a database table.
  - Nullabillity 2y ago
    No[0], default= is inserted into the DML queries by the client. A DDL DEFAULT clause is only generated if you set server_default= instead. [0]: https://docs.sqlalchemy.org/en/20/core/defaults.html https://docs.sqlalchemy.org/en/20/core/defaults.html

Lunrtick 2y ago

They could also have done default=uuid.uuid4 to have a new id each time, or default=lambda : str(uuid.uuid4()). It's not really related to whether or not it's a database default. It seems quite unfair to place the blame on SQLAlchemy here, or even Python. Even a statically typed language wouldn't prevent this kind of issue - the author of the code is the only person who can decide when they mean "use this exact string each time" or "use this function to give me a new string each time". I suppose a column description API could follow the dataclass style definition, with different argument names for default and default_func. That would (I think) prevent this from happening.

jowea 2y ago
I could imagine some sort of static analysis saying that using a constant value instead of a function is a mistake for columns with unique set.
- throw156754228 2y ago
  Exactly, this is an error that should've been picked up statically, before unit even.
- Lunrtick 2y ago
  That's a good point! I suppose in Python that could be a guard in the column definition function.

hn_throwaway_99 2y ago

> This is a great example of why SQLAlchemy is a terrible ORM I feel like "terrible ORM" is a tautology. This issue highlighted in one respect a fundamental problem with all ORMs and why I despise them so much - they're the absolute worst of a leaky abstraction, and as I like to say "they make the easiest stuff a bit easier, and they make the harder stuff way harder". E.g. in this specific case they don't prevent you from needing to know the details of default column value initialization, but apparently they must have also obscured a simple duplicate key constraint to some level that it took 5 days to find the root cause of this bug. Just learn SQL. You'll need to know it anyway even if you do use an ORM, the basics aren't hard, the skill is much more transferable than the esoteric details of some ORM, and there are lots of good libraries that make dealing directly with SQL easy and safe (slonik for postgres is a favorite of mine, but there are other similar ones).

hfusdvdkchfbfb 2y ago

[flagged]