Re: [PATCHES] domain constraints and UNKNOWN params

2006-01-13 Thread David Wheeler

On Jan 11, 2006, at 9:28 PM, Neil Conway wrote:


Ah, right. Attached is a corrected patch.


I can confirm that this patch does indeed fix the issue for me on 8.1.2.

Thanks!

David

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] domain constraints and UNKNOWN params

2006-01-12 Thread Neil Conway
On Thu, 2006-01-12 at 00:28 -0500, Neil Conway wrote:
> Ah, right. Attached is a corrected patch.

Applied to HEAD, REL8_1_STABLE, REL8_0_STABLE, and REL7_4_STABLE.

-Neil



---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] domain constraints and UNKNOWN params

2006-01-11 Thread Tom Lane
Neil Conway <[EMAIL PROTECTED]> writes:
> On Wed, 2006-01-11 at 23:31 -0500, Tom Lane wrote:
>> I wonder whether there is any reasonably simple way to audit the whole
>> backend for missing domain processing...

> I don't really see a way to
> check the code that doesn't require a fair amount of manual auditing,

Me either.  I have a nagging feeling that the problem is really a
question of mis-factoring, ie we should be doing the domain checks
at a different level than now, but I don't yet see how to translate
that intuition into code.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] domain constraints and UNKNOWN params

2006-01-11 Thread Neil Conway
On Wed, 2006-01-11 at 23:31 -0500, Tom Lane wrote:
> This is a good catch, but the patch's added check on targetTyptype is a
> waste of code and cycles.  coerce_to_domain is perfectly capable of
> doing nothing when nothing is called for.

Ah, right. Attached is a corrected patch.

> I wonder whether there is any reasonably simple way to audit the whole
> backend for missing domain processing...

Yeah, I've been thinking along the same lines -- the recent spate of
issues doesn't give me a lot of confidence in the correctness or
completeness of the current implementation. I don't really see a way to
check the code that doesn't require a fair amount of manual auditing,
though...

-Neil


*** src/backend/parser/parse_coerce.c	8b4850b0ee25092c7c6166233049c4b48f05d443
--- src/backend/parser/parse_coerce.c	53b570c1de15dce8755d904b3a1643d85015c5d4
***
*** 243,249 
  		}
  
  		param->paramtype = targetTypeId;
! 		return (Node *) param;
  	}
  	if (find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
  			  &funcId))
--- 243,252 
  		}
  
  		param->paramtype = targetTypeId;
! 
! 		/* Apply domain constraints, if necessary */
! 		return coerce_to_domain((Node *) param, InvalidOid, targetTypeId,
! cformat, false, false);
  	}
  	if (find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
  			  &funcId))

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] domain constraints and UNKNOWN params

2006-01-11 Thread Tom Lane
Neil Conway <[EMAIL PROTECTED]> writes:
> I've also attached a patch that should fix the issue -- coerce_type()
> neglected to apply coerce_to_domain() to the type inferred for an
> UNKNOWN Param.

This is a good catch, but the patch's added check on targetTyptype is a
waste of code and cycles.  coerce_to_domain is perfectly capable of
doing nothing when nothing is called for.  (The reason the other paths
in the routine make such checks is that they can do so more or less for
free, thereby saving one catalog lookup in coerce_to_domain.  If it's
going to cost a catalog lookup anyway to find out if the type is a
domain, you might as well let coerce_to_domain do it.)

IOW, all you need to add is a coerce_to_domain call.  Compare a somewhat
related recent patch here:
http://archives.postgresql.org/pgsql-committers/2006-01/msg00125.php

> Barring any objections, I intend to apply the patch to
> HEAD and release branches as far back as the problem exists (likely 8.0
> and 8.1, and possibly 7.4 -- I haven't checked yet).

I think it's probably in 7.4 too.

I wonder whether there is any reasonably simple way to audit the whole
backend for missing domain processing...

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings