Обсуждение: BUG #14164: Postgres allow to insert more data into field than this field allow

VGhlIGZvbGxvd2luZyBidWcgaGFzIGJlZW4gbG9nZ2VkIG9uIHRoZSB3ZWJz
aXRlOgoKQnVnIHJlZmVyZW5jZTogICAgICAxNDE2NApMb2dnZWQgYnk6ICAg
ICAgICAgIEV1Z2VuIEtvbmtvdgpFbWFpbCBhZGRyZXNzOiAgICAgIGtlcy1r
ZXNAeWFuZGV4LnJ1ClBvc3RncmVTUUwgdmVyc2lvbjogOS41LjIKT3BlcmF0
aW5nIHN5c3RlbTogICB4ODZfNjQtcGMtbGludXgtZ251LCAoVWJ1bnR1IDQu
OC4yLTE5dWJ1bnR1MSkgNjRiaQpEZXNjcmlwdGlvbjogICAgICAgIAoKIyBc
ZCsgZmlsZXMyXzEwMjAwOTsgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBUYWJsZQoicHVibGljLmZpbGVzMl8xMDIw
MDkiDQogICBDb2x1bW4gICAgfCAgICAgICAgICAgIFR5cGUgICAgICAgICAg
ICAgfCAgICAgICAgICAgICAgIE1vZGlmaWVycyAgICAgICAgCiAgICAgICB8
IFN0b3JhZ2UgIHwgU3RhdHMgdGFyZ2V0IHwgRGVzY3JpcHRpb24gDQotLS0t
LS0tLS0tLS0tKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tKy0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0rLS0tLS0tLS0t
LSstLS0tLS0tLS0tLS0tLSstLS0tLS0tLS0tLS0tDQogZmlsZW5hbWUgICAg
fCBjaGFyYWN0ZXIgdmFyeWluZygyNTUpICAgICAgfCBkZWZhdWx0IE5VTEw6
OmNoYXJhY3RlciB2YXJ5aW5nCiAgICAgICB8IGV4dGVuZGVkIHwgICAgICAg
ICAgICAgIHwgDQoNCg0KIyBzZWxlY3QgZmlsZW5hbWUgZnJvbSBmaWxlczJf
MTAyMDA5IHdoZXJlIGlkID0gJzA1N2NiZDc1YzE2MGEzOGUnOyAgICAgICAg
IAogICAgICAgICAgICAgICBmaWxlbmFtZSAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgCiAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgDQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0NCgoyMC0lRDAlQTElRDAlQjIlRDAlQjglRDAlQkQlRDAlQkQlRDAl
QjAlRDElOEYlMjAlRDElODAlRDElODMlRDAlQkIlRDElOEMlRDAlQkElRDAl
QjAlMjglRDAlQTElRDAlQjIlRDAlQjglRDAlQkQlRDAlQjglRDAlQkQlRDAl
QjAlMkMlMjAlRDAlQkElRDAlQjUlRDAlQjQlRDElODAlRDAlQkUlRDAlQjIl
RDElOEIlRDAlQjklMjAlRDAlQkUlRDElODAlRDAlQjUlRDElODUlMkMlMjAl
RDElODElRDAlQkUlRDElODMlRDElODElMjk2MjAlRDElODAuanBnDQooMSBy
b3cpDQoNCkhvdyB0byByZXByb2R1Y2U6DQpXZSBuZWVkIG15c3FsIHNlcnZl
ciwgbXlzcWxfZmR3LCBwb3N0Z3JlczsNCg0KMSkgY3JlYXRlIG15c3FsIHRh
YmxlOg0KQ1JFQVRFIFRBQkxFIGBmaWxlczJgICgNCi4uLg0KYGZpbGVuYW1l
YCB0ZXh0DQopOw0KDQoyKSBjcmVhdGUgZXh0ZW5zaW9uIG15c3FsX2ZkdyBh
bmQgZm9yZWlnbiB0YWJsZToNCiMgXGQrIGZpbGVzMl9teXNxbA0KICBGb3Jl
aWduIHRhYmxlICJwdWJsaWMuZmlsZXMyX215c3FsIg0KICAgQ29sdW1uICAg
IHwgICAgICAgICAgICBUeXBlICAgICAgICAgICAgIHwgICAgICAgICAgICAg
ICBNb2RpZmllcnMgICAgICAgIAogICAgICAgfCBGRFcgT3B0aW9ucyB8IFN0
b3JhZ2UgIHwgU3RhdHMgdGFyZ2V0IHwgRGVzY3JpcHRpb24gDQouLi4NCiBm
aWxlbmFtZSAgICB8IGNoYXJhY3RlciB2YXJ5aW5nKDI1NSkgICAgICB8IGRl
ZmF1bHQgTlVMTDo6Y2hhcmFjdGVyIHZhcnlpbmcKICAgICAgIHwgICAgICAg
ICAgICAgfCBleHRlbmRlZCB8ICAgICAgICAgICAgICB8IA0KDQozKSBjcmVh
dGUgcG9zdGdyZXMgdGFibGU6DQojIFxkKyBmaWxlczJfMTAyMDA5OyAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFRh
YmxlCiJwdWJsaWMuZmlsZXMyXzEwMjAwOSINCiAgIENvbHVtbiAgICB8ICAg
ICAgICAgICAgVHlwZSAgICAgICAgICAgICB8ICAgICAgICAgICAgICAgTW9k
aWZpZXJzICAgICAgICAKICAgICAgIHwgU3RvcmFnZSAgfCBTdGF0cyB0YXJn
ZXQgfCBEZXNjcmlwdGlvbiANCi0tLS0tLS0tLS0tLS0rLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0rLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLSstLS0tLS0tLS0tKy0tLS0tLS0tLS0tLS0tKy0tLS0t
LS0tLS0tLS0NCiBmaWxlbmFtZSAgICB8IGNoYXJhY3RlciB2YXJ5aW5nKDI1
NSkgICAgICB8IGRlZmF1bHQgTlVMTDo6Y2hhcmFjdGVyIHZhcnlpbmcKICAg
ICAgIHwgZXh0ZW5kZWQgfCAgICAgICAgICAgICAgfCANCg0KDQo0KQ0KSU5T
RVJUIElOVE8gZmlsZXMyXzEwMjAwOSBTRUxFQ1QgKiBGUk9NIGZpbGVzMl9t
eXNxbDsNCg0KQmVjYXVzZSBvZiB0eXBlIG9mIGNvbHVtbiBvZiBzb3VyY2Ug
YW5kIGRlc3RpbmF0aW9uIHRhYmxlcyBhcmUgZXF1YWwKcG9zdGdyZXMgZG9l
cyBub3QgY2hlY2sgYWN0dWFsIGRhdGEgc28gdmVyeSBsb25nIGRhdGEgYXQg
dGV4dCBmaWVsZCBvZiBteXNxbApkYXRhYmFzZSBpcyBpbnNlcnRlZCBpbnRv
IGxpbWl0ZWQgZmllbGQgYXQgcG9zdGdyZXMgREIuDQoNCkVYUEVDVEVEOiBw
b3N0Z3JlcyBzaG91bGQgbm90IGFsbG93IHRvIGluc2VydCBpbnRvIGZpZWxk
IG1vcmUgZGF0YSB0aGFuCmZpZWxkIGFsbG93cy4KCg==

