fix allowed table and field names

[niphlod]

As discussed in web2py-developers (such as here ), we should avoid any table and field name that is not coherent with python identifiers. The cleanup() needs a revision and tests should be added to avoid regressions. We have rname for legacy and/or funny names.

[michele-comitini]

Problem could be back compatibility

[niphlod]

it was that way. Then cleanup was relaxed to.... pretty anything.
Versions slipped by, and now we face a problem.

[ilvalle]

I agree, and we should have the same approach for situations such as:

db = DAL("sqlite:memory")
db.define_table('a', Field('1'))

raises

AttributeError: 'Field' object has no attribute 'iteritems'

while

db = DAL("sqlite:memory")
db.define_table('a', Field(1))

raises

SyntaxError: Field: invalid field name: 1, use rname for "funny" names

In addition, the following works at 'pydal' level:

db = DAL("sqlite:memory", migrate=False)
db.define_table("1", Field('a'))

but then it fails at 'db' level:

db = DAL("sqlite:memory")
db.define_table("1", Field('a'))

raises

sqlite3.OperationalError: near "1": syntax error

[niphlod]

this should be fixed before the next release to avoid finding ourselves in 6 months locked-in for backward compatibility

[gi0baro]

I've moved this to 15.04 milestone. If anybody is interested into fixing this, let us know.

[niphlod]

this is quite an easy fix, once we all agree. There's nothing documented in the book, so a clear and large statement in the CHANGELOG would suffice.

I didn't read anything conclusive on web2py-developers, but I strongly think that this should be closed once and for all.
I (re)vote for cleanup() restricting table and field names to valid python identifiers. If I'm not mistaken, this is the reference.
Pinging everybody for the call-to-arms: @mdipierro , @gi0baro , @michele-comitini , @abastardi , @ilvalle , @leonelcamara ... feel free to chime in.

[gi0baro]

+1 on @niphlod vote

[niphlod]

ok, I think we can safely assume the only responsible for whining users would be me and @gi0baro . I'm ready to take the heat.

@gi0baro : ^[^\d\W]\w*\Z should be the regex.
BTW: I'm not sure about leaving up the whole python3 range for identifiers, which accounts for unicode characters as well. I'd stay with valid python2 identifiers.

[gi0baro]

@niphlod ok with that

[niphlod]

ok, will send a PR soon enough.

[niphlod]

@gi0baro : another question... I'd go for the same routine for both tablenames and fieldnames, which is basically:

• a str
• a valid python identifier
• not a python keyword
• not starting with underscore
• not containing dots

table would have also the restriction of not being a yet-existing attribute of DAL.

tl;dr: adding "a valid python identifier" constraint to the existing one on Table and all of the above for Field

[niphlod]

umpf. quite there. How the heck should we check that è is not a str in python3 ? Meaning... we don't check for Field and Tablenames in python2 to be valid basestr that would make things easier porting to python3... we only check with isinstance(obj, str) . How can we make a validation on python3 for the same thing ?

[gi0baro]

@niphlod sorry, I was AFK. Anyway, I'm missing your point, since

isinstance('è', str)

is True both on python 2 and python 3. And on python 2 isinstance('è', basestring) is True as well.
If we want to remove special characters I think we should use a regex.

[niphlod]

right ........ will do this evening.

[michele-comitini]

sorry if I chime in late

@niphlod, does this help?
https://stackoverflow.com/questions/5474008/regular-expression-to-confirm-whether-a-string-is-a-valid-identifier-in-python

2015-05-20 10:40 GMT+02:00 niphlod notifications@github.com:

right ........ will do this evening.

—
Reply to this email directly or view it on GitHub
#67 (comment).

[niphlod]

unfortunately not. in py3 it would allow also unicode, since \W is in. Seeing the code, I won't touch "cleanup()" as it seems that the validation is both in Table and Field __init__
I'm in a place where no python exist but I think I'll refactor a bit those checks and just leave ^[^\d_][_0-9a-zA-Z]+$

#current
if (not isinstance(tablename, str) or tablename[0] == '_'
            or hasattr(DAL, tablename) or '.' in tablename
            or REGEX_PYTHON_KEYWORDS.match(tablename)
            ):
#will probably become
# in regex.py
NEWREGEX = re.compile('^[^\d_][_0-9a-zA-Z]+$')
....
if (not isinstance(tablename, basestr) or hasattr(DAL, tablename)
            or NEWREGEX.match(tablename)
            or REGEX_PYTHON_KEYWORDS.match(tablename)
            ):