'sqlparse.sql.Identifier' instances: Field 'value' is 'None'

apple / ccs-twistedextensions

Extensions to Twisted used by CalendarServer

https://www.calendarserver.org

Apache License 2.0

17 stars 29 forks source link

'sqlparse.sql.Identifier' instances: Field 'value' is 'None' #11

Closed macosforgebot closed 10 years ago

macosforgebot commented 12 years ago

carlos@… originally submitted this as ticket:486

Owner: @glyph

When parsing the following SQL fragment:

create table INVITE (
    INVITE_UID         varchar(255) not null,
    NAME               varchar(255) not null,
    RECIPIENT_ADDRESS  varchar(255) not null,
    HOME_RESOURCE_ID   integer      not null,
    RESOURCE_ID        integer      not null

    -- Need primary key on (INVITE_UID, NAME, RECIPIENT_ADDRESS)?
);

sqlparse generates an instance of type sqlparse.sql.Identifier to represent the varchar keyword.

With my version of sqlparse (0.1.3), the field value of that instance is None, and CalendarServer dies at twext.enterprise.dal.parseschema._ColumnParser.parseColumn().

The attached patch (diff against branches/release/CalendarServer-4.1.1-dev) attemts to address this.

macosforgebot commented 12 years ago

carlos@… originally submitted this as attachment:001-Fix-for-Identifier-instances.diff:⁠ticket:486

Attachment 001-Fix-for-Identifier-instances.diff (613 bytes) added

Diff against branches/release/CalendarServer-4.1.1-dev

macosforgebot commented 12 years ago

@glyph originally submitted this as comment:1:⁠ticket:486

Type changed from Defect to Task
Priority changed from 5: Not set to 4: Nice to have

Thanks for submitting this patch.

A few minor points:

patches should generally be submitted against trunk; we'll backport them to earlier releases if necessary.

we do have a very precise version requirement for sqlparse. Is our build script allowing for newer versions to be used? That would be problematic.

this would be quicker to integrate if you wouldn't mind writing a direct test, something to go in test_sqlparse.py. If you don't feel up to it, don't worry, we'll pick it up eventually.

Thanks again,

-glyph

macosforgebot commented 12 years ago

@glyph originally submitted this as comment:2:⁠ticket:486

Owner changed from @wsanchez to @glyph

macosforgebot commented 12 years ago

mail@… originally submitted this as comment:4:⁠ticket:486

btw. issue #⁠469 modifies the same line of code.

macosforgebot commented 11 years ago

@wsanchez originally submitted this as comment:5:⁠ticket:486

Milestone changed from CalendarServer-4.x to Later

macosforgebot commented 11 years ago

@wsanchez originally submitted this as comment:6:⁠ticket:486

Same problem as #⁠469?

macosforgebot commented 11 years ago

@wsanchez originally submitted this as comment:7:⁠ticket:486