1. Its only in two places. Once for tablename, and once for fieldname.
And then this needs to be done for sql.py just in case
2. Yes very chatty. What about the following?
DAL('sqlite....', pool_size=1, check_reserve=None)
DAL('sqlite....', pool_size=1, check_reserve='common')
DAL('sqlite....', pool_size=1, check_reserve='all')
DAL('sqlite....', pool_size=1, check_reserve='postgres')
DAL('sqlite....', pool_size=1, check_reserve='mssql')
This way we can define the level of check. Then the check will be
placed in a function of db. db._verify_reserved_keyword(keyword) which
will perform the checks based on the self.check_reserve.
None would be no name checking, to keep backwards compatibility and
for production systems. comments?
3. How is it ambiguous, you hit the definition right on target. Its
common SQL commands that without these commands you don't have SQL,
and if you use one of these terms in the current system you get an
OperationalError anyways.
-Thadeus
On Tue, Feb 2, 2010 at 3:09 PM, Jonathan Lundell <jlund...@pobox.com> wrote:
> On Feb 2, 2010, at 12:48 PM, Thadeus Burgess wrote:
>
>> You can view a proposed change, I have sent this to Massimo to look at.
>>
>> http://code.google.com/r/thadeusburgess-web2py/source/detail?r=e875496cc5978200fb6c0aa0f85a8df1a945df21
>>
>> I'm not sure what keywords we really want to use or not, but this is a
>> starting point.
>>
>> I added two lists to BaseAdapter global. KEYWORDS_ALL and KEYWORDS_COMMON.
>>
>> My thought is that no tablename or column name should pass through
>> KEYWORDS_COMMON, then KEYWORDS_ALL can be used to check integration
>> with other database types. So if it is in KEYWORDS_ALL just log a
>> warning.
>>
>> Then each adapter will have their own set of reserved words that will
>> be stored in a list named KEYWORDS. So a tablename or column name will
>> fail if it is in either KEYWORDS_COMMON or KEYWORDS, and will send a
>> warning if it is in KEYWORDS_ALL.
>>
>> I think this could go into sql.py as just fail on KEYWORDS_COMMON and
>> warn on KEYWORDS_ALL, and then the new dal will break it down by
>> database specifics.
>>
>> What do you think?
>
> I think it's a pretty good start. A couple of nits, though.
>
> 1. The test ought to be encapsulated somewhere, presumably in dal.py, so
> you're not repeating the logic so many places.
>
> 2. I'm not so sure about logging a warning; seems like it'd be noisy. Could
> it maybe be a property of a table that defaults to COMMON but can be set to
> ALL at table-creation time? Or something like that? I probably don't care
> about making the test in production, after all.
>
> 3. KEYWORDS_COMMON as a name: I assume you mean "keywords that are commonly
> reserved"? It also suggests "keywords that are common to all adapters".
> Regardless, it's ambiguous; at the very least the criteria for inclusion
> should be documented in comments with the lists.
>
> --
> You received this message because you are subscribed to the Google Groups
> "web2py-users" group.
> To post to this group, send email to web...@googlegroups.com.
> To unsubscribe from this group, send email to
> web2py+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/web2py?hl=en.
>
>
--
You received this message because you are subscribed to the Google Groups
"web2py-users" group.
To post to this group, send email to web...@googlegroups.com.
To unsubscribe from this group, send email to
web2py+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/web2py?hl=en.