kes-kes@yandex.ru writes:
> Because of type of column of source and destination tables are equal
> postgres does not check actual data so very long data at text field of mysql
> database is inserted into limited field at postgres DB.

I would say this is a bug in mysql_fdw, not Postgres proper.  In general
it's the responsibility of an FDW to ensure that data it passes back
meets the constraints of the foreign table declaration.

            regards, tom lane

On Mon, May 30, 2016 at 1:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> kes-kes@yandex.ru writes:
> > Because of type of column of source and destination tables are equal
> > postgres does not check actual data so very long data at text field of
> mysql
> > database is inserted into limited field at postgres DB.
>
> I would say this is a bug in mysql_fdw, not Postgres proper.  In general
> it's the responsibility of an FDW to ensure that data it passes back
> meets the constraints of the foreign table declaration.
>

K>>=E2=80=8B
4)
=E2=80=8BK>>=E2=80=8B
INSERT INTO files2_102009 SELECT * FROM files2_mysql;

=E2=80=8BI'm surprised that the definition of the object in the FROM clause=
 affects
behavior in this manner...

For the uninitiated while we do allow text type definitions to include
length restrictions the underlying storage model is the same for any and
all text variations you can legally describe (char, varchar, text,
varchar(n)) though some special behaviors occur during input/output
depending upon the type name and optional attribute.

