Thread: [pgadmin-hackers][Patch] Refactor sql template version picking

[pgadmin-hackers][Patch] Refactor sql template version picking

From
George Gelashvili
Date:
Hi Hackers!

Since we are preparing to add greenplum support, we made a new template loader to automatically pick the available sql template file that corresponds to the postgres version number.

Our next patch will be to remove duplicated sql template files since many of the files are identical between versions.

Cheers,
George & Tira
Attachment

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
George Gelashvili
Date:
As a followup, we created this additional patch removing now-redundant sql templates. If you're interested in how we decided what files to delete, the script we used is here. We only removed files that were exactly identical between multiple postgres versions for the same feature.

This patch should be applied on a master patched with version-aware-sql-template-loader-refactor.diff


On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili <ggelashvili@pivotal.io> wrote:
Hi Hackers!

Since we are preparing to add greenplum support, we made a new template loader to automatically pick the available sql template file that corresponds to the postgres version number.

Our next patch will be to remove duplicated sql template files since many of the files are identical between versions.

Cheers,
George & Tira

Attachment

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Dave Page
Date:
Very nice indeed! I didn't realise we'd ended up with quite so many
duplicated templates.

Both patches look good to me - really the only thing that caught my
eye was the name versioned_template_loader which is somewhat longer
than I'd prefer.

As it's a major change, and we're going to be wrapping 1.2 in a little
over a week, I'd like some further review before committing. Khushboo,
can you take a look first thing on Monday please? If you're happy,
I'll commit and ask Fahar to do some testing.

Thanks!

On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
<ggelashvili@pivotal.io> wrote:
> As a followup, we created this additional patch removing now-redundant sql
> templates. If you're interested in how we decided what files to delete, the
> script we used is here. We only removed files that were exactly identical
> between multiple postgres versions for the same feature.
>
> This patch should be applied on a master patched with
> version-aware-sql-template-loader-refactor.diff
>
>
> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili <ggelashvili@pivotal.io>
> wrote:
>>
>> Hi Hackers!
>>
>> Since we are preparing to add greenplum support, we made a new template
>> loader to automatically pick the available sql template file that
>> corresponds to the postgres version number.
>>
>> Our next patch will be to remove duplicated sql template files since many
>> of the files are identical between versions.
>>
>> Cheers,
>> George & Tira
>
>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Dave Page
Date:
Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
being told Khushboo is on the critical path for something else at the
moment.

On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
> Very nice indeed! I didn't realise we'd ended up with quite so many
> duplicated templates.
>
> Both patches look good to me - really the only thing that caught my
> eye was the name versioned_template_loader which is somewhat longer
> than I'd prefer.
>
> As it's a major change, and we're going to be wrapping 1.2 in a little
> over a week, I'd like some further review before committing. Khushboo,
> can you take a look first thing on Monday please? If you're happy,
> I'll commit and ask Fahar to do some testing.
>
> Thanks!
>
> On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
> <ggelashvili@pivotal.io> wrote:
>> As a followup, we created this additional patch removing now-redundant sql
>> templates. If you're interested in how we decided what files to delete, the
>> script we used is here. We only removed files that were exactly identical
>> between multiple postgres versions for the same feature.
>>
>> This patch should be applied on a master patched with
>> version-aware-sql-template-loader-refactor.diff
>>
>>
>> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili <ggelashvili@pivotal.io>
>> wrote:
>>>
>>> Hi Hackers!
>>>
>>> Since we are preparing to add greenplum support, we made a new template
>>> loader to automatically pick the available sql template file that
>>> corresponds to the postgres version number.
>>>
>>> Our next patch will be to remove duplicated sql template files since many
>>> of the files are identical between versions.
>>>
>>> Cheers,
>>> George & Tira
>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
George Gelashvili
Date:
Thanks!
Did you have a name in mind? We're not sure we could come up with a clear name in fewer words.

On Fri, Jan 27, 2017 at 11:27 AM, Dave Page <dpage@pgadmin.org> wrote:
Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
being told Khushboo is on the critical path for something else at the
moment.

On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
> Very nice indeed! I didn't realise we'd ended up with quite so many
> duplicated templates.
>
> Both patches look good to me - really the only thing that caught my
> eye was the name versioned_template_loader which is somewhat longer
> than I'd prefer.
>
> As it's a major change, and we're going to be wrapping 1.2 in a little
> over a week, I'd like some further review before committing. Khushboo,
> can you take a look first thing on Monday please? If you're happy,
> I'll commit and ask Fahar to do some testing.
>
> Thanks!
>
> On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
> <ggelashvili@pivotal.io> wrote:
>> As a followup, we created this additional patch removing now-redundant sql
>> templates. If you're interested in how we decided what files to delete, the
>> script we used is here. We only removed files that were exactly identical
>> between multiple postgres versions for the same feature.
>>
>> This patch should be applied on a master patched with
>> version-aware-sql-template-loader-refactor.diff
>>
>>
>> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili <ggelashvili@pivotal.io>
>> wrote:
>>>
>>> Hi Hackers!
>>>
>>> Since we are preparing to add greenplum support, we made a new template
>>> loader to automatically pick the available sql template file that
>>> corresponds to the postgres version number.
>>>
>>> Our next patch will be to remove duplicated sql template files since many
>>> of the files are identical between versions.
>>>
>>> Cheers,
>>> George & Tira
>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Dave Page
Date:
sql_loader ?

On Fri, Jan 27, 2017 at 4:38 PM, George Gelashvili
<ggelashvili@pivotal.io> wrote:
> Thanks!
> Did you have a name in mind? We're not sure we could come up with a clear
> name in fewer words.
>
> On Fri, Jan 27, 2017 at 11:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>> being told Khushboo is on the critical path for something else at the
>> moment.
>>
>> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>> > Very nice indeed! I didn't realise we'd ended up with quite so many
>> > duplicated templates.
>> >
>> > Both patches look good to me - really the only thing that caught my
>> > eye was the name versioned_template_loader which is somewhat longer
>> > than I'd prefer.
>> >
>> > As it's a major change, and we're going to be wrapping 1.2 in a little
>> > over a week, I'd like some further review before committing. Khushboo,
>> > can you take a look first thing on Monday please? If you're happy,
>> > I'll commit and ask Fahar to do some testing.
>> >
>> > Thanks!
>> >
>> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>> > <ggelashvili@pivotal.io> wrote:
>> >> As a followup, we created this additional patch removing now-redundant
>> >> sql
>> >> templates. If you're interested in how we decided what files to delete,
>> >> the
>> >> script we used is here. We only removed files that were exactly
>> >> identical
>> >> between multiple postgres versions for the same feature.
>> >>
>> >> This patch should be applied on a master patched with
>> >> version-aware-sql-template-loader-refactor.diff
>> >>
>> >>
>> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>> >> <ggelashvili@pivotal.io>
>> >> wrote:
>> >>>
>> >>> Hi Hackers!
>> >>>
>> >>> Since we are preparing to add greenplum support, we made a new
>> >>> template
>> >>> loader to automatically pick the available sql template file that
>> >>> corresponds to the postgres version number.
>> >>>
>> >>> Our next patch will be to remove duplicated sql template files since
>> >>> many
>> >>> of the files are identical between versions.
>> >>>
>> >>> Cheers,
>> >>> George & Tira
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> >> To make changes to your subscription:
>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>> >>
>> >
>> >
>> >
>> > --
>> > Dave Page
>> > Blog: http://pgsnake.blogspot.com
>> > Twitter: @pgsnake
>> >
>> > EnterpriseDB UK: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
George Gelashvili
Date:
That would work, but the versioned template loader can load anything that is a template, as it extends from DispatchingJinjaLoader

On Fri, Jan 27, 2017 at 11:39 AM, Dave Page <dpage@pgadmin.org> wrote:
sql_loader ?

On Fri, Jan 27, 2017 at 4:38 PM, George Gelashvili
<ggelashvili@pivotal.io> wrote:
> Thanks!
> Did you have a name in mind? We're not sure we could come up with a clear
> name in fewer words.
>
> On Fri, Jan 27, 2017 at 11:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>> being told Khushboo is on the critical path for something else at the
>> moment.
>>
>> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>> > Very nice indeed! I didn't realise we'd ended up with quite so many
>> > duplicated templates.
>> >
>> > Both patches look good to me - really the only thing that caught my
>> > eye was the name versioned_template_loader which is somewhat longer
>> > than I'd prefer.
>> >
>> > As it's a major change, and we're going to be wrapping 1.2 in a little
>> > over a week, I'd like some further review before committing. Khushboo,
>> > can you take a look first thing on Monday please? If you're happy,
>> > I'll commit and ask Fahar to do some testing.
>> >
>> > Thanks!
>> >
>> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>> > <ggelashvili@pivotal.io> wrote:
>> >> As a followup, we created this additional patch removing now-redundant
>> >> sql
>> >> templates. If you're interested in how we decided what files to delete,
>> >> the
>> >> script we used is here. We only removed files that were exactly
>> >> identical
>> >> between multiple postgres versions for the same feature.
>> >>
>> >> This patch should be applied on a master patched with
>> >> version-aware-sql-template-loader-refactor.diff
>> >>
>> >>
>> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>> >> <ggelashvili@pivotal.io>
>> >> wrote:
>> >>>
>> >>> Hi Hackers!
>> >>>
>> >>> Since we are preparing to add greenplum support, we made a new
>> >>> template
>> >>> loader to automatically pick the available sql template file that
>> >>> corresponds to the postgres version number.
>> >>>
>> >>> Our next patch will be to remove duplicated sql template files since
>> >>> many
>> >>> of the files are identical between versions.
>> >>>
>> >>> Cheers,
>> >>> George & Tira
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> >> To make changes to your subscription:
>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>> >>
>> >
>> >
>> >
>> > --
>> > Dave Page
>> > Blog: http://pgsnake.blogspot.com
>> > Twitter: @pgsnake
>> >
>> > EnterpriseDB UK: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Dave Page
Date:
Good point. How about just versioned_loader?

On Fri, Jan 27, 2017 at 4:40 PM, George Gelashvili
<ggelashvili@pivotal.io> wrote:
> That would work, but the versioned template loader can load anything that is
> a template, as it extends from DispatchingJinjaLoader
>
> On Fri, Jan 27, 2017 at 11:39 AM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> sql_loader ?
>>
>> On Fri, Jan 27, 2017 at 4:38 PM, George Gelashvili
>> <ggelashvili@pivotal.io> wrote:
>> > Thanks!
>> > Did you have a name in mind? We're not sure we could come up with a
>> > clear
>> > name in fewer words.
>> >
>> > On Fri, Jan 27, 2017 at 11:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>> >> being told Khushboo is on the critical path for something else at the
>> >> moment.
>> >>
>> >> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >> > Very nice indeed! I didn't realise we'd ended up with quite so many
>> >> > duplicated templates.
>> >> >
>> >> > Both patches look good to me - really the only thing that caught my
>> >> > eye was the name versioned_template_loader which is somewhat longer
>> >> > than I'd prefer.
>> >> >
>> >> > As it's a major change, and we're going to be wrapping 1.2 in a
>> >> > little
>> >> > over a week, I'd like some further review before committing.
>> >> > Khushboo,
>> >> > can you take a look first thing on Monday please? If you're happy,
>> >> > I'll commit and ask Fahar to do some testing.
>> >> >
>> >> > Thanks!
>> >> >
>> >> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>> >> > <ggelashvili@pivotal.io> wrote:
>> >> >> As a followup, we created this additional patch removing
>> >> >> now-redundant
>> >> >> sql
>> >> >> templates. If you're interested in how we decided what files to
>> >> >> delete,
>> >> >> the
>> >> >> script we used is here. We only removed files that were exactly
>> >> >> identical
>> >> >> between multiple postgres versions for the same feature.
>> >> >>
>> >> >> This patch should be applied on a master patched with
>> >> >> version-aware-sql-template-loader-refactor.diff
>> >> >>
>> >> >>
>> >> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>> >> >> <ggelashvili@pivotal.io>
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi Hackers!
>> >> >>>
>> >> >>> Since we are preparing to add greenplum support, we made a new
>> >> >>> template
>> >> >>> loader to automatically pick the available sql template file that
>> >> >>> corresponds to the postgres version number.
>> >> >>>
>> >> >>> Our next patch will be to remove duplicated sql template files
>> >> >>> since
>> >> >>> many
>> >> >>> of the files are identical between versions.
>> >> >>>
>> >> >>> Cheers,
>> >> >>> George & Tira
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Sent via pgadmin-hackers mailing list
>> >> >> (pgadmin-hackers@postgresql.org)
>> >> >> To make changes to your subscription:
>> >> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Dave Page
>> >> > Blog: http://pgsnake.blogspot.com
>> >> > Twitter: @pgsnake
>> >> >
>> >> > EnterpriseDB UK: http://www.enterprisedb.com
>> >> > The Enterprise PostgreSQL Company
>> >>
>> >>
>> >>
>> >> --
>> >> Dave Page
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >>
>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
George Gelashvili
Date:
We thought of that one too. That sort of makes it sound like the loader is versioned rather than the template files. Also, it's unclear what it is a loader for without "template".

That said, do you think there is a better place for it to live? We stuck it under utils just because we didn't see anywhere else obvious.

On Fri, Jan 27, 2017 at 11:43 AM, Dave Page <dpage@pgadmin.org> wrote:
Good point. How about just versioned_loader?

On Fri, Jan 27, 2017 at 4:40 PM, George Gelashvili
<ggelashvili@pivotal.io> wrote:
> That would work, but the versioned template loader can load anything that is
> a template, as it extends from DispatchingJinjaLoader
>
> On Fri, Jan 27, 2017 at 11:39 AM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> sql_loader ?
>>
>> On Fri, Jan 27, 2017 at 4:38 PM, George Gelashvili
>> <ggelashvili@pivotal.io> wrote:
>> > Thanks!
>> > Did you have a name in mind? We're not sure we could come up with a
>> > clear
>> > name in fewer words.
>> >
>> > On Fri, Jan 27, 2017 at 11:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>> >> being told Khushboo is on the critical path for something else at the
>> >> moment.
>> >>
>> >> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >> > Very nice indeed! I didn't realise we'd ended up with quite so many
>> >> > duplicated templates.
>> >> >
>> >> > Both patches look good to me - really the only thing that caught my
>> >> > eye was the name versioned_template_loader which is somewhat longer
>> >> > than I'd prefer.
>> >> >
>> >> > As it's a major change, and we're going to be wrapping 1.2 in a
>> >> > little
>> >> > over a week, I'd like some further review before committing.
>> >> > Khushboo,
>> >> > can you take a look first thing on Monday please? If you're happy,
>> >> > I'll commit and ask Fahar to do some testing.
>> >> >
>> >> > Thanks!
>> >> >
>> >> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>> >> > <ggelashvili@pivotal.io> wrote:
>> >> >> As a followup, we created this additional patch removing
>> >> >> now-redundant
>> >> >> sql
>> >> >> templates. If you're interested in how we decided what files to
>> >> >> delete,
>> >> >> the
>> >> >> script we used is here. We only removed files that were exactly
>> >> >> identical
>> >> >> between multiple postgres versions for the same feature.
>> >> >>
>> >> >> This patch should be applied on a master patched with
>> >> >> version-aware-sql-template-loader-refactor.diff
>> >> >>
>> >> >>
>> >> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>> >> >> <ggelashvili@pivotal.io>
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi Hackers!
>> >> >>>
>> >> >>> Since we are preparing to add greenplum support, we made a new
>> >> >>> template
>> >> >>> loader to automatically pick the available sql template file that
>> >> >>> corresponds to the postgres version number.
>> >> >>>
>> >> >>> Our next patch will be to remove duplicated sql template files
>> >> >>> since
>> >> >>> many
>> >> >>> of the files are identical between versions.
>> >> >>>
>> >> >>> Cheers,
>> >> >>> George & Tira
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Sent via pgadmin-hackers mailing list
>> >> >> (pgadmin-hackers@postgresql.org)
>> >> >> To make changes to your subscription:
>> >> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Dave Page
>> >> > Blog: http://pgsnake.blogspot.com
>> >> > Twitter: @pgsnake
>> >> >
>> >> > EnterpriseDB UK: http://www.enterprisedb.com
>> >> > The Enterprise PostgreSQL Company
>> >>
>> >>
>> >>
>> >> --
>> >> Dave Page
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >>
>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Murtuza Zabuawala
Date:
Sure Dave, I will take a look.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Fri, Jan 27, 2017 at 9:57 PM, Dave Page <dpage@pgadmin.org> wrote:
Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
being told Khushboo is on the critical path for something else at the
moment.

