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

postgresql compare_server_default with floats #241

Closed
sqlalchemy-bot opened this Issue Nov 3, 2014 · 7 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Nov 3, 2014

Migrated issue, originally created by Dimitrios Theodorou (@dtheodor)

After updating Alembic from 0.6.2 to the latest I am getting an exception in the PostgresqlIpml.compare_server_default. In particular:

My DB schema:

CREATE TABLE stuff (
value1 float DEFAULT 0
)

My SQL Alchemy model:

class Stuff(Base):
  value1 = Column(Float, server_default="0.0")

The compare_server_default emits this SQL query (on the left is the rendered_inspector_default, on the right the rendered_metadata_default):

SELECT 0 = '0.0';

which fails with the DataError ERROR: invalid input syntax for integer: "0.0"

In Postgres, the DEFAULT values 0, 0.0 and '0.0' are all valid and have identical end-result for a float column.

What would the "proper" solution be here, catch the exception and return False in the comparison, or check the column type and cast to float before comparing?

To avoid the error I set the server_default to a float value (server_default=text("0.0"))

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Nov 3, 2014

Changes by Dimitrios Theodorou (@dtheodor):

  • edited description
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Nov 3, 2014

Michael Bayer (@zzzeek) wrote:

was about to say, this should really be stated as text("0.0"); when you put "0.0" directly that means the default is the string value "0.0". the docs for server_default state that it will be quoted as a string otherwise.

as far as the exception here, not sure. one idea is to catch the exception and emit as a warning, since this can be fixed on the user end. or to do the cast. let's see how the cast works can you send me a PR ?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 12, 2015

Michael Bayer (@zzzeek) wrote:

OK, I guess the idea from the PR is that Postgresql is fine with "default" having the quotes, e.g. this:

CREATE TABLE a (
	id SERIAL NOT NULL, 
	value1 FLOAT DEFAULT '0.0', 
	PRIMARY KEY (id)
)

so in that sense, even though text() is more specific here, this works anyway so might as well not have alembic's tests choke on it.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 12, 2015

Dimitrios Theodorou (@dtheodor) wrote:

Yeah, so this example (from real-life code) is bogus (both DEFAULT 0 and server_default="0.0" are misconfigurations), however it is valid Postgres. This PR tries to avoid the alembic autogenerate crash, and in particular it decides to consider the 2 values equal.

Since postgres comparisons use db queries, right now they are kind of unique in that they may crash, leaving you with a failed autogenerate with a stack trace. Is this acceptable or should alembic do some catching of exceptions with some logic whether to consider the comparison as failed and/or logging towards the user?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 12, 2015

Michael Bayer (@zzzeek) wrote:

changelog for #241, fixes #241

c2ee9f5

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 12, 2015

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 12, 2015

Michael Bayer (@zzzeek) wrote:

i think its fine, if pg accepts '0.0' for the default but then not in the SQL, we just work around it as we're doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment