Reflecting default values in Firebird databases

[Former user]

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.

[Lele Gaifax]

Could you attach the original definition of the involved table?

[Lele Gaifax]

Fixed markup

[Lele Gaifax]

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.

[Lele Gaifax]

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.

[Lele Gaifax]

Michael, I notice you marked the issue for the 0.6.xx milestone: should I (eventually) apply it in that branch instead of trunk?

[Lele Gaifax]

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)))

[Mike Bayer]

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.

[Lele Gaifax]

This should be fixed by 6257.

[Mike Bayer]

Removing milestone: 0.6.xx (automated comment)