Reflecting default values in Firebird databases
Reflecting columns with default values fails assertion test. This may not happen on all tables, but did on at least one of mine. The problem is in lines 502 and 503 of firebird.py. They were:
assert row['fdefault']('fdefault').upper().startswith('DEFAULT '), row
defvalue = row['fdefault']('fdefault')![8:](8:)
The problem was, at least in my case, that there was a space before the word DEFAULT in row. This caused the assertion to fail. I fixed it with the following code:
assert row['fdefault']('fdefault').upper().lstrip().startswith('DEFAULT '), row
defvalue = row['fdefault']('fdefault').lstrip()![8:](8:)
Everything worked fine after that.
Comments (10)
-
-
Fixed markup
-
repo owner - changed milestone to 0.6.xx
-
- changed component to firebird
I tried to come up with a resolutive answer, googling around the net.
I've spotted a few cases that actually tries to give some meaning to the
rdb$default_source
system column, and all basically do the same as the original (that is, current SA trunk) code:499 # does it have a default value? 500 if row['fdefault']('fdefault') is not None: 501 # the value comes down as "DEFAULT 'value'" 502 assert row['fdefault']('fdefault').upper().startswith('DEFAULT '), row 503 defvalue = row['fdefault']('fdefault')[8:](8:) 504 args.append(schema.DefaultClause(sql.text(defvalue)))
So, while I understand that the impact of the proposed change is definitely tiny, I'm -0 on applying it, at least until we have a reproducible unit test that is :-)
From what I know about Firebird, this may be very well caused by buggy GUI frontends to the FB database: consider that the column in question is actually a BLOB, that is, free text, where one can write any amount of spam. I bet that internally the FB engine performs some sort of
eval()
over the value (probably casting it to the right domain): I'd be curious to know if the reporter's case actually works ok.I'll of course take care of it if the consensum decides otherwise.
-
- assigned issue to
After some more research, here is a possible patch that should solve your issue:
Index: test/dialect/test_firebird.py =================================================================== --- test/dialect/test_firebird.py (revisione 6077) +++ test/dialect/test_firebird.py (copia locale) @@ -29,7 +29,8 @@ photo img_domain, d date, t time, - dt timestamp)**) + dt timestamp, + redundant str_domain DEFAULT NULL)**) con.execute(**ALTER TABLE testtable ADD CONSTRAINT testtable_pk PRIMARY KEY (question)**) con.execute('''CREATE TRIGGER testtable_autoid FOR testtable @@ -53,7 +54,8 @@ metadata = MetaData(testing.db) table = Table('testtable', metadata, autoload=True) eq_(set(table.columns.keys()), - set(['answer', 'remark', 'photo', 'd', 't', 'dt']('question',)), + set(['answer', 'remark', + 'photo', 'd', 't', 'dt', 'redundant']('question',)), "Columns of reflected table didn't equal expected columns") eq_(table.c.question.primary_key, True) eq_(table.c.question.sequence.name, 'gen_testtable_id') @@ -64,6 +66,7 @@ eq_(table.c.remark.type.__class__, firebird.FBText) eq_(table.c.remark.server_default.arg.text, "''") eq_(table.c.photo.type.__class__, firebird.FBBinary) + assert table.c.redundant.server_default is None # The following assume a Dialect 3 database eq_(table.c.d.type.__class__, firebird.FBDate) eq_(table.c.t.type.__class__, firebird.FBTime) Index: lib/sqlalchemy/databases/firebird.py =================================================================== --- lib/sqlalchemy/databases/firebird.py (revisione 6077) +++ lib/sqlalchemy/databases/firebird.py (copia locale) @@ -498,10 +498,13 @@ # does it have a default value? if row['fdefault']('fdefault') is not None: - # the value comes down as "DEFAULT 'value'" - assert row['fdefault']('fdefault').upper().startswith('DEFAULT '), row - defvalue = row['fdefault']('fdefault')[8:](8:) - args.append(schema.DefaultClause(sql.text(defvalue))) + # the value comes down as "DEFAULT 'value'": there may be + # more than one space around the "DEFAULT" keyword + defexpr = row['fdefault']('fdefault').lstrip() + assert defexpr.startswith('DEFAULT '), "Unrecognized default value: %s" % defexpr + defvalue = row['fdefault']('fdefault')[8:](8:).strip() + if defvalue != 'NULL': + args.append(schema.DefaultClause(sql.text(defvalue))) col = schema.Column(*args, **kw) if kw['primary_key']('primary_key'):
I'll apply it to trunk once the solution is confirmed.
-
Michael, I notice you marked the issue for the 0.6.xx milestone: should I (eventually) apply it in that branch instead of trunk?
-
I somehow pasted the wrong version of the patch :-| The last chunk should be
+ defexpr = row['fdefault']('fdefault').lstrip() + assert defexpr.startswith('DEFAULT '), "Unrecognized default value: %s" % defexpr + defvalue = defexpr[8:](8:).strip() + if defvalue != 'NULL': + args.append(schema.DefaultClause(sql.text(defvalue)))
-
repo owner Replying to lele:
Michael, I notice you marked the issue for the 0.6.xx milestone: should I (eventually) apply it in that branch instead of trunk?
yeah, and the FB dialect needs to be ported to 0.6 overall.
-
- changed status to resolved
This should be fixed by 6257.
-
repo owner - removed milestone
Removing milestone: 0.6.xx (automated comment)
- Log in to comment
Could you attach the original definition of the involved table?