Thread: Re: [BUGS] Prepared Statement Name Truncation
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > NOTICE: identifier > "this_is_a_really_long_identifier_for_a_prepared_statement_name_ok" > will be truncated to > "this_is_a_really_long_identifier_for_a_prepared_statement_name_" > PREPARE ... > The ORM could use a shorter identifier, but it supports multiple backends > and this is probably not something in their test suite. In addition it > actually works! For now. If it really works, then by definition it does not /need/ to be that long, as the truncated version is not blowing things up. > So I am sharing this with the list to see what people think. Is this a > configuration bug? An ORM bug? A postgres bug? An unfortunate > interaction? Part ORM fault, part Postgres. We really should be throwing something stronger than a NOTICE on such a radical change to what the user asked for. I'd lobby for WARNING instead of ERROR, but either way, one could argue that applications would be more likely to notice and fix themselves if it was stronger than a NOTICE. > If it's a postgres bug, what is the fix? Make the identifier max size > longer? I'd also be in favor of this, in addition to upgrading from a NOTICE. We no longer have any technical reason to keep it NAMEDATALEN, with the listen/notify rewrite, correct? If so, I'd like to see the max bumped to at least 128 to match the default SQL spec length for similar items. > Set a hard limit and ERROR instead of truncating and NOTICE? > Both? Neither because that would break backward compatibility? My vote is WARNING and bump limit to 128 in 9.3. That's the combo most likely to make dumb applications work better while not breaking existing smart ones. - -- Greg Sabino Mullane greg@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201211172246 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAlCoWpYACgkQvJuQZxSWSsi4NwCfQfq7NEQ3xiLpPZLsu0I9iGT4 pOAAmgPEsm2iYCPiVfzMEM2EX2nihQE9 =wLpM -----END PGP SIGNATURE-----
On 18/11/12 16:49, Greg Sabino Mullane wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: RIPEMD160 > > >> NOTICE: identifier >> "this_is_a_really_long_identifier_for_a_prepared_statement_name_ok" >> will be truncated to >> "this_is_a_really_long_identifier_for_a_prepared_statement_name_" >> PREPARE > ... >> The ORM could use a shorter identifier, but it supports multiple backends >> and this is probably not something in their test suite. In addition it >> actually works! > For now. If it really works, then by definition it does not /need/ to > be that long, as the truncated version is not blowing things up. > >> So I am sharing this with the list to see what people think. Is this a >> configuration bug? An ORM bug? A postgres bug? An unfortunate >> interaction? > Part ORM fault, part Postgres. We really should be throwing something > stronger than a NOTICE on such a radical change to what the user > asked for. I'd lobby for WARNING instead of ERROR, but either way, one > could argue that applications would be more likely to notice and > fix themselves if it was stronger than a NOTICE. > >> If it's a postgres bug, what is the fix? Make the identifier max size >> longer? > I'd also be in favor of this, in addition to upgrading from a NOTICE. We > no longer have any technical reason to keep it NAMEDATALEN, with > the listen/notify rewrite, correct? If so, I'd like to see the max bumped > to at least 128 to match the default SQL spec length for similar items. > >> Set a hard limit and ERROR instead of truncating and NOTICE? >> Both? Neither because that would break backward compatibility? > My vote is WARNING and bump limit to 128 in 9.3. That's the combo most > likely to make dumb applications work better while not breaking > existing smart ones. > > > [...] > Would it be appropriate to make it a WARNING in 9.2.2, then increase the length in 9.3? Though I still feel I'd like it to be an ERROR, may be a configuration variable in 9.3 to promote it to an ERROR with WARNING being the default? Cheers, Gavin
On Nov 17, 2012 11:06 PM, "Gavin Flower" <GavinFlower@archidevsys.co.nz> wrote:
>
> On 18/11/12 16:49, Greg Sabino Mullane wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: RIPEMD160
>>
>>
>>> NOTICE: identifier
>>> "this_is_a_really_long_identifier_for_a_prepared_statement_name_ok"
>>> will be truncated to
>>> "this_is_a_really_long_identifier_for_a_prepared_statement_name_"
>>> PREPARE
>>
>> ...
>>>
>>> The ORM could use a shorter identifier, but it supports multiple backends
>>> and this is probably not something in their test suite. In addition it
>>> actually works!
>>
>> For now. If it really works, then by definition it does not /need/ to
>> be that long, as the truncated version is not blowing things up.
>>
>>> So I am sharing this with the list to see what people think. Is this a
>>> configuration bug? An ORM bug? A postgres bug? An unfortunate
>>> interaction?
>>
>> Part ORM fault, part Postgres. We really should be throwing something
>> stronger than a NOTICE on such a radical change to what the user
>> asked for. I'd lobby for WARNING instead of ERROR, but either way, one
>> could argue that applications would be more likely to notice and
>> fix themselves if it was stronger than a NOTICE.
>>
>>> If it's a postgres bug, what is the fix? Make the identifier max size
>>> longer?
>>
>> I'd also be in favor of this, in addition to upgrading from a NOTICE. We
>> no longer have any technical reason to keep it NAMEDATALEN, with
>> the listen/notify rewrite, correct? If so, I'd like to see the max bumped
>> to at least 128 to match the default SQL spec length for similar items.
>>
>>> Set a hard limit and ERROR instead of truncating and NOTICE?
>>> Both? Neither because that would break backward compatibility?
>>
>> My vote is WARNING and bump limit to 128 in 9.3. That's the combo most
>> likely to make dumb applications work better while not breaking
>> existing smart ones.
>>
>>
>> [...]
>>
> Would it be appropriate to make it a WARNING in 9.2.2, then increase the length in 9.3?
>
> Though I still feel I'd like it to be an ERROR, may be a configuration variable in 9.3 to promote it to an ERROR with WARNING being the default?
>
In that case I'd make it ERROR by default and make people override to WARNING if it breaks things. Otherwise no one will change.
>
> Cheers,
> Gavin
>
>
>
>
>
>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs
[...]On Nov 17, 2012 11:06 PM, "Gavin Flower" <GavinFlower@archidevsys.co.nz> wrote:
>
> On 18/11/12 16:49, Greg Sabino Mullane wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: RIPEMD160
>>
>>
>>> NOTICE: identifier
>>> "this_is_a_really_long_identifier_for_a_prepared_statement_name_ok"
>>> will be truncated to
>>> "this_is_a_really_long_identifier_for_a_prepared_statement_name_"
>>> PREPARE
>>
>> ...
>>>
>>> The ORM could use a shorter identifier, but it supports multiple backends
>>> and this is probably not something in their test suite. In addition it
>>> actually works!
>>
>> For now. If it really works, then by definition it does not /need/ to
>> be that long, as the truncated version is not blowing things up.
[...]
>>> Set a hard limit and ERROR instead of truncating and NOTICE?
>>> Both? Neither because that would break backward compatibility?
>>
>> My vote is WARNING and bump limit to 128 in 9.3. That's the combo most
>> likely to make dumb applications work better while not breaking
>> existing smart ones.
>>
>>
How about a WARNING in 9.2.2, and ERROR in 9.3 with a configuration option to downgrade it to WARNING - as well as increasing the max length to 128 to match the standard in 9.3 (I assume the size increase is to drastic for 9.2.x!)?> Would it be appropriate to make it a WARNING in 9.2.2, then increase the length in 9.3?
>
> Though I still feel I'd like it to be an ERROR, may be a configuration variable in 9.3 to promote it to an ERROR with WARNING being the default?
>In that case I'd make it ERROR by default and make people override to WARNING if it breaks things. Otherwise no one will change.
[...]
Cheers,
Gavin
"Greg Sabino Mullane" <greg@turnstep.com> writes:
>> If it's a postgres bug, what is the fix? Make the identifier max size
>> longer?
> I'd also be in favor of this, in addition to upgrading from a NOTICE.
Increasing NAMEDATALEN has been discussed, and rejected, before. It
is very very far from being a free change: it would double the storage
space required for "name" columns, which is a sizable fraction of the
space eaten in the pg_class and pg_attribute catalogs. (Or we could
convert "name" to a variable length type, but the fallout from that
would vastly exceed what this feature seems worth.)
I think there probably is some case for treating overlength names as
errors not warnings, but on the other hand there's a case for fearing
this will break applications that work fine today. In particular, it
would break applications that expect to be able to use spec-compliant
128-character names, *whether or not they actually have any collisions*.
That seems pretty undesirable, especially in view of the fact that
duplicate-name checks would catch any actual collisions in most cases.
Another point here is that appealing to the letter of the spec in this
area is a bit dubious anyway given the number of ways in which we vary
from exact spec compliance --- notably, allowing non-ASCII characters in
identifiers, allowing dollar signs, allowing leading underscore (no,
that's not per spec), folding to lower case not upper case.
On the whole I'm not too excited about changing this.
regards, tom lane
On Nov 18, 2012, at 2:24, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Greg Sabino Mullane" <greg@turnstep.com> writes: >>> If it's a postgres bug, what is the fix? Make the identifier max size >>> longer? > >> I'd also be in favor of this, in addition to upgrading from a NOTICE. > > On the whole I'm not too excited about changing this. > Then I'd agree with the OP and think the notice should go away on usage in DML; though it should be kept for DDL. Can the system be made smart enough to not allow intra-schema collisions in addition to same schema ones? That would seemto be the area of greatest concern - particularly around the usage of truncate/delete/drop. Thought: would there be some way to flag a table like this to always require the use of a schema prefix to be accessed (sinceright now truncated names only have to be schema unique) in certain conditions (drop, delete, truncate)? David J.
On Sunday, November 18, 2012 at 01:10, David Johnston wrote:
>
> Can the system be made smart enough to not allow intra-schema
> collisions in addition to same schema ones? That would seem to be the
> area of greatest concern - particularly around the usage of
> truncate/delete/drop.
>
>
My summary FWIW:
1. Potential exists for internally generated names to exceed maxlen; and
2. this maxlen is shorter than the SQL standard specification; but
3. it may not be worth the performance hit to be SQL compliant in this; with
4. potential for (undetected) name collision and unintended consequences.
May I suggest an idea from the days when memory was counted in (tiny int) kB:
represent the over maxlen identifiers "as is" up to maxlen-8 bytes
use those last 8 bytes for a 40bit hash in Base32 for disambiguation
and,
if 1:10^^12 residual collision risk is considered too high
a side list of overlong names would allow for a second hash disambiguation process.
Notes:
1. The choice of Base32 encoding may be a matter of personal preference
<http://en.wikipedia.org/wiki/Base32>, and, if so, I suggest using the
Crockford encoding <http://www.crockford.com/wrmg/base32.html>.
(I am impressed his design is excellent, while also averting some
accidental obscenities. None of the others offer this feature :)
2. Something along these lines, with the side table to track the
(hopefully) occasional overlong identifiers, could give standards
compliance in identifier length while still keeping the working
tables compact.
3. (Wild speculation) There may be a "sweet spot" using even shorter
identifiers than is the case now, with full disambiguation, which
might improve overall performance.
Regards
Gavan Schneider