Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Alvaro Herrera
Tom Lane escribió:
 Alvaro Herrera [EMAIL PROTECTED] writes:
  Brendan Jurd escribi�:
  Here's my attempt to remove the typename field from A_Const.  There
  were a few places (notably flatten_set_variable_args() in guc.c, and
  typenameTypeMod() in parse_type.c) where the code expected to see an
  A_Const with a typename, and I had to adjust for an A_Const within a
  TypeCast.  Nonetheless, there was an overall net reduction of 34 lines
  of code, so I think this was a win.
 
  Do say ... why don't we do away with A_Const altogether and just replace
  it with Value?  After this patch, I don't see what's the difference.
 
 They're logically different things, and after I get done putting a parse
 location field into A_Const, they'll still be physically different too.

Aha.  Are you working from Brendan's patch?  I was going to commit it.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes:
 Tom Lane escribió:
 They're logically different things, and after I get done putting a parse
 location field into A_Const, they'll still be physically different too.

 Aha.  Are you working from Brendan's patch?  I was going to commit it.

Sure, go ahead.  I was going to add the parse location at the same time,
but it can perfectly well be done as a separate patch.

regards, tom lane

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Alvaro Herrera
Tom Lane escribió:
 Alvaro Herrera [EMAIL PROTECTED] writes:
  Tom Lane escribió:
  They're logically different things, and after I get done putting a parse
  location field into A_Const, they'll still be physically different too.
 
  Aha.  Are you working from Brendan's patch?  I was going to commit it.
 
 Sure, go ahead.  I was going to add the parse location at the same time,
 but it can perfectly well be done as a separate patch.

I came up with the attached patch.  I added the location bits (although
I am unsure if I got the locations right in the parser), but they are
unused -- figuring out how to use them would take me longer than I can
to spend on this.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/nodes/copyfuncs.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.392
diff -c -p -r1.392 copyfuncs.c
*** src/backend/nodes/copyfuncs.c	14 Apr 2008 17:05:33 -	1.392
--- src/backend/nodes/copyfuncs.c	28 Apr 2008 20:20:44 -
*** _copyAConst(A_Const *from)
*** 1639,1645 
  			break;
  	}
  
! 	COPY_NODE_FIELD(typename);
  
  	return newnode;
  }
--- 1639,1645 
  			break;
  	}
  
! 	COPY_SCALAR_FIELD(location);
  
  	return newnode;
  }
Index: src/backend/nodes/equalfuncs.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.321
diff -c -p -r1.321 equalfuncs.c
*** src/backend/nodes/equalfuncs.c	14 Apr 2008 17:05:33 -	1.321
--- src/backend/nodes/equalfuncs.c	28 Apr 2008 16:17:02 -
*** _equalParamRef(ParamRef *a, ParamRef *b)
*** 1691,1701 
  static bool
  _equalAConst(A_Const *a, A_Const *b)
  {
! 	if (!equal(a-val, b-val))		/* hack for in-line Value field */
! 		return false;
! 	COMPARE_NODE_FIELD(typename);
! 
! 	return true;
  }
  
  static bool
--- 1691,1697 
  static bool
  _equalAConst(A_Const *a, A_Const *b)
  {
! 	return equal(a-val, b-val);		/* hack for in-line Value field */
  }
  
  static bool
Index: src/backend/nodes/outfuncs.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/nodes/outfuncs.c,v
retrieving revision 1.325
diff -c -p -r1.325 outfuncs.c
*** src/backend/nodes/outfuncs.c	13 Apr 2008 20:51:20 -	1.325
--- src/backend/nodes/outfuncs.c	28 Apr 2008 17:24:10 -
*** _outAConst(StringInfo str, A_Const *node
*** 1945,1951 
  
  	appendStringInfo(str,  :val );
  	_outValue(str, (node-val));
! 	WRITE_NODE_FIELD(typename);
  }
  
  static void
--- 1945,1951 
  
  	appendStringInfo(str,  :val );
  	_outValue(str, (node-val));
! 	WRITE_INT_FIELD(location);
  }
  
  static void
