Skip site navigation (1) Skip section navigation (2)

Re: %TYPE and array declaration patch review

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Greg Smith <greg(at)2ndquadrant(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Wojciech Muła <wojciech_mula(at)poczta(dot)onet(dot)pl>
Subject: Re: %TYPE and array declaration patch review
Date: 2011-11-30 15:42:32
Message-ID: CAFj8pRCU+tF5zhx=B1FEU4OLSTL6nbwo1PX9mmW2Oru8ujbu2A@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Hello

2011/11/28 Greg Smith <greg(at)2ndquadrant(dot)com>:
> I'm trying to find someone for the "[PL/pgSQL] %TYPE and array declaration -
> second patch" patch submitted recently:
>  https://commitfest.postgresql.org/action/patch_view?id=666
>
> Not too many people work on the PL/pgSQL code, and I see you reviewed an
> earlier version of this patch.  Do you think you could find time to review
> the update to it as well?
>

This patch is not applyed cleanly now

bash-4.2$ patch -p1 < type_array.patch
patching file doc/src/sgml/plpgsql.sgml
patching file src/pl/plpgsql/src/gram.y
Hunk #5 succeeded at 2540 (offset -12 lines).
Hunk #6 succeeded at 2554 (offset -12 lines).
Hunk #7 succeeded at 2595 (offset -12 lines).
patching file src/pl/plpgsql/src/pl_comp.c
Hunk #1 succeeded at 1586 (offset 2 lines).
Hunk #2 succeeded at 1883 (offset 2 lines).
Hunk #3 FAILED at 1901.
Hunk #4 succeeded at 2034 (offset 3 lines).
1 out of 4 hunks FAILED -- saving rejects to file
src/pl/plpgsql/src/pl_comp.c.rej
patching file src/pl/plpgsql/src/plpgsql.h
Hunk #2 succeeded at 895 (offset 8 lines).
patching file src/test/regress/expected/plpgsql.out
patching file src/test/regress/sql/plpgsql.sql


I dislike using macros without parameters

+#define word1 strVal(linitial(idents))
+#define word2 strVal(lsecond(idents))
+#define word3 strVal(lthird(idents))

and
-               nse = plpgsql_ns_lookup(plpgsql_ns_top(), false,
-
strVal(linitial(idents)),
-
strVal(lsecond(idents)),
-                                                               NULL,
+               var = (PLpgSQL_var *) plpgsql_get_variable2(
+                                                                       word1,
+                                                                       word2,
+
 PLPGSQL_NSTYPE_VAR,
+                                                                       NULL);

This change is useless, and smudges a code - a list operations are
well known and is not neccessary hide it.

macros

#define linitial_str(lc) strVal(linitial(lc))
#define lsecond_str(lc) strVal(lsecond(lc))
#define lthird_str(lc) strVal(lthird(lc))

these macros should be defined only once per module - #undef is not
used usually in pg source code, don't use it in this case

Regress tests are really large - it is question if about 900 lines is
necessary - should be more compact

Regards

Pavel



>

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2011-11-30 15:53:42
Subject: Re: review: CHECK FUNCTION statement
Previous:From: Albe LaurenzDate: 2011-11-30 15:23:16
Subject: Re: review: CHECK FUNCTION statement

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group