I'd be more inclined to play with this if it didn't require such a specific
combination of technologies to readily reproduce.

I'd at least like to understand the implications this would have on our
system if it is allowed to stand.  The specific situation I'm thinking of
is a table that didn't require a toast but is now faced with a text value
too large to fit onto a page.

At some point in this SELECT->INSERT process there has to be a recognition
that the data we are seeing is external and needs to be validated against
the rules and assumptions of PostgreSQL.  It's not our fault that mysql_fdw
is broken but it is our fault that we allowed its brokenness to result is
non-conforming persisted data.

Either the SELECT should fail because a PostgreSQL varchar(255) cannot hold
longer data (probably this) or the INSERT should fail so that at least the
impact of the broken varchar is limited to an active query and doesn't make
it onto disk.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, May 30, 2016 at 1:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I would say this is a bug in mysql_fdw, not Postgres proper.  In general
>> it's the responsibility of an FDW to ensure that data it passes back
>> meets the constraints of the foreign table declaration.

> At some point in this SELECT->INSERT process there has to be a recognition
> that the data we are seeing is external and needs to be validated against
> the rules and assumptions of PostgreSQL.  It's not our fault that mysql_fdw
> is broken but it is our fault that we allowed its brokenness to result is
> non-conforming persisted data.

> Either the SELECT should fail because a PostgreSQL varchar(255) cannot hold
> longer data (probably this) or the INSERT should fail so that at least the
> impact of the broken varchar is limited to an active query and doesn't make
> it onto disk.

Indeed, the SELECT should have failed, and it's mysql_fdw that should have
issued the error.  We cannot expect to protect users against every sort of
malfeasance or misfeasance that might occur in extension C code.  Consider
for example that the Datum mysql_fdw is handing back and claiming to be
varchar(255) might not be textual at all, or it might be in the wrong
encoding, etc etc.  It wouldn't even be possible for the core code to
detect some of those cases, and it certainly would impose a lot of
overhead to add checking that should be redundant.

