patches: Fix a bug on sqlvar type="nb".

product/ERP5Type/patches/sqlvar.py

@@ -153,14 +153,10 @@ else: # For easy diff with original (ZSQLMethods 3.14)
                     ('.%06u' % (v.micros() % 1000000))[:1+n] if n else '')
             except Exception:
                 t = 'datetime'
-
-        elif t=='nb' and not v:
-            t = 'empty string'
-
         else:
             if not isinstance(v, (str, StringTypes)):
                 v = str(v)
-            if t == 'nb':
+            if not v and t == 'nb':
                 if 'optional' in args and args['optional']:
                     return 'null'
                 else:

[Vincent Pelletier]

Could you share the error this was causing ?

Also is this change a backport from some recent ZSQLMethods version (and this would be a monkey-patch update) or is it a local fix to the monkey-patch ? I do not know how far we already diverge from ZSQLMethods 3.14, but it seems this monkey-patch was intended to keep following ZSQLMethods.

[Yusei Tahara]

Without my patch, the following error occurs even if non-empty string is given:

  Module Products.ERP5Type.patches.sqlvar, line 168, in render
    raise ValueError(err)
ValueError: Invalid empty string value for <em>sequence-item</em>

Original ZSQLMethods 3.14 code is below, this is the same as my patch:

https://github.com/zopefoundation/Products.ZSQLMethods/blob/3.14/src/Shared/DC/ZRDB/sqlvar.py#L128

I think it was just a simple mistake, but it seems that we do not have any tests for type="nb". Maybe I should add it.

[Yusei Tahara]

One more thing, in patches/sqlvar.py there are actually two patches, one is a patch for Zope2 based on old ZSQLMethod, the other one is a patch for Zope4 based on recent ZSQLMethod(3.14 or more recent version). This time, the patch for Zope4 had a bug.

[Vincent Pelletier]

Original ZSQLMethods 3.14 code is below, this is the same as my patch:

Indeed, which made me wonder why our code diverged so much then, and I found this: 8e073789 . So basically we do not intend to follow ZSQLMethod any more, and just took ownership (in a maintenance sense) of this code. And what you are fixing is a bug maybe introduced in that change and somehow triggered by the use of Zope4. Or maybe something in 552ffed5 .

[Yusei Tahara]

That bug was introduced in 610972af