Index: src/backend/parser/gram.y
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.612
diff -c -p -r2.612 gram.y
*** src/backend/parser/gram.y	14 Apr 2008 17:05:33 -	2.612
--- src/backend/parser/gram.y	28 Apr 2008 19:48:16 -
*** static bool QueryIsRule = FALSE;
*** 91,101 
  
  static Node *makeColumnRef(char *relname, List *indirection, int location);
  static Node *makeTypeCast(Node *arg, TypeName *typename);
! static Node *makeStringConst(char *str, TypeName *typename);
! static Node *makeIntConst(int val);
! static Node *makeFloatConst(char *str);
! static Node *makeAConst(Value *v);
! static A_Const *makeBoolAConst(bool state);
  static FuncCall *makeOverlaps(List *largs, List *rargs, int location);
  static void check_qualified_name(List *names);
  static List *check_func_name(List *names);
--- 91,102 
  
  static Node *makeColumnRef(char *relname, List *indirection, int location);
  static Node *makeTypeCast(Node *arg, TypeName *typename);
! static Node *makeStringConst(char *str, int location);
! static Node *makeStringConstCast(char *str, TypeName *typename, int location);
! static Node *makeIntConst(int val, int location);
! static Node *makeFloatConst(char *str, int location);
! static Node *makeAConst(Value *v, int location);
! static Node *makeBoolAConst(bool state, int location);
  static FuncCall *makeOverlaps(List *largs, List *rargs, int location);
  static void check_qualified_name(List *names);
  static List *check_func_name(List *names);
*** set_rest:	/* Generic SET syntaxes: */
*** 1112,1118 
  	n-kind = VAR_SET_VALUE;
  	n-name = client_encoding;
  	if ($2 != NULL)
! 		n-args = list_make1(makeStringConst($2, NULL));
  	else
  		n-kind = VAR_SET_DEFAULT;
  	$$ = n;
--- 1113,1119 
  	n-kind = VAR_SET_VALUE;
  	n-name = client_encoding;
  	if ($2 != NULL)
! 		n-args = list_make1(makeStringConst($2, @2));
  	else
  		

Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Alvaro Herrera
Alvaro Herrera escribió:
 Tom Lane escribió:
  Alvaro Herrera [EMAIL PROTECTED] writes:
   Tom Lane escribió:
   They're logically different things, and after I get done putting a parse
   location field into A_Const, they'll still be physically different too.
  
   Aha.  Are you working from Brendan's patch?  I was going to commit it.
  
  Sure, go ahead.  I was going to add the parse location at the same time,
  but it can perfectly well be done as a separate patch.
 
 I came up with the attached patch.  I added the location bits (although
 I am unsure if I got the locations right in the parser), but they are
 unused -- figuring out how to use them would take me longer than I can
 to spend on this.

Hmm, I'm now wondering if the location should be added to Value as well,
so that it can be passed down to Const?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes:
 I came up with the attached patch.

I wasn't envisioning anything anywhere near this invasive.  We only
need locations on constants in a few contexts, I think.

BTW, you broke _equalAConst() ... it was a bad idea anyway to recast
it on the assumption that there would never again be more than one
field.

regards, tom lane

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Alvaro Herrera
Tom Lane escribió:
 Alvaro Herrera [EMAIL PROTECTED] writes:
  I came up with the attached patch.
 
 I wasn't envisioning anything anywhere near this invasive.  We only
 need locations on constants in a few contexts, I think.

Aha.  OK, I'll commit the original patch and let you deal with the rest
of it :-)

 BTW, you broke _equalAConst() ... it was a bad idea anyway to recast
 it on the assumption that there would never again be more than one
 field.

Oops, sorry, I had intended to revert that part and forgot.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [HACKERS] [PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])

2008-04-28 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes:
 Hmm, I'm now wondering if the location should be added to Value as well,
 so that it can be passed down to Const?

Just for the record, we don't want it in Const.  Parse locations are
only useful in the raw grammar output, mainly because they aren't
helpful unless you still have the original query string laying about.

regards, tom lane

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches