Re: [Patch] Refactor sql template version picking

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(at)pgadmin(dot)org> wrote:

> Good point. How about just versioned_loader?
>
> On Fri, Jan 27, 2017 at 4:40 PM, George Gelashvili
> <ggelashvili(at)pivotal(dot)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(at)pgadmin(dot)org> wrote:
> >>
> >> sql_loader ?
> >>
> >> On Fri, Jan 27, 2017 at 4:38 PM, George Gelashvili
> >> <ggelashvili(at)pivotal(dot)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(at)pgadmin(dot)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(at)pgadmin(dot)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(at)pivotal(dot)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(at)pivotal(dot)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(at)postgresql(dot)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
>