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

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Dave Page <dave(dot)page(at)enterprisedb(dot)com>, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, navnath gadakh <navnath(dot)gadakh(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM5210] pgAdmin4 silently truncates text larger than underlying field size
Date: 2020-04-15 12:52:39
Message-ID: CAG7mmox5HvVfGNod2jjOthk2ZOc7xaLKWhz87RpL4-fUR_Q5eA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Neel Patel 2020-04-15 13:11:38 Re: RM4292 - Dark mode support for Windows/macOS
Previous Message Ashesh Vashi 2020-04-15 12:51:16 Re: [pgAdmin][RM5210] pgAdmin4 silently truncates text larger than underlying field size