(BTW, I checked this and verified that an equivalent case in postgres_fdw
does throw an error; so it's *possible* for an FDW to get this right.
mysql_fdw simply doesn't.)

            regards, tom lane

To my mind, postgres should not rely on third party code and write broken data to the disk.
I suppose this may override other records (even those the user have no access) which are stored close to this broken
oneif data in the 'text' field of mysql are large enough. 

And, yes, mysql_fdw, should also do checks for data constistency

30.05.2016, 20:10, "Tom Lane" <tgl@sss.pgh.pa.us>:
> kes-kes@yandex.ru writes:
>> šBecause of type of column of source and destination tables are equal
>> špostgres does not check actual data so very long data at text field of mysql
>> šdatabase is inserted into limited field at postgres DB.
>
> I would say this is a bug in mysql_fdw, not Postgres proper. In general
> it's the responsibility of an FDW to ensure that data it passes back
> meets the constraints of the foreign table declaration.
>
> ššššššššššššššššššššššššregards, tom lane

The convention here is to inline or bottom post.

30.05.2016, 20:10, "Tom Lane" <tgl@sss.pgh.pa.us>:
> > kes-kes@yandex.ru writes:
> >>  Because of type of column of source and destination tables are equal
> >>  postgres does not check actual data so very long data at text field o=
f
> mysql
> >>  database is inserted into limited field at postgres DB.
> >
> > I would say this is a bug in mysql_fdw, not Postgres proper. In general
> > it's the responsibility of an FDW to ensure that data it passes back
> > meets the constraints of the foreign table declaration.
>
>
=E2=80=8B
=E2=80=8B
On Mon, May 30, 2016 at 2:28 PM, KES <kes-kes@yandex.ru> wrote:

> To my mind, postgres should not rely on third party code and write broken
> data to the disk.
>

=E2=80=8B=E2=80=8BIt isn't quite this cut-and-dry.  The user takes some res=
ponsibility for
the stuff they load into their database.

https://www.postgresql.org/docs/9.6/static/sql-createforeigntable.html
"""
=E2=80=8BAlthough PostgreSQL does not attempt to enforce constraints on for=
eign
tables, it does assume that they are correct for purposes of query
optimization. If there are rows visible in the foreign table that do not
satisfy a declared constraint, queries on the table might produce incorrect
answers. It is the user's responsibility to ensure that the constraint
definition matches reality.
"""

While this isn't a table constraint it seems the same provision applies to
data types and their attributes.

I suspect that the only reason this example gets through whatever checks
are present is because of the fact that varchar(n) is just, basically, a
domain over text.

I suppose this may override other records (even those the user have no
> access) which are stored close to this broken one if data in the 'text'
> field of mysql are large enough.
>

=E2=80=8BJust because PostgreSQL doesn't validate that the length is within=
 the
constrained limit doesn't impact how it gets stored.  PostgreSQL will
measure the length as it writes the data and request the necessary amount
of free memory/space to hold it.  That is what I was =E2=80=8Btrying to poi=
nt out
with my brief description of the implementation of the text data type.

I'll add my last comment to Tom's most recent reply.

=E2=80=8BDavid J.=E2=80=8B

On Mon, May 30, 2016 at 3:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> "David G. Johnston" <david.g.johnston@gmail.com> writes:
> > On Mon, May 30, 2016 at 1:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I would say this is a bug in mysql_fdw, not Postgres proper.  In general
> >> it's the responsibility of an FDW to ensure that data it passes back
> >> meets the constraints of the foreign table declaration.
>
> > At some point in this SELECT->INSERT process there has to be a
> recognition
> > that the data we are seeing is external and needs to be validated against
> > the rules and assumptions of PostgreSQL.  It's not our fault that
> mysql_fdw
> > is broken but it is our fault that we allowed its brokenness to result is
> > non-conforming persisted data.
>
> > Either the SELECT should fail because a PostgreSQL varchar(255) cannot
> hold
> > longer data (probably this) or the INSERT should fail so that at least
> the
> > impact of the broken varchar is limited to an active query and doesn't
> make
> > it onto disk.
>
> Indeed, the SELECT should have failed, and it's mysql_fdw that should have
> issued the error.  We cannot expect to protect users against every sort of
> malfeasance or misfeasance that might occur in extension C code.


Ok, but do you really not want to handle (or at least explore) this
particular situation that will result in a dump-restore hazard - while
appearing as fully functioning in all other aspects.

I suspect that these records are not checked on COPY TO reading either but
that they are checked when going through COPY FROM - at which point they
will invoke an error.

Is there maybe some way to mark the datum coming from FDW as being dirty
which would preempt optimizations (or force some kind of cleaning) when
writing to a permanent table?  We could supply "cleaning" functions to
remove the dirty bit for the included types.

David J.