On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
> Very nice indeed! I didn't realise we'd ended up with quite so many
> duplicated templates.
>
> Both patches look good to me - really the only thing that caught my
> eye was the name versioned_template_loader which is somewhat longer
> than I'd prefer.
>
> As it's a major change, and we're going to be wrapping 1.2 in a little
> over a week, I'd like some further review before committing. Khushboo,
> can you take a look first thing on Monday please? If you're happy,
> I'll commit and ask Fahar to do some testing.
>
> Thanks!
>
> On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
> <ggelashvili@pivotal.io> wrote:
>> As a followup, we created this additional patch removing now-redundant sql
>> templates. If you're interested in how we decided what files to delete, the
>> script we used is here. We only removed files that were exactly identical
>> between multiple postgres versions for the same feature.
>>
>> This patch should be applied on a master patched with
>> version-aware-sql-template-loader-refactor.diff
>>
>>
>> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili <ggelashvili@pivotal.io>
>> wrote:
>>>
>>> Hi Hackers!
>>>
>>> Since we are preparing to add greenplum support, we made a new template
>>> loader to automatically pick the available sql template file that
>>> corresponds to the postgres version number.
>>>
>>> Our next patch will be to remove duplicated sql template files since many
>>> of the files are identical between versions.
>>>
>>> Cheers,
>>> George & Tira
>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Murtuza Zabuawala
Date:
Hi Dave,

I found one typo as given below and apart from that, code is working as expected with new template loader,

  • web/pgadmin/browser/server_groups/servers/databases/schemas/__init__.py:241:        return 'ppas/#{0#}'.format(ver)

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Mon, Jan 30, 2017 at 10:56 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Sure Dave, I will take a look.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Fri, Jan 27, 2017 at 9:57 PM, Dave Page <dpage@pgadmin.org> wrote:
Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
being told Khushboo is on the critical path for something else at the
moment.

On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
> Very nice indeed! I didn't realise we'd ended up with quite so many
> duplicated templates.
>
> Both patches look good to me - really the only thing that caught my
> eye was the name versioned_template_loader which is somewhat longer
> than I'd prefer.
>
> As it's a major change, and we're going to be wrapping 1.2 in a little
> over a week, I'd like some further review before committing. Khushboo,
> can you take a look first thing on Monday please? If you're happy,
> I'll commit and ask Fahar to do some testing.
>
> Thanks!
>
> On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
> <ggelashvili@pivotal.io> wrote:
>> As a followup, we created this additional patch removing now-redundant sql
>> templates. If you're interested in how we decided what files to delete, the
>> script we used is here. We only removed files that were exactly identical
>> between multiple postgres versions for the same feature.
>>
>> This patch should be applied on a master patched with
>> version-aware-sql-template-loader-refactor.diff
>>
>>
>> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili <ggelashvili@pivotal.io>
>> wrote:
>>>
>>> Hi Hackers!
>>>
>>> Since we are preparing to add greenplum support, we made a new template
>>> loader to automatically pick the available sql template file that
>>> corresponds to the postgres version number.
>>>
>>> Our next patch will be to remove duplicated sql template files since many
>>> of the files are identical between versions.
>>>
>>> Cheers,
>>> George & Tira
>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Dave Page
Date:
Thanks Murtuza. Fixed and committed. George; I stuck with the original
filename for the loader - I couldn't come up with anything better.

Fahar; This is a fairly major change. Please test thoroughly on
different versions of PG and EPAS to ensure that the correct SQL
templates are being used. They're utilised for everything from
generating SQL to create new objects, getting object properties,
updating objects and more.

Thanks all!

