Thread: Re: [HACKERS] elog(FATAL)ing non-existent roles during client

Re: [HACKERS] elog(FATAL)ing non-existent roles during client

From
Gavin Sherry
Date:
On Tue, 5 Dec 2006, Gavin Sherry wrote:

> On Thu, 30 Nov 2006, Tom Lane wrote:
>
> > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > I wonder if we should check if the role exists for the other
> > > authentication methods too? get_role_line() should be very cheap and it
> > > would prevent unnecessary authentication work if we did it before
> > > contacting, for example, the client ident server. Even with trust, it
> > > would save work because otherwise we do not check if the user exists until
> > > InitializeSessionUserId(), at which time we're set up our proc entry etc.
> >
> > This only saves work if the supplied ID is in fact invalid, which one
> > would surely think isn't the normal case; otherwise it costs more.
>
> Yes.
>
> > I could see doing this in the ident path, because contacting a remote
> > ident server is certainly expensive on both sides.  I doubt it's a good
> > idea in the trust case.
>
> Agreed. How about Kerberos too, applying the same logic?

Attached is a patch check adds the checks.

Gavin

Attachment

Re: [HACKERS] elog(FATAL)ing non-existent roles during client

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Gavin Sherry wrote:
> On Tue, 5 Dec 2006, Gavin Sherry wrote:
>
> > On Thu, 30 Nov 2006, Tom Lane wrote:
> >
> > > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > > I wonder if we should check if the role exists for the other
> > > > authentication methods too? get_role_line() should be very cheap and it
> > > > would prevent unnecessary authentication work if we did it before
> > > > contacting, for example, the client ident server. Even with trust, it
> > > > would save work because otherwise we do not check if the user exists until
> > > > InitializeSessionUserId(), at which time we're set up our proc entry etc.
> > >
> > > This only saves work if the supplied ID is in fact invalid, which one
> > > would surely think isn't the normal case; otherwise it costs more.
> >
> > Yes.
> >
> > > I could see doing this in the ident path, because contacting a remote
> > > ident server is certainly expensive on both sides.  I doubt it's a good
> > > idea in the trust case.
> >
> > Agreed. How about Kerberos too, applying the same logic?
>
> Attached is a patch check adds the checks.
>
> Gavin
Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] elog(FATAL)ing non-existent roles during client

From
Bruce Momjian
Date:
Patch applied.  Thanks.

---------------------------------------------------------------------------


Gavin Sherry wrote:
> On Tue, 5 Dec 2006, Gavin Sherry wrote:
>
> > On Thu, 30 Nov 2006, Tom Lane wrote:
> >
> > > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > > I wonder if we should check if the role exists for the other
> > > > authentication methods too? get_role_line() should be very cheap and it
> > > > would prevent unnecessary authentication work if we did it before
> > > > contacting, for example, the client ident server. Even with trust, it
> > > > would save work because otherwise we do not check if the user exists until
> > > > InitializeSessionUserId(), at which time we're set up our proc entry etc.
> > >
> > > This only saves work if the supplied ID is in fact invalid, which one
> > > would surely think isn't the normal case; otherwise it costs more.
> >
> > Yes.
> >
> > > I could see doing this in the ident path, because contacting a remote
> > > ident server is certainly expensive on both sides.  I doubt it's a good
> > > idea in the trust case.
> >
> > Agreed. How about Kerberos too, applying the same logic?
>
> Attached is a patch check adds the checks.
>
> Gavin
Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +