FreeTDS / freetds

Official FreeTDS repository
http://www.freetds.org/
GNU General Public License v2.0
462 stars 161 forks source link

max length of CHAR / VARCHAR on Sybase 15+ (and maybe Sybase 12.5+) #392

Open blieusong opened 3 years ago

blieusong commented 3 years ago

Hi,

On Sybase 15.7, datacopy fails to copy data from a table which has a VARCHAR(2500) field.

It recreates the table mostly correctly but the VARCHAR(2500) ends up as a CHAR(255).

When looking at the code, I'm a bit puzzled by that part:

TDSRET
tds_get_column_declaration(TDSSOCKET * tds, TDSCOLUMN * curcol, char *out)
{
    const char *fmt = NULL;
    /* unsigned int is required by printf format, don't use size_t */
    unsigned int max_len = IS_TDS7_PLUS(tds->conn) ? 8000 : 255;
...
}

Is there any reason why the length is capped at 255 on TDS prior to 7? To me, and on Sybase ASE, it is the pagesize which dictates the maximum length of a row and, in turn, of a field. I think 255 was a limitation from very old versions of Sybase. Am I not getting the code correctly?

There also seems to be some confusion between a field's type and usertype. But again, I might misunderstand the code.

Thank you.

Benh

blieusong commented 3 years ago

OK I took the time to look at this. There are a few things in datacopy itself that I have a fix for. But that specific issue might come from the enum TDS_SERVER_TYPE which has two declarations for value 175

typedef enum
{
    ...
/*
 * MS only types
 */
        ...
    XSYBCHAR = 175,     /* 0xAF */
        ...
/*
 * Sybase only types
 */
        ...
    SYBLONGCHAR = 175,  /* 0xAF */
        ...
} TDS_SERVER_TYPE;

And on Sybase, for (var)char columns longer than 256 the server type becomes 175 (SYBLONGCHAR) instead of 47 (CHAR) or 39 (VARCHAR)

because of all this, tds_get_column_declaration doesn't work as expected.

switch (tds_get_conversion_type(curcol->on_server.column_type, curcol->on_server.column_size)) {
case XSYBCHAR:
case SYBCHAR:
    fmt = "CHAR(%u)";
    break;
...
}

XSYBCHAR happens to be 175 but for MSSQL Server, not for Sybase. So tds_get_column_declaration returns a CHAR(xxx) type that we don't want.

Working on a fix.

freddy77 commented 3 years ago

Hi, nice catch. Yes, Sybase supports up to 32K-1 characters in varchar. I suppose 32K-1 should be the limit for TDS5.

Regards, Frediano

Il giorno dom 4 apr 2021 alle ore 23:10 Benh LIEU SONG < @.***> ha scritto:

Hi,

On Sybase 15.7, datacopy fails to copy data from a table which has a VARCHAR(2500) field.

It recreates the table mostly correctly but the VARCHAR(2500) ends up as a CHAR(255).

When looking at the code, I'm a bit puzzled by that part:

TDSRETtds_get_column_declaration(TDSSOCKET tds, TDSCOLUMN curcol, char out) { const char fmt = NULL; / unsigned int is required by printf format, don't use size_t / unsigned int max_len = IS_TDS7_PLUS(tds->conn) ? 8000 : 255; ... }

Is there any reason why the length is capped at 255 on TDS prior to 7? To me, and on Sybase ASE, it is the pagesize which yields the maximum length of a row and, in turn, of a field. I think 255 was a limitation from very old versions of Sybase. Am I not getting the code correctly?

There also seems to be some confusion between a field's type and usertype. But again, I might misunderstand the code.

Thank you.

Benh

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/FreeTDS/freetds/issues/392, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHWP2HOD3VAOLYFMHP2OGDTHDPU5ANCNFSM42L2XL7A .

blieusong commented 3 years ago

Just to let know I'm still working on this. I just need time cuz, let's face it, I'm not so good with TDS and the whole FreeTDS codebase. Commit coming today or tomorrow (hopefully)

blieusong commented 3 years ago

Hi @freddy77,

Is there a possibility for me to have rights to create new branches at least? Or shall I keep pushing my changes to a fork of the repo? (Doesn't really matter, just asking, cause I think this may yield less hassles later)

Thanks.

fziglio commented 3 years ago

That's the normal way of doing. I have a separate (forked) repository too at https://github.com/freddy77/freetds. As I don't need to open PR (as currently I'm the only reviewer) I use an appveyor branch on this repository but if you open a PR it will be tested too so I don't see the problem.

Usually you have a working directory with 2 remotes, the origin, from which you clone and your one.

If you have question for the code feel free to ask.

blieusong commented 3 years ago

OK thanks for explaining. I will push my changes tonight. And yes, I probably will have tons of questions...

Regards,