On Mon, Jan 30, 2017 at 7:52 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> I found one typo as given below and apart from that, code is working as
> expected with new template loader,
>
> web/pgadmin/browser/server_groups/servers/databases/schemas/__init__.py:241:
> return 'ppas/#{0#}'.format(ver)
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Jan 30, 2017 at 10:56 AM, Murtuza Zabuawala
> <murtuza.zabuawala@enterprisedb.com> wrote:
>>
>> Sure Dave, I will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Fri, Jan 27, 2017 at 9:57 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>>> being told Khushboo is on the critical path for something else at the
>>> moment.
>>>
>>> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>>> > Very nice indeed! I didn't realise we'd ended up with quite so many
>>> > duplicated templates.
>>> >
>>> > Both patches look good to me - really the only thing that caught my
>>> > eye was the name versioned_template_loader which is somewhat longer
>>> > than I'd prefer.
>>> >
>>> > As it's a major change, and we're going to be wrapping 1.2 in a little
>>> > over a week, I'd like some further review before committing. Khushboo,
>>> > can you take a look first thing on Monday please? If you're happy,
>>> > I'll commit and ask Fahar to do some testing.
>>> >
>>> > Thanks!
>>> >
>>> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>>> > <ggelashvili@pivotal.io> wrote:
>>> >> As a followup, we created this additional patch removing now-redundant
>>> >> sql
>>> >> templates. If you're interested in how we decided what files to
>>> >> delete, the
>>> >> script we used is here. We only removed files that were exactly
>>> >> identical
>>> >> between multiple postgres versions for the same feature.
>>> >>
>>> >> This patch should be applied on a master patched with
>>> >> version-aware-sql-template-loader-refactor.diff
>>> >>
>>> >>
>>> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>>> >> <ggelashvili@pivotal.io>
>>> >> wrote:
>>> >>>
>>> >>> Hi Hackers!
>>> >>>
>>> >>> Since we are preparing to add greenplum support, we made a new
>>> >>> template
>>> >>> loader to automatically pick the available sql template file that
>>> >>> corresponds to the postgres version number.
>>> >>>
>>> >>> Our next patch will be to remove duplicated sql template files since
>>> >>> many
>>> >>> of the files are identical between versions.
>>> >>>
>>> >>> Cheers,
>>> >>> George & Tira
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> >> To make changes to your subscription:
>>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Dave Page
>>> > Blog: http://pgsnake.blogspot.com
>>> > Twitter: @pgsnake
>>> >
>>> > EnterpriseDB UK: http://www.enterprisedb.com
>>> > The Enterprise PostgreSQL Company
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Fahar Abbas
Date:
Sure Dave.

On Mon, Jan 30, 2017 at 4:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Murtuza. Fixed and committed. George; I stuck with the original
filename for the loader - I couldn't come up with anything better.

Fahar; This is a fairly major change. Please test thoroughly on
different versions of PG and EPAS to ensure that the correct SQL
templates are being used. They're utilised for everything from
generating SQL to create new objects, getting object properties,
updating objects and more.

Thanks all!

On Mon, Jan 30, 2017 at 7:52 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> I found one typo as given below and apart from that, code is working as
> expected with new template loader,
>
> web/pgadmin/browser/server_groups/servers/databases/schemas/__init__.py:241:
> return 'ppas/#{0#}'.format(ver)
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Jan 30, 2017 at 10:56 AM, Murtuza Zabuawala
> <murtuza.zabuawala@enterprisedb.com> wrote:
>>
>> Sure Dave, I will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Fri, Jan 27, 2017 at 9:57 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>>> being told Khushboo is on the critical path for something else at the
>>> moment.
>>>
>>> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>>> > Very nice indeed! I didn't realise we'd ended up with quite so many
>>> > duplicated templates.
>>> >
>>> > Both patches look good to me - really the only thing that caught my
>>> > eye was the name versioned_template_loader which is somewhat longer
>>> > than I'd prefer.
>>> >
>>> > As it's a major change, and we're going to be wrapping 1.2 in a little
>>> > over a week, I'd like some further review before committing. Khushboo,
>>> > can you take a look first thing on Monday please? If you're happy,
>>> > I'll commit and ask Fahar to do some testing.
>>> >
>>> > Thanks!
>>> >
>>> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>>> > <ggelashvili@pivotal.io> wrote:
>>> >> As a followup, we created this additional patch removing now-redundant
>>> >> sql
>>> >> templates. If you're interested in how we decided what files to
>>> >> delete, the
>>> >> script we used is here. We only removed files that were exactly
>>> >> identical
>>> >> between multiple postgres versions for the same feature.
>>> >>
>>> >> This patch should be applied on a master patched with
>>> >> version-aware-sql-template-loader-refactor.diff
>>> >>
>>> >>
>>> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>>> >> <ggelashvili@pivotal.io>
>>> >> wrote:
>>> >>>
>>> >>> Hi Hackers!
>>> >>>
>>> >>> Since we are preparing to add greenplum support, we made a new
>>> >>> template
>>> >>> loader to automatically pick the available sql template file that
>>> >>> corresponds to the postgres version number.
>>> >>>
>>> >>> Our next patch will be to remove duplicated sql template files since
>>> >>> many
>>> >>> of the files are identical between versions.
>>> >>>
>>> >>> Cheers,
>>> >>> George & Tira
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> >> To make changes to your subscription:
>>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Dave Page
>>> > Blog: http://pgsnake.blogspot.com
>>> > Twitter: @pgsnake
>>> >
>>> > EnterpriseDB UK: http://www.enterprisedb.com
>>> > The Enterprise PostgreSQL Company
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Syed Fahar Abbas
Quality Management Group

