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

Hi,

Patch looks good to me.

Thanks,
Khushboo

On Thu, Apr 16, 2020 at 12:07 PM navnath gadakh <
navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:

> 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
>