Re: [pgAdmin][RM5210] pgAdmin4 silently truncates text larger than underlying field size

From: navnath gadakh <navnath(dot)gadakh(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM5210] pgAdmin4 silently truncates text larger than underlying field size
Date: 2020-04-16 06:37:01
Message-ID: CAOAJCYrqU-nL3Z-q3H7KG+e9pVKb1YjKYaT3vWqPn1pormqQhw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Khushboo,

Please find the modified patch. I have removed the length from the data
types. Test cases also passing on all Postgres versions.
Thanks!

On Wed, Apr 15, 2020 at 6:22 PM Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
wrote:

>
>
> On Wed, 15 Apr 2020 at 18:21, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
> wrote:
>
>>
>>
>> On Wed, 15 Apr 2020 at 18:18, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Hello,
>>>
>>> We are sending the data to backend and depending on errors from backend. Any
>>> thoughts on implementation of basic fronted validations? so that we can
>>> alert user before it clicks on save button.
>>>
>> We should not try to implement the database functionality in JavaScript.
>> One of the important reason for the same, we would never be able to
>> understand each and every types supported by PG/PEM.
>>
> and - types from custom extensions.
> So - no to that.
>
> — Ashesh
>
>>
>> — Ashesh
>>
>>
>>>
>>> On Wed, 15 Apr 2020, 18:08 Dave Page, <dave(dot)page(at)enterprisedb(dot)com>
>>> wrote:
>>>
>>>> Removing the typecast will almost certainly lead to other problems. I
>>>> think we should just remove the length from it.
>>>>
>>>> On Wed, Apr 15, 2020 at 1:33 PM navnath gadakh <
>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> @Dave Page <dave(dot)page(at)enterprisedb(dot)com> @Akshay Joshi
>>>>> <akshay(dot)joshi(at)enterprisedb(dot)com> your input please?
>>>>>
>>>>> On Wed, Apr 15, 2020 at 3:13 PM Neel Patel <
>>>>> neel(dot)patel(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I think we should remove the type cast from query during update and
>>>>>> whatever error is thrown should be shown to UI as per scenario 3.
>>>>>>
>>>>>> Thanks,
>>>>>> Neel Patel
>>>>>>
>>>>>> On Wed, Apr 15, 2020 at 3:06 PM Khushboo Vashi <
>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Apr 15, 2020 at 2:48 PM navnath gadakh <
>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hello Hackers,
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Apr 14, 2020 at 5:14 PM Khushboo Vashi <
>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Navnath,
>>>>>>>>>
>>>>>>>>> You have compared the column's internal size with the length of
>>>>>>>>> the value given by the user.
>>>>>>>>> For example, column having integer would have internal size 4 and
>>>>>>>>> if I give the value 12121 which is the correct input for the field will
>>>>>>>>> fail here because as per your logic column internal size (4) < len(value)
>>>>>>>>> (5).
>>>>>>>>>
>>>>>>>>> I think this implementation is not correct here.
>>>>>>>>>
>>>>>>>> Yes, my implementations might be wrong.
>>>>>>>>
>>>>>>>> Below are some important findings on the parameterised query(as we
>>>>>>>> are using Jinja templates for building SQL queries).
>>>>>>>> Here I have created a table 'account' with some records in it.
>>>>>>>> CREATE TABLE public.account
>>>>>>>> (
>>>>>>>> user_id integer NOT NULL,
>>>>>>>> username character varying(5)
>>>>>>>> )
>>>>>>>>
>>>>>>>> psycopg2 throws a proper error if I pass username value greater
>>>>>>>> than the length of the data type(5)
>>>>>>>> Now, I want to pass username value greater than data type length (5)
>>>>>>>>
>>>>>>>> Scenario 1: Query with data type and length
>>>>>>>>
>>>>>>>> import psycopg2
>>>>>>>> try:
>>>>>>>> conn = psycopg2.connect("dbname='postgres' user='postgres' host='XXX.XXX.XXX.XXX' password='test' port=5432")
>>>>>>>> cur = conn.cursor()
>>>>>>>> cur.execute("UPDATE public.account SET username = %(username)s::character varying(5) WHERE user_id = 1;", {"username": "username-test-123"})
>>>>>>>> cur.execute("COMMIT;")
>>>>>>>> except Exception as e:
>>>>>>>> print('Exception : {0}'.format(e))
>>>>>>>>
>>>>>>>> *Output:*
>>>>>>>>
>>>>>>>> It will save the record with 5 char data without any error.
>>>>>>>>
>>>>>>>> *psql output:*
>>>>>>>>
>>>>>>>> postgres=# select * from public.account;
>>>>>>>> user_id | username
>>>>>>>> ---------+----------
>>>>>>>> 1 | usern
>>>>>>>> (1 row)
>>>>>>>>
>>>>>>>> Scenario 2: Query with only data type
>>>>>>>>
>>>>>>>> import psycopg2
>>>>>>>> try:
>>>>>>>> conn = psycopg2.connect("dbname='postgres' user='postgres' host='XXX.XXX.XXX.XXX' password='test' port=5432")
>>>>>>>> cur = conn.cursor()
>>>>>>>> cur.execute("UPDATE public.account SET username = %(username)s::character varying WHERE user_id = 1;", {"username": "username-test-123"})
>>>>>>>> cur.execute("COMMIT;")
>>>>>>>> except Exception as e:
>>>>>>>> print('Exception : {0}'.format(e))
>>>>>>>>
>>>>>>>> *Output:*
>>>>>>>>
>>>>>>>> Exception : value too long for type character varying(5)
>>>>>>>>
>>>>>>>> data will not save in the table.
>>>>>>>>
>>>>>>>> We can consider scenario 2 as it will throw the valid exception
>>>>>>> and also typecast the value in the proper format.
>>>>>>>
>>>>>>>> Scenario 3: Query without data type
>>>>>>>>
>>>>>>>> import psycopg2
>>>>>>>> try:
>>>>>>>> conn = psycopg2.connect("dbname='postgres' user='postgres' host='XXX.XXX.XXX.XXX' password='test' port=5432")
>>>>>>>> cur = conn.cursor()
>>>>>>>> cur.execute("UPDATE public.account SET username = %(username)s WHERE user_id = 1;", {"username": "username-test-123"})
>>>>>>>> cur.execute("COMMIT;")
>>>>>>>> except Exception as e:
>>>>>>>> print('Exception : {0}'.format(e))
>>>>>>>>
>>>>>>>> *Output:*
>>>>>>>>
>>>>>>>> Exception : value too long for type character varying(5)
>>>>>>>>
>>>>>>>> again data will not save in the table.
>>>>>>>>
>>>>>>>> These are some different behaviours with psycopg2. So to complete this patch which apporach should I follow? or any new approach is also welcome.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Khushboo
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Apr 14, 2020 at 4:33 PM navnath gadakh <
>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hello Hackers,
>>>>>>>>>> Please find the attached patch for below fixes:
>>>>>>>>>>
>>>>>>>>>> - Added validation for table row data that should not be larger
>>>>>>>>>> than the field size.
>>>>>>>>>> - Rearrange the existing functions to add validation.
>>>>>>>>>> - Added test cases.
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Navnath Gadakh
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Regards,
>>>>>>>> Navnath Gadakh
>>>>>>>>
>>>>>>>
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Navnath Gadakh
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> VP & Chief Architect, Database Infrastructure
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>> --
>>
>> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com>
>>
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi>
>>
> --
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> <http://www.enterprisedb.com>
>
>
> *http://www.linkedin.com/in/asheshvashi*
> <http://www.linkedin.com/in/asheshvashi>
>

--
Regards,
Navnath Gadakh

Attachment Content-Type Size
rm_5210_v2.patch application/octet-stream 6.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-04-16 07:49:18 pgAdmin 4 commit: Ensure that Constraint Trigger, Deferrable, Deferred
Previous Message Aditya Toshniwal 2020-04-16 06:03:13 [pgAdmin][RM5400] An unexpected occurred: INTERNAL SERVER ERROR message displayed if database server connected with non super user