EnterpriseDB Corporation
Phone Office: +92-51-835-8874
Phone Direct: +92-51-8466803
Mobile: +92-333-5409707
Skype ID: syed.fahar.abbas
Website: www.enterprisedb.com

Re: [pgadmin-hackers][Patch] Refactor sql template version picking

From
Atira Odhner
Date:
Awesome. Thanks folks!

On Mon, Jan 30, 2017 at 6:35 AM, Fahar Abbas <fahar.abbas@enterprisedb.com> wrote:
Sure Dave.

On Mon, Jan 30, 2017 at 4:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Murtuza. Fixed and committed. George; I stuck with the original
filename for the loader - I couldn't come up with anything better.

Fahar; This is a fairly major change. Please test thoroughly on
different versions of PG and EPAS to ensure that the correct SQL
templates are being used. They're utilised for everything from
generating SQL to create new objects, getting object properties,
updating objects and more.

Thanks all!

On Mon, Jan 30, 2017 at 7:52 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> I found one typo as given below and apart from that, code is working as
> expected with new template loader,
>
> web/pgadmin/browser/server_groups/servers/databases/schemas/__init__.py:241:
> return 'ppas/#{0#}'.format(ver)
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Jan 30, 2017 at 10:56 AM, Murtuza Zabuawala
> <murtuza.zabuawala@enterprisedb.com> wrote:
>>
>> Sure Dave, I will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Fri, Jan 27, 2017 at 9:57 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm
>>> being told Khushboo is on the critical path for something else at the
>>> moment.
>>>
>>> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dpage@pgadmin.org> wrote:
>>> > Very nice indeed! I didn't realise we'd ended up with quite so many
>>> > duplicated templates.
>>> >
>>> > Both patches look good to me - really the only thing that caught my
>>> > eye was the name versioned_template_loader which is somewhat longer
>>> > than I'd prefer.
>>> >
>>> > As it's a major change, and we're going to be wrapping 1.2 in a little
>>> > over a week, I'd like some further review before committing. Khushboo,
>>> > can you take a look first thing on Monday please? If you're happy,
>>> > I'll commit and ask Fahar to do some testing.
>>> >
>>> > Thanks!
>>> >
>>> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili
>>> > <ggelashvili@pivotal.io> wrote:
>>> >> As a followup, we created this additional patch removing now-redundant
>>> >> sql
>>> >> templates. If you're interested in how we decided what files to
>>> >> delete, the
>>> >> script we used is here. We only removed files that were exactly
>>> >> identical
>>> >> between multiple postgres versions for the same feature.
>>> >>
>>> >> This patch should be applied on a master patched with
>>> >> version-aware-sql-template-loader-refactor.diff
>>> >>
>>> >>
>>> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili
>>> >> <ggelashvili@pivotal.io>
>>> >> wrote:
>>> >>>
>>> >>> Hi Hackers!
>>> >>>
>>> >>> Since we are preparing to add greenplum support, we made a new
>>> >>> template
>>> >>> loader to automatically pick the available sql template file that
>>> >>> corresponds to the postgres version number.
>>> >>>
>>> >>> Our next patch will be to remove duplicated sql template files since
>>> >>> many
>>> >>> of the files are identical between versions.
>>> >>>
>>> >>> Cheers,
>>> >>> George & Tira
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> >> To make changes to your subscription:
>>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Dave Page
>>> > Blog: http://pgsnake.blogspot.com
>>> > Twitter: @pgsnake
>>> >
>>> > EnterpriseDB UK: http://www.enterprisedb.com
>>> > The Enterprise PostgreSQL Company
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Syed Fahar Abbas
Quality Management Group

EnterpriseDB Corporation
Phone Office: +92-51-835-8874
Phone Direct: +92-51-8466803
Mobile: +92-333-5409707
Skype ID: syed.fahar.abbas
Website: www.enterprisedb.com