Re: [PATCHES] Suppress compiler warnings on mingw
On Fri, 14 Mar 2008, ITAGAKI Takahiro wrote: DWORD is an alias for 'unsigned long' in 32bit Windows. Do you know how it defined in 64bit Windows? sizeof(DWORD) is always 4, even on 64-bit windows. sizeof(long) is also always 4. If you want the unsigned integral type that is the same size as pointers, there are some MS-defined types you can use: DWORD_PTR, LONG_PTR, ULONG_PTR. Or use the standard size_t and ptrdiff_t. Postgres requires sizeof(long) = sizeof(void *) Which is why 64-bit windows is not supported. Hopefully at some point someone can remove this restriction, but it is likely to be a major undertaking... , but sizeof(DWORD) is always 4. I fear the formatter for long integer might be broken in 64bit platform. If we could expect C99 is always available, 'PRIu32' would be the best choice. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Worst Month of 1981 for Downhill Skiing: August. The lines are the shortest, though. -- Steve Rubenstein -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [DOCS] Partition: use triggers instead of rules
On Wed, 28 Nov 2007, Alvaro Herrera wrote: David Fetter wrote: Greg Sabino Mullane managed to contrive an example where RULEs might conceivably be the least-bad way to do this, that being a machine where no PLs may be installed. Perhaps this just means we should consider installing plpgsql by default. I have run into this myself, and a patch that I contributed (which made it in to 8.3) made it possible for a database owner to create trusted languages from templates in the default configuration. Which means that if an admin wants to prevent usage of the language, they can revoke the right to create it, but db owners still opt-in to any languages they want. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] patch adding new regexp functions
Jeremy Drake wrote:
On Thu, 22 Mar 2007, Tom Lane wrote:
I'd vote for making this new code look like the rest of it, to wit
hardwire the values.
Attached please find a patch which does this.
I just realized that the last patch removed all usage of fcinfo in the
setup_regexp_matches function, so this version of the patch also removes
it as a parameter to that function.
--
Think of it! With VLSI we can pack 100 ENIACs in 1 sq. cm.!Index: src/backend/utils/adt/regexp.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/backend/utils/adt/regexp.c,v
retrieving revision 1.70
diff -c -r1.70 regexp.c
*** src/backend/utils/adt/regexp.c 20 Mar 2007 05:44:59 - 1.70
--- src/backend/utils/adt/regexp.c 28 Mar 2007 18:57:28 -
***
*** 30,35
--- 30,36
#include postgres.h
#include access/heapam.h
+ #include catalog/pg_type.h
#include funcapi.h
#include regex/regex.h
#include utils/builtins.h
***
*** 95,106
size_toffset;
re_comp_flags flags;
-
- /* text type info */
- Oid param_type;
- int16 typlen;
- bool typbyval;
- char typalign;
} regexp_matches_ctx;
typedef struct regexp_split_ctx
--- 96,101
***
*** 119,126
static intnum_res = 0;/* # of cached re's */
static cached_re_str re_array[MAX_CACHED_RES];/* cached re's */
! static regexp_matches_ctx *setup_regexp_matches(FunctionCallInfo fcinfo,
!
text *orig_str, text *pattern,
text *flags);
static ArrayType *perform_regexp_matches(regexp_matches_ctx *matchctx);
--- 114,120
static intnum_res = 0;/* # of cached re's */
static cached_re_str re_array[MAX_CACHED_RES];/* cached re's */
! static regexp_matches_ctx *setup_regexp_matches(text *orig_str, text *pattern,
text *flags);
static ArrayType *perform_regexp_matches(regexp_matches_ctx *matchctx);
***
*** 760,767
oldcontext =
MemoryContextSwitchTo(funcctx-multi_call_memory_ctx);
/* be sure to copy the input string into the multi-call ctx */
! matchctx = setup_regexp_matches(fcinfo,
PG_GETARG_TEXT_P_COPY(0),
!
pattern, flags);
MemoryContextSwitchTo(oldcontext);
funcctx-user_fctx = (void *) matchctx;
--- 754,761
oldcontext =
MemoryContextSwitchTo(funcctx-multi_call_memory_ctx);
/* be sure to copy the input string into the multi-call ctx */
! matchctx = setup_regexp_matches(PG_GETARG_TEXT_P_COPY(0),
pattern,
!
flags);
MemoryContextSwitchTo(oldcontext);
funcctx-user_fctx = (void *) matchctx;
***
*** 822,828
}
static regexp_matches_ctx *
! setup_regexp_matches(FunctionCallInfo fcinfo, text *orig_str, text *pattern,
text *flags)
{
regexp_matches_ctx *matchctx = palloc(sizeof(regexp_matches_ctx));
--- 816,822
}
static regexp_matches_ctx *
! setup_regexp_matches(text *orig_str, text *pattern, text *flags)
{
regexp_matches_ctx *matchctx = palloc(sizeof(regexp_matches_ctx));
***
*** 835,845
matchctx-pmatch = palloc(sizeof(regmatch_t) *
(matchctx-cpattern-re_nsub + 1));
matchctx-offset = 0;
- /* get text type oid, too lazy to do it some other way */
- matchctx-param_type = get_fn_expr_argtype(fcinfo-flinfo, 0);
- get_typlenbyvalalign(matchctx-param_type, matchctx-typlen,
-matchctx-typbyval,
matchctx-typalign);
-
matchctx-wide_str = palloc(sizeof(pg_wchar) * (matchctx-orig_len +
1));
matchctx-wide_len = pg_mb2wchar_with_len(VARDATA(matchctx-orig_str),
matchctx-wide_str, matchctx-orig_len);
--- 829,834
***
*** 915,923
dims[0] = 1;
}
return construct_md_array(elems, nulls, ndims, dims, lbs,
! matchctx-param_type,
matchctx-typlen,
! matchctx-typbyval,
matchctx-typalign);
}
Datum
--- 904,912
dims[0] = 1;
}
+ /* XXX: this hardcodes
Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
On Mon, 26 Mar 2007, Tom Lane wrote: Jeremy Drake [EMAIL PROTECTED] writes: This version of the patch includes documentation changes. Please review... Applied with minor revisions --- in particular, I thought the initial owner of a language should be its creator, full stop, rather than the rather strange (and undocumented) behavior you had. The reason I did it like that was this email from you: http://archives.postgresql.org/pgsql-hackers/2007-01/msg01186.php Also you missed updating pg_dump. Indeed. Now, all I need is the 'tsearch2 in core' patch to go in, and 8.3 will solve almost all of my problems :) Then I just need to nag my hosting provider to upgrade once it is released. I have been refraining from nagging about them still running 8.1.3 in anticipation of this ;) -- Five is a sufficiently close approximation to infinity. -- Robert Firth ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] patch adding new regexp functions
On Mon, 26 Mar 2007, Bruce Momjian wrote: Tom Lane wrote: Or were you speaking to the question of whether to adjust the regexp patch to conform more nearly to the coding practices found elsewhere? I agree with that, but I thought there was already a submitted patch for it. Yes, regex patch adjustment, and I have not seen a patch which makes such adjustments. http://archives.postgresql.org/pgsql-patches/2007-03/msg00285.php -- Pope Goestheveezl was the shortest reigning pope in the history of the Church, reigning for two hours and six minutes on 1 April 1866. The white smoke had hardly faded into the blue of the Vatican skies before it dawned on the assembled multitudes in St. Peter's Square that his name had hilarious possibilities. The crowds fell about, helpless with laughter, singing Half a pound of tuppenny rice Half a pound of treacle That's the way the chimney smokes Pope Goestheveezl The square was finally cleared by armed carabineri with tears of laughter streaming down their faces. The event set a record for hilarious civic functions, smashing the previous record set when Baron Hans Neizant Bompzidaize was elected Landburgher of Koln in 1653. -- Mike Harding, The Armchair Anarchist's Almanac ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] patch adding new regexp functions
On Wed, 21 Mar 2007, Tom Lane wrote: Jeremy Drake [EMAIL PROTECTED] writes: BTW, should I be calling get_typlenbyvalalign on TEXTOID or are there macros for those also? By and large we tend to hard-wire those properties too, eg there are plenty of places that do things like this: /* XXX: this hardcodes assumptions about the regtype type */ result = construct_array(tmp_ary, num_params, REGTYPEOID, 4, true, 'i'); Some are better commented than others ... So, what action (if any) should be taken? Should all (or some) of these values be hardcoded, or should the current calls to determine them be left in place? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Fortune's Real-Life Courtroom Quote #19: Q: Doctor, how many autopsies have you performed on dead people? A: All my autopsies have been performed on dead people. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] patch adding new regexp functions
On Wed, 21 Mar 2007, Gregory Stark wrote: The only thing I would say is that this should maybe be a TextGetDatum() just for code hygiene. It should be exactly equivalent though: I could not find such a thing using ctags, nor TextPGetDatum(). I looked at PG_RETURN_TEXT_P and it just uses PG_RETURN_POINTER, which in turn uses PointerGetDatum. If there is such a thing, it is well camoflaged... -- If you think the problem is bad now, just wait until we've solved it. -- Arthur Kasspe ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] patch adding new regexp functions
On Thu, 22 Mar 2007, Tom Lane wrote:
I'd vote for making this new code look like the rest of it, to wit
hardwire the values.
Attached please find a patch which does this.
--
Although written many years ago, Lady Chatterley's Lover has just been
reissued by the Grove Press, and this pictorial account of the
day-to-day life of an English gamekeeper is full of considerable
interest to outdoor minded readers, as it contains many passages on
pheasant-raising, the apprehending of poachers, ways to control vermin,
and other chores and duties of the professional gamekeeper.
Unfortunately, one is obliged to wade through many pages of extraneous
material in order to discover and savour those sidelights on the
management of a midland shooting estate, and in this reviewer's opinion
the book cannot take the place of J. R. Miller's Practical
Gamekeeping.
-- Ed Zern, Field and Stream (Nov. 1959)Index: src/backend/utils/adt/regexp.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/backend/utils/adt/regexp.c,v
retrieving revision 1.70
diff -c -r1.70 regexp.c
*** src/backend/utils/adt/regexp.c 20 Mar 2007 05:44:59 - 1.70
--- src/backend/utils/adt/regexp.c 22 Mar 2007 05:17:15 -
***
*** 30,35
--- 30,36
#include postgres.h
#include access/heapam.h
+ #include catalog/pg_type.h
#include funcapi.h
#include regex/regex.h
#include utils/builtins.h
***
*** 95,106
size_toffset;
re_comp_flags flags;
-
- /* text type info */
- Oid param_type;
- int16 typlen;
- bool typbyval;
- char typalign;
} regexp_matches_ctx;
typedef struct regexp_split_ctx
--- 96,101
***
*** 835,845
matchctx-pmatch = palloc(sizeof(regmatch_t) *
(matchctx-cpattern-re_nsub + 1));
matchctx-offset = 0;
- /* get text type oid, too lazy to do it some other way */
- matchctx-param_type = get_fn_expr_argtype(fcinfo-flinfo, 0);
- get_typlenbyvalalign(matchctx-param_type, matchctx-typlen,
-matchctx-typbyval,
matchctx-typalign);
-
matchctx-wide_str = palloc(sizeof(pg_wchar) * (matchctx-orig_len +
1));
matchctx-wide_len = pg_mb2wchar_with_len(VARDATA(matchctx-orig_str),
matchctx-wide_str, matchctx-orig_len);
--- 830,835
***
*** 915,923
dims[0] = 1;
}
return construct_md_array(elems, nulls, ndims, dims, lbs,
! matchctx-param_type,
matchctx-typlen,
! matchctx-typbyval,
matchctx-typalign);
}
Datum
--- 905,913
dims[0] = 1;
}
+ /* XXX: this hardcodes assumptions about the text type */
return construct_md_array(elems, nulls, ndims, dims, lbs,
! TEXTOID, -1, false,
'i');
}
Datum
***
*** 976,991
{
ArrayBuildState *astate = NULL;
regexp_split_ctx*splitctx;
- Oid param_type;
int nitems;
splitctx = setup_regexp_split(PG_GETARG_TEXT_P(0),
PG_GETARG_TEXT_P(1),
PG_GETARG_TEXT_P_IF_EXISTS(2));
- /* get text type oid, too lazy to do it some other way */
- param_type = get_fn_expr_argtype(fcinfo-flinfo, 0);
-
for (nitems = 0; splitctx-offset splitctx-wide_len; nitems++)
{
if (nitems splitctx-wide_len)
--- 966,977
***
*** 995,1001
astate = accumArrayResult(astate,
get_next_split(splitctx),
false,
! param_type,
CurrentMemoryContext);
}
--- 981,987
astate = accumArrayResult(astate,
get_next_split(splitctx),
false,
! TEXTOID,
CurrentMemoryContext);
}
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] patch adding new regexp functions
On Thu, 22 Mar 2007, Tom Lane wrote: AFAIR, the reason there's no TextPGetDatum (and ditto for lots of other datatypes) is lack of obvious usefulness. A function dealing with a text * doesn't normally have reason to convert that to a Datum until it returns --- and at that point PG_RETURN_TEXT_P is the thing to use. Do you have a counterexample, or does this just suggest that the regexp function patch needs some refactoring? If you are asking why I have reason to convert text * to a Datum in cases other than PG_RETURN_TEXT_P, it is used for calling text_substr functions using DirectFunctionCallN. BTW, this usage of text_substr using PointerGetDatum was copied from the pre-existing textregexsubstr function. -- Malek's Law: Any simple idea will be worded in the most complicated way. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] patch adding new regexp functions
On Wed, 21 Mar 2007, Tom Lane wrote: Neil Conway [EMAIL PROTECTED] writes: * Comments like /* get text type oid, too lazy to do it some other way */ do not exactly inspire confidence :) Surely the code was just using the TEXTOID macro? If so, it does not require a comment. If not, it should be fixed ... Nope, it does get_fn_expr_argtype(fcinfo-flinfo, 0); The too lazy remark was that I thought there may be a better way (like the macro you mentioned) but did not go looking for it because I already had something that worked that I found in the manual. If you like, I can put together another patch that removes the param_type members from the structs and hardcodes TEXTOID in the proper places. BTW, should I be calling get_typlenbyvalalign on TEXTOID or are there macros for those also? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- We're only in it for the volume. -- Black Sabbath ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] cosmetic patch to large object regression test
On Sat, 3 Mar 2007, Bruce Momjian wrote: Patch applied. Thanks. Another usage of the old workaround for the flags was added with the application of the lo_truncate patch. This patch changes that one to be consistent with the others. --- Jeremy Drake wrote: Since I have now learned that it is possible to input values in hex in postgres, I submit this patch to clean up the ugly workaround I did in the largeobject regression test to try to input hex values. It does not change the functionality of the test at all, it just makes it more readable. In detail, before when I needed to write hex values, for example 0x2 | 0x4, for the flags to lo_open, I would write it: CAST((2 | 4) * 16^4 AS integer) Now, I write it: CAST(x'2' | x'4' AS integer) which is more like the C and other language consumers of the API, and also is more obvious what I am trying to do, IMHO. -- Real Programs don't use shared text. Otherwise, how can they use functions for scratch space after they are finished calling them? Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq -- One of the main causes of the fall of the Roman Empire was that, lacking zero, they had no way to indicate successful termination of their C programs. -- Robert FirthIndex: src/test/regress/input/largeobject.source === RCS file: /home/jeremyd/local/postgres/cvsuproot/pgsql/src/test/regress/input/largeobject.source,v retrieving revision 1.3 diff -c -r1.3 largeobject.source *** src/test/regress/input/largeobject.source 3 Mar 2007 20:17:25 - 1.3 --- src/test/regress/input/largeobject.source 3 Mar 2007 22:41:05 - *** *** 85,91 -- Test truncation. BEGIN; ! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST((2 | 4) * 16^4 AS integer)); SELECT lo_truncate(fd, 10) FROM lotest_stash_values; SELECT loread(fd, 15) FROM lotest_stash_values; --- 85,91 -- Test truncation. BEGIN; ! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'2' | x'4' AS integer)); SELECT lo_truncate(fd, 10) FROM lotest_stash_values; SELECT loread(fd, 15) FROM lotest_stash_values; Index: src/test/regress/output/largeobject.source === RCS file: /home/jeremyd/local/postgres/cvsuproot/pgsql/src/test/regress/output/largeobject.source,v retrieving revision 1.3 diff -c -r1.3 largeobject.source *** src/test/regress/output/largeobject.source 3 Mar 2007 20:17:25 - 1.3 --- src/test/regress/output/largeobject.source 3 Mar 2007 22:41:27 - *** *** 118,124 END; -- Test truncation. BEGIN; ! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST((2 | 4) * 16^4 AS integer)); SELECT lo_truncate(fd, 10) FROM lotest_stash_values; lo_truncate - --- 118,124 END; -- Test truncation. BEGIN; ! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'2' | x'4' AS integer)); SELECT lo_truncate(fd, 10) FROM lotest_stash_values; lo_truncate - ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] cosmetic patch to large object regression test
Since I have now learned that it is possible to input values in hex in
postgres, I submit this patch to clean up the ugly workaround I did in the
largeobject regression test to try to input hex values. It does not
change the functionality of the test at all, it just makes it more
readable.
In detail, before when I needed to write hex values, for example
0x2 | 0x4,
for the flags to lo_open, I would write it:
CAST((2 | 4) * 16^4 AS integer)
Now, I write it:
CAST(x'2' | x'4' AS integer)
which is more like the C and other language consumers of the API, and also
is more obvious what I am trying to do, IMHO.
--
Real Programs don't use shared text. Otherwise, how can they use
functions for scratch space after they are finished calling them?Index: src/test/regress/input/largeobject.source
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/test/regress/input/largeobject.source,v
retrieving revision 1.1
diff -c -r1.1 largeobject.source
*** src/test/regress/input/largeobject.source 20 Jan 2007 17:15:44 -
1.1
--- src/test/regress/input/largeobject.source 2 Mar 2007 21:35:32 -
***
*** 14,24
-- lo_open(lobjId oid, mode integer) returns integer
-- The mode parameter to lo_open uses two constants:
! -- INV_READ = 0x2 = 2 * 16^4
! -- INV_WRITE = 0x4 = 4 * 16^4
-- The return value is a file descriptor-like value which remains valid for
the
-- transaction.
! UPDATE lotest_stash_values SET fd = lo_open(loid, CAST((2 | 4) * 16^4 AS
integer));
-- loread/lowrite names are wonky, different from other functions which are
lo_*
-- lowrite(fd integer, data bytea) returns integer
--- 14,24
-- lo_open(lobjId oid, mode integer) returns integer
-- The mode parameter to lo_open uses two constants:
! -- INV_READ = 0x2
! -- INV_WRITE = 0x4
-- The return value is a file descriptor-like value which remains valid for
the
-- transaction.
! UPDATE lotest_stash_values SET fd = lo_open(loid, CAST(x'2' | x'4' AS
integer));
-- loread/lowrite names are wonky, different from other functions which are
lo_*
-- lowrite(fd integer, data bytea) returns integer
***
*** 55,61
-- Read out a portion
BEGIN;
! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST((2 | 4) * 16^4 AS
integer));
-- lo_lseek(fd integer, offset integer, whence integer) returns integer
-- offset is in bytes, whence is one of three values:
--- 55,61
-- Read out a portion
BEGIN;
! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'2' | x'4' AS
integer));
-- lo_lseek(fd integer, offset integer, whence integer) returns integer
-- offset is in bytes, whence is one of three values:
***
*** 92,98
INSERT INTO lotest_stash_values (loid) SELECT
lo_import('@abs_srcdir@/data/tenk.data');
BEGIN;
! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST((2 | 4) * 16^4 AS
integer));
-- with the default BLKSZ, LOBLKSZ = 2048, so this positions us for a block
-- edge case
--- 92,98
INSERT INTO lotest_stash_values (loid) SELECT
lo_import('@abs_srcdir@/data/tenk.data');
BEGIN;
! UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'2' | x'4' AS
integer));
-- with the default BLKSZ, LOBLKSZ = 2048, so this positions us for a block
-- edge case
Index: src/test/regress/output/largeobject.source
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/test/regress/output/largeobject.source,v
retrieving revision 1.1
diff -c -r1.1 largeobject.source
*** src/test/regress/output/largeobject.source 20 Jan 2007 17:15:44 -
1.1
--- src/test/regress/output/largeobject.source 2 Mar 2007 21:36:24 -
***
*** 11,21
BEGIN;
-- lo_open(lobjId oid, mode integer) returns integer
-- The mode parameter to lo_open uses two constants:
! -- INV_READ = 0x2 = 2 * 16^4
! -- INV_WRITE = 0x4 = 4 * 16^4
-- The return value is a file descriptor-like value which remains valid for
the
-- transaction.
! UPDATE lotest_stash_values SET fd = lo_open(loid, CAST((2 | 4) * 16^4 AS
integer));
-- loread/lowrite names are wonky, different from other functions which are
lo_*
-- lowrite(fd integer, data bytea) returns integer
-- the integer is the number of bytes written
--- 11,21
BEGIN;
-- lo_open(lobjId oid, mode integer) returns integer
-- The mode parameter to lo_open uses two constants:
! -- INV_READ = 0x2
! -- INV_WRITE = 0x4
-- The return value is a file descriptor-like value which remains valid for
the
-- transaction.
! UPDATE lotest_stash_values SET fd = lo_open(loid, CAST(x'2' | x'4' AS
integer));
-- loread/lowrite names are wonky, different from other functions which are
lo_*
-- lowrite(fd integer, data bytea) returns integer
--
Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
On Tue, 20 Feb 2007, Bruce Momjian wrote: The most recent version of this patch has been added. 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. Cool, I was going to bring this up again once the regexp patch got in. There is one thing in this patch I was not sure on, and that is in AlterLanguageOwner what should the second parameter of heap_close be? I have RowExclusiveLock in the patch, but I am not sure that is correct. It would be good if someone more knowledgeable about such things checked on this when applying it... The latest version of the patch is currently at http://momjian.us/mhonarc/patches/msg00014.html --- Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: I think that an ALTER LANGUAGE OWNER TO is the proper response to these things, and unless I hear otherwise I will attempt to add this to my patch. Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME TO for the owner, which I missed before. I would appreciate someone with more knowledge of the permissions infrastructure to take a look at it since I am fairly new to it and may not fully understand its intricacies. I have refactored the owner checking of languages in the same manner as it is for other owned objects. I have changed to using standard permissions error messages (aclcheck_error) for the language permissions errors. I consider this patch ready for review, assuming the permissions rules outlined by Tom Lane on -hackers are valid. For reference, here are the rules that this patch is intended to implement: On Wed, 24 Jan 2007, Tom Lane wrote: In detail, it'd look something like: * For an untrusted language: must be superuser to either create or use the language (no change from current rules). Ownership of the pg_language entry is really irrelevant, as is its ACL. * For a trusted language: * if pg_pltemplate.something is ON: either a superuser or the current DB's owner can CREATE the language. In either case the pg_language entry will be marked as owned by the DB owner (pg_database.datdba), which means that subsequently he (or a superuser) can grant or deny USAGE within his DB. * if pg_pltemplate.something is OFF: must be superuser to CREATE the language; subsequently it will be owned by you, so only you or another superuser can grant or deny USAGE (same behavior as currently). The only difference from this is, that when superuser is required, the owner of the language is not the superuser who created it, but BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the same behavior as currently took precedence. The current behavior in cvs is that languages have no owner, and for purposes where one would be needed it is assumed to be BOOTSTRAP_SUPERUSERID. Is this valid, or should I instead set the owner to GetUserId() in those cases? -- Academic politics is the most vicious and bitter form of politics, because the stakes are so low. -- Wallace Sayre Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 6: explain analyze is your friend -- A UNIX saleslady, Lenore, Enjoys work, but she likes the beach more. She found a good way To combine work and play: She sells C shells by the seashore. ---(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
Re: [PATCHES] patch adding new regexp functions
On Sun, 18 Feb 2007, Jeremy Drake wrote: On Sun, 18 Feb 2007, Peter Eisentraut wrote: regexp_split(string text, pattern text[, flags text]) returns setof text regexp_split_array(string text, pattern text[. flags text[, limit int]]) returns text[] Since you are not splitting an array but returning an array, I would think that regexp_split_to_array would be better, and the other should then be regexp_split_to_table. OK But why does the second one have a limit and the first one doesn't? I will rename the functions regexp_split_to_(table|array) and I will add an optional limit parameter to the regexp_split_to_table function, for consistency and to avoid ordering concerns with LIMIT. -- Sometimes I worry about being a success in a mediocre world. -- Lily Tomlin ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] patch adding new regexp functions
On Sat, 17 Feb 2007, Peter Eisentraut wrote:
Jeremy Drake wrote:
In case you haven't noticed, I am rather averse to making this return
text[] because it is much easier in my experience to use the results
when returned in SETOF rather than text[],
The primary use case I know for string splitting is parsing
comma/pipe/whatever separated fields into a row structure, and the way
I see it your API proposal makes that exceptionally difficult.
For this case see string_to_array:
http://developer.postgresql.org/pgdocs/postgres/functions-array.html
select string_to_array('a|b|c', '|');
string_to_array
-
{a,b,c}
(1 row)
I don't know what your use case is, though. All of this is missing
actual use cases.
The particular use case I had for this function was at a previous
employer, and I am not sure exactly how much detail is appropriate to
divulge. Basically, the project was doing some text processing inside of
postgres, and getting all of the words from a string into a table with
some processing (excluding stopwords and so forth) as efficiently as
possible was a big concern.
The regexp_split function code was based on some code that a friend of
mine wrote which used PCRE rather than postgres' internal regexp support.
I don't know exactly what his use-case was, but he probably had
one because he wrote the function and had it returning SETOF text ;)
Perhaps he can share a general idea of what it was (nudge nudge)?
While, if you
really really wanted a text[], you could use the (fully documented)
ARRAY(select resultstr from regexp_split(...) order by startpos)
construct.
I think, however, that we should be providing simple primitives that can
be combined into complex expressions rather than complex primitives
that have to be dissected apart to get simple results.
The most simple primitive is string_to_array(text, text) returns text[],
but it was not sufficient for our needs.
As for the regexp_matches() function, it seems to me that it
returns too much information at once. What is the use case for
getting all of prematch, fullmatch, matches, and postmatch in one
call?
It was requested by David Fetter:
http://archives.postgresql.org/pgsql-hackers/2007-02/msg00056.php
It was not horribly difficult to provide, and it seemed reasonable to
me. I have no need for them personally.
David Fetter has also repeated failed to offer a use case for this, so I
hesitate to accept this.
I have no strong opinion either way, so I will let those who do argue it
out and wait for the dust to settle ;)
--
The Law, in its majestic equality, forbids the rich, as well as the
poor, to sleep under the bridges, to beg in the streets, and to steal
bread.
-- Anatole France
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] patch adding new regexp functions
Finally the voice of reason :) On Sat, 17 Feb 2007, Tom Lane wrote: So I'd vote against complicating the API in order to make special provision for these results. I claim that all we need is a function taking (string text, pattern text, flags text) and returning either array of text or setof text For this function, it would be setof array of text, as the capture groups would definitely go in an array, but if you asked for global in the flags, there could be more than one match in the string. containing the matched substrings in whatever order is standard (order by left-parenthesis position, I think). In the degenerate case where there are no parenthesized subpatterns, just return the whole match as a single element. Good idea, that did not occur to me. I was planning to throw an error in that case. As for the argument about array vs setof, I could see doing both to end the argument of which one is really superior for any particular problem. The array vs setof argument was on the split function. I will work on doing both. Any idea how the name and/or arguments should differ? I think that an optional limit argument for the array version like perl has would be reasonable, but a name difference in the functions is probably necessary to avoid confusion. -- Speaking of Godzilla and other things that convey horror: With a purposeful grimace and a Mongo-like flair He throws the spinning disk drives in the air! And he picks up a Vax and he throws it back down As he wades through the lab making terrible sounds! Helpless users with projects due Scream My God! as he stomps on the tape drives, too! Oh, no! He says Unix runs too slow! Go, go, DECzilla! Oh, yes! He's gonna bring up VMS! Go, go, DECzilla! * VMS is a trademark of Digital Equipment Corporation * DECzilla is a trademark of Hollow Chocolate Bunnies of Death, Inc. -- Curtis Jackson ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] patch adding new regexp functions
On Sat, 17 Feb 2007, Tom Lane wrote:
Jeremy Drake [EMAIL PROTECTED] writes:
On Sat, 17 Feb 2007, Tom Lane wrote:
So I'd vote against complicating the API in order to make special
provision for these results. I claim that all we need is a function
taking (string text, pattern text, flags text) and returning either
array of text or setof text
For this function, it would be setof array of text, as the capture groups
would definitely go in an array, but if you asked for global in the flags,
there could be more than one match in the string.
Oh, right. And you could do a 2-D array if you wanted it all in one
blob (or a guarantee of order).
I don't think that there is much of an argument for having this one return
a 2d array, in this particular case, even perl requires you to build a
loop, IIRC.
my $str = 'foobarbecuebazilbarf';
while($str=~/(b[^b]+)(b[^b]+)/g) {
print $1, \t, length($1), \n;
print $2, \t, length($2), \n;
print ---\n;
}
bar 3
becue 5
---
bazil 5
barf4
---
So no need for record-returning functions?
No.
--
Go placidly amid the noise and waste, and remember what value there may
be in owning a piece thereof.
-- National Lampoon, Deteriorata
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] patch adding new regexp functions
On Fri, 16 Feb 2007, Peter Eisentraut wrote: Am Freitag, 16. Februar 2007 08:02 schrieb Jeremy Drake: Does this version sufficiently address your concerns? I don't think anyone asked for the start position and length in the result of regexp_split(). The result should be an array of text. That is what Perl et al. do. The length is not returned, I simply call length() on the string result to make sure that no whitespace gets in. The start position was suggested in these two messages from Alvaro Herrera: http://archives.postgresql.org/pgsql-patches/2007-02/msg00276.php http://archives.postgresql.org/pgsql-patches/2007-02/msg00281.php This was meant to address your concern about the order not necessarily being preserved of the split results when being returned as SETOF. This gives the benefits of returning SETOF while still allowing the order to be preserved if desired (simply add ORDER BY startpos to guarantee the correct order). In case you haven't noticed, I am rather averse to making this return text[] because it is much easier in my experience to use the results when returned in SETOF rather than text[], and in all of the code that I have experience with where this would be useful I would end up using information_schema._pg_expandarray (a function that, AFAIK, is documented nowhere) to convert it into SETOF text. While, if you really really wanted a text[], you could use the (fully documented) ARRAY(select resultstr from regexp_split(...) order by startpos) construct. As for the regexp_matches() function, it seems to me that it returns too much information at once. What is the use case for getting all of prematch, fullmatch, matches, and postmatch in one call? It was requested by David Fetter: http://archives.postgresql.org/pgsql-hackers/2007-02/msg00056.php It was not horribly difficult to provide, and it seemed reasonable to me. I have no need for them personally. -- Some people in this department wouldn't recognize subtlety if it hit them on the head. ---(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
Re: [PATCHES] patch adding new regexp functions
On Thu, 15 Feb 2007, Peter Eisentraut wrote:
Neil Conway wrote:
On Wed, 2007-02-14 at 16:49 -0800, Jeremy Drake wrote:
What was the status of this? Was there anything else I needed to
do with this patch, or is it ready to be applied? Let me know if
there is anything else I need to do on this...
Will do -- I'm planning to apply this as soon as I have the free
cycles to do so, likely tomorrow or Friday.
I don't know which patch is actually being proposed now. It would be
good to make this more explicit and maybe include a synopsis of the
functions in the email, so we know what's going on.
Sorry, my intent was just to check to see if I had gotten the patch
sufficiently fixed for Neil to apply and he just hadn't gotten to it yet
(which seems to be the case), or if there was something else he still
expected me to fix that I had missed in the prior discussions. I suppose
I should have emailed him privately.
The patch in question can be seen in the archives here:
http://archives.postgresql.org/pgsql-patches/2007-02/msg00214.php
The functions added are:
* regexp_split(str text, pattern text) RETURNS SETOF text
regexp_split(str text, pattern text, flags text) RETURNS SETOF text
returns each section of the string delimited by the pattern.
* regexp_matches(str text, pattern text) RETURNS text[]
returns all capture groups when matching pattern against string in an
array
* regexp_matches(str text, pattern text, flags text) RETURNS SETOF
(prematch text, fullmatch text, matches text[], postmatch text)
returns all capture groups when matching pattern against string in an
array. also returns the entire match in fullmatch. if the 'g' option
is given, returns all matches in the string. if the 'r' option is
given, also return the text before and after the match in prematch and
postmatch respectively.
What confuses me about some of the functions I've seen in earlier
patches in this thread is that they return setof something. But in my
mind, regular expression matches or string splits are inherently
ordered, so an array would be the correct return type.
They do return SETOF. Addressing them separately:
regexp_matches uses a text[] for the match groups. If you specify the
global flag, it could return multiple matches. Couple this with the
requested feature of pre- and postmatch returns (with its own flag) and
the return would turn into some sort of nasty array of tuples, or multiple
arrays. It seems much cleaner to me to return a set of the matches found,
and I find which order the matches occur in to be much less important than
the fact that they did occur and their contents.
regexp_split returns setof text. This has, in my opinion, a much greater
case to return an array. However, there are several issues with this
approach:
# My experience with the array code leads me to believe that building up
an array is an expensive proposition. I know I could code it smarter so
that the array is only constructed in the end.
# With a set-returning function, it is possible to add a LIMIT clause, to
prevent splitting up more of the string than is necessary. It is also
immediately possible to insert them into a table, or do grouping on them,
or call a function on each value. Most of the time when I do a split, I
intend to do something like this with each split value.
# You can still get an array if you really want it:
#* SELECT ARRAY(SELECT * FROM regexp_split('first, second, third', E',\\s*'))
--
No problem is so formidable that you can't just walk away from it.
-- C. Schulz
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [PATCHES] patch adding new regexp functions
On Thu, 15 Feb 2007, Alvaro Herrera wrote: Jeremy Drake wrote: The functions added are: * regexp_split(str text, pattern text) RETURNS SETOF text regexp_split(str text, pattern text, flags text) RETURNS SETOF text returns each section of the string delimited by the pattern. * regexp_matches(str text, pattern text) RETURNS text[] returns all capture groups when matching pattern against string in an array * regexp_matches(str text, pattern text, flags text) RETURNS SETOF (prematch text, fullmatch text, matches text[], postmatch text) returns all capture groups when matching pattern against string in an array. also returns the entire match in fullmatch. if the 'g' option is given, returns all matches in the string. if the 'r' option is given, also return the text before and after the match in prematch and postmatch respectively. I think the position the match is in could be important. I'm wondering if you could define them like create type re_match(match text, position int) regexp_split(str text, pattern text) returns setof re_match So it looks like the issues are: 1. regexp_matches without flags has a different return type than regexp_matches with flags. I can make them return the same OUT parameters, but should I declare it as returning SETOF when I know for a fact that the no-flags version will never return more than one row? 2. regexp_split does not represent the order of the results. I can do something like: regexp_split(str text, pattern text[, flags text], OUT result text, OUT startpos int) RETURNS SETOF record; It could also have the int being a simple counter to represent the relative order, rather than the position. Thoughts? Do these changes address the issues recently expressed? -- I have yet to see any problem, however complicated, which, when looked at in the right way, did not become still more complicated. -- Poul Anderson ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] patch adding new regexp functions
What was the status of this? Was there anything else I needed to do with this patch, or is it ready to be applied? Let me know if there is anything else I need to do on this... -- What this country needs is a good five cent nickel. ---(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
Re: [PATCHES] psql \lo_* quiet mode patch
On Tue, 13 Feb 2007, Bruce Momjian wrote: 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. This one has also already been applied. --- Jeremy Drake wrote: I sent this in a while back, but never heard anything about it. This patch makes psql's \lo_* commands respect the -q flag (or other methods of setting quiet mode) as well as HTML output mode. This came in very handy when writing a regression test which uses the \lo_import command since it would otherwise output the OID of the new large object which would be different every run. Please let me know if it is ok, or if I need to do it differently. -- Let me assure you that to us here at First National, you're not just a number. You're two numbers, a dash, three more numbers, another dash and another number. -- James Estes Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq -- Computers can figure out all kinds of problems, except the things in the world that just don't add up. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] large object regression tests, take two
On Thu, 8 Feb 2007, Alvaro Herrera wrote: Bruce Momjian wrote: Is this patch ready for application? It already has been. I think the sed usage would cause problems for the VC++ builds. It would be good if the files could be reformulated as files in the input, output or data directories; pg_regress would take care to generate the files as needed. It Just Worked with the changes made to pg-regress to support the other, similar tests (ie, copy). --- Jeremy Drake wrote: This is the latest version of the large object regression test I have been working on. Note that a prerequisite for this version of the test is the patch I made to psql to make it not output on \lo_* commands in quiet mode is required (also attached, it's small). Sorry that it still makes use of the sed trickery like the copy test does, but: 1) I think the server side lo_(import|export) functions need to be tested as well as the psql variants 2) ISTM that making assumptions about the working directory of psql during the regression tests would open a can of worms, especially wrt VPATH builds where the data files could be in a completely separate tree from the regression tests. -- The human mind treats a new idea the way the body treats a strange protein -- it rejects it. -- P. Medawar ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[PATCHES] patch adding new regexp functions
On Wed, 7 Feb 2007, Jeremy Drake wrote: Here is a patch to add these functions to core. Please take a look and let me know what you think. I had to jump through a bit of a hoop to avoid opr_sanity test from breaking, may not have done that right. Also, this patch allows regexp_replace to take more flags, since my two new functions needed flags also, I split flag parsing into a seperate function and made regexp_replace use it too. It also results that the error message for an invalid flag in regexp_replace changes, since it is now more generic as it is called from multiple functions. I still need to write documentation for the new functions before I consider the patch completed, but please take a look at the code and see if it is acceptable as I do not have any further changes planned for it. I have added documentation for the functions in this patch. At this point I am ready to submit this patch for the review and application process. Please let me know if you find any issues with it. Thank you -- The trouble with being punctual is that nobody's there to appreciate it. -- Franklin P. Jones regexp-split-matches-documented.patch.gz Description: Binary data ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] WIP patch adding new regexp functions
On Wed, 7 Feb 2007, David Fetter wrote:
On Wed, Feb 07, 2007 at 09:23:58AM -0500, Tom Lane wrote:
Jeremy Drake [EMAIL PROTECTED] writes:
* Put together a patch to add these functions to core. I could put them
directly in regexp.c, so the support functions could stay static. My
concern here is that I don't know if there are any functions currently
in core with OUT parameters.
As of 8.2 there are.
If we are going to include these I would vote for core not contrib
status, exactly to avoid having to export those functions.
+1 for core. :)
Here is a patch to add these functions to core. Please take a look and
let me know what you think. I had to jump through a bit of a hoop to
avoid opr_sanity test from breaking, may not have done that right.
Also, this patch allows regexp_replace to take more flags, since my two
new functions needed flags also, I split flag parsing into a seperate
function and made regexp_replace use it too. It also results that the
error message for an invalid flag in regexp_replace changes, since it is
now more generic as it is called from multiple functions.
I still need to write documentation for the new functions before I
consider the patch completed, but please take a look at the code and see
if it is acceptable as I do not have any further changes planned for it.
--
Chicken Soup, n.:
An ancient miracle drug containing equal parts of aureomycin,
cocaine, interferon, and TLC. The only ailment chicken soup
can't cure is neurotic dependence on one's mother.
-- Arthur Naiman, Every Goy's Guide to YiddishIndex: src/backend/utils/adt/regexp.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/backend/utils/adt/regexp.c,v
retrieving revision 1.68
diff -c -r1.68 regexp.c
*** src/backend/utils/adt/regexp.c 5 Jan 2007 22:19:41 - 1.68
--- src/backend/utils/adt/regexp.c 8 Feb 2007 00:04:08 -
***
*** 29,36
--- 29,39
*/
#include postgres.h
+ #include funcapi.h
+ #include access/heapam.h
#include regex/regex.h
#include utils/builtins.h
+ #include utils/lsyscache.h
#include utils/guc.h
***
*** 75,80
--- 78,127
regex_t cre_re; /* the compiled regular
expression */
} cached_re_str;
+ typedef struct re_comp_flags {
+ bool return_pre_and_post;
+ bool glob;
+ int cflags;
+ } re_comp_flags;
+
+ typedef struct regexp_matches_ctx {
+ text* orig_str;
+ size_torig_len;
+ pg_wchar* wide_str;
+ size_twide_len;
+ regex_t * cpattern;
+ size_toffset;
+ regmatch_t * pmatch;
+
+ /* flag options */
+ re_comp_flags flags;
+
+ /* return type info */
+ TupleDesc rettupdesc;
+ Oid rettype;
+ TypeFuncClass typefunc;
+
+ /* text type info */
+ Oid param_type;
+ int16 typlen;
+ bool typbyval;
+ char typalign;
+ } regexp_matches_ctx;
+
+ typedef struct regexp_split_ctx {
+ text* orig_str;
+ size_torig_len;
+ pg_wchar* wide_str;
+ size_twide_len;
+ regex_t * cpattern;
+ regmatch_tmatch;
+ size_toffset;
+
+ /* flag options */
+ re_comp_flags flags;
+ } regexp_split_ctx;
+
+
static intnum_res = 0;/* # of cached re's */
static cached_re_str re_array[MAX_CACHED_RES];/* cached re's */
***
*** 191,238
}
/*
! * RE_compile_and_execute - compile and execute a RE
*
* Returns TRUE on match, FALSE on no match
*
! *text_re --- the pattern, expressed as an *untoasted* TEXT object
! *dat --- the data to match against (need not be null-terminated)
! *dat_len --- the length of the data string
! *cflags --- compile options for the pattern
*nmatch, pmatch --- optional return area for match details
*
! * Both pattern and data are given in the database encoding. We internally
! * convert to array of pg_wchar which is what Spencer's regex package wants.
*/
static bool
! RE_compile_and_execute(text *text_re, char *dat, int dat_len,
! int cflags, int nmatch, regmatch_t
*pmatch)
{
- pg_wchar *data;
- size_t data_len;
int regexec_result;
- regex_t*re;
charerrMsg[100];
- /* Convert data string to wide characters */
- data = (pg_wchar *) palloc((dat_len + 1) * sizeof(pg_wchar));
- data_len = pg_mb2wchar_with_len(dat, data, dat_len);
-
- /* Compile RE */
- re
Re: [PATCHES] [HACKERS] writing new regexp functions
On Sun, 4 Feb 2007, David Fetter wrote:
On Fri, Feb 02, 2007 at 07:01:33PM -0800, Jeremy Drake wrote:
Let me know if you see any bugs or issues with this code, and I am
open to suggestions for further regression tests ;)
Things that I still want to look into:
* regexp flags (a la regexp_replace).
One more text field at the end is how the regexp_replace() one does
it.
That's how I did it.
* maybe make regexp_matches return setof whatever, if given a 'g' flag
return all matches in string.
This is doable with current machinery, albeit a little clumsily.
I have implemented this too.
* maybe a join function that works as an aggregate
SELECT join(',', col) FROM tbl
currently can be written as
SELECT array_to_string(ARRAY(SELECT col FROM tbl), ',')
The array_accum() aggregate in the docs works OK for this purpose.
I have not tackled this yet, I think it may be better to stick with the
ARRAY() construct for now.
So, here is the new version of the code, and also a new version of the
patch to core, which fixes some compile warnings that I did not see at
first because I was using ICC rather than GCC.
Here is the README.regexp_ext from the tar file:
This package contains regexp functions beyond those currently provided
in core PostgreSQL, utilizing the regexp engine built into core. This
is still a work-in-progress.
The most recent version of this code can be found at
http://www.jdrake.com/postgresql/regexp/regexp_ext.tar.gz
and the prerequisite patch to PostgreSQL core, which has been submitted
for review, can be found at
http://www.jdrake.com/postgresql/regexp/regexp-export.patch
The .tar.gz file expects to be untarred in contrib/. I have made some
regression tests that can be run using 'make installcheck' as normal for
contrib. I think they exercise the corner cases in the code, but I may
very well have missed some. It requires the above mentioned patch to
core to compile, as it takes advantage of new exported functions from
src/backend/utils/adt/regexp.c.
Let me know if you see any bugs or issues with this code, and I am open to
suggestions for further regression tests ;)
Functions implemented in this module:
* regexp_split(str text, pattern text) RETURNS SETOF text
regexp_split(str text, pattern text, flags text) RETURNS SETOF text
returns each section of the string delimited by the pattern.
* regexp_matches(str text, pattern text) RETURNS text[]
returns all capture groups when matching pattern against string in an array
* regexp_matches(str text, pattern text, flags text) RETURNS SETOF
(prematch text, fullmatch text, matches text[], postmatch text)
returns all capture groups when matching pattern against string in an array.
also returns the entire match in fullmatch. if the 'g' option is given,
returns all matches in the string. if the 'r' option is given, also return
the text before and after the match in prematch and postmatch respectively.
See the regression tests for more details about usage and return values.
Recent changes:
* I have put the pattern after the string in all of the functions, as
discussed on the pgsql-hackers mailing list.
* regexp flags (a la regexp_replace).
* make regexp_matches return setof whatever, if given a 'g' flag return
all matches in string.
Things that I still want to look into:
* maybe a join function that works as an aggregate
SELECT join(',', col) FROM tbl
currently can be written as
SELECT array_to_string(ARRAY(SELECT col FROM tbl), ',')
--
Philogeny recapitulates erogeny; erogeny recapitulates philogeny.Index: src/backend/utils/adt/regexp.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/backend/utils/adt/regexp.c,v
retrieving revision 1.68
diff -c -r1.68 regexp.c
*** src/backend/utils/adt/regexp.c 5 Jan 2007 22:19:41 - 1.68
--- src/backend/utils/adt/regexp.c 4 Feb 2007 07:58:26 -
***
*** 29,41
*/
#include postgres.h
- #include regex/regex.h
#include utils/builtins.h
#include utils/guc.h
/* GUC-settable flavor parameter */
! static intregex_flavor = REG_ADVANCED;
/*
--- 29,41
*/
#include postgres.h
#include utils/builtins.h
#include utils/guc.h
+ #include utils/regexp.h
/* GUC-settable flavor parameter */
! int regex_flavor = REG_ADVANCED;
/*
***
*** 90,96
* Pattern is given in the database encoding. We internally convert to
* array of pg_wchar which is what Spencer's regex package wants.
*/
! static regex_t *
RE_compile_and_cache(text *text_re, int cflags)
{
int text_re_len = VARSIZE(text_re);
--- 90,96
* Pattern is given in the database encoding. We internally convert to
* array of pg_wchar which is what Spencer's regex package wants.
*/
! regex_t *
RE_compile_and_cache(text *text_re, int cflags
Re: [pgsql-patches] [HACKERS] less privileged pl install
This version of the patch includes documentation changes. Please review... -- The more things change, the more they stay insane.Index: doc/src/sgml/catalogs.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/catalogs.sgml,v retrieving revision 2.143 diff -c -r2.143 catalogs.sgml *** doc/src/sgml/catalogs.sgml 22 Jan 2007 01:35:19 - 2.143 --- doc/src/sgml/catalogs.sgml 27 Jan 2007 23:05:33 - *** *** 2635,2640 --- 2635,2647 /row row + entrystructfieldlanowner/structfield/entry + entrytypeoid/type/entry + entryliterallink linkend=catalog-pg-authidstructnamepg_authid/structname/link.oid/literal/entry + entryOwner of the language, usually either the owner of the database, or the superuser who created it/entry + /row + + row entrystructfieldlanispl/structfield/entry entrytypebool/type/entry entry/entry *** *** 3267,3272 --- 3274,3285 /row row + entrystructfieldtmpldbaallowed/structfield/entry + entrytypeboolean/type/entry + entryTrue if language can be created by a database owner if it is trusted/entry + /row + + row entrystructfieldtmplhandler/structfield/entry entrytypetext/type/entry entryName of call handler function/entry Index: doc/src/sgml/ref/alter_language.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/alter_language.sgml,v retrieving revision 1.6 diff -c -r1.6 alter_language.sgml *** doc/src/sgml/ref/alter_language.sgml16 Sep 2006 00:30:16 - 1.6 --- doc/src/sgml/ref/alter_language.sgml26 Jan 2007 01:01:40 - *** *** 21,26 --- 21,28 refsynopsisdiv synopsis ALTER LANGUAGE replaceablename/replaceable RENAME TO replaceablenewname/replaceable + + ALTER LANGUAGE replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable /synopsis /refsynopsisdiv *** *** 48,53 --- 50,64 /varlistentry varlistentry + termreplaceablenew_owner/replaceable/term + listitem + para + The new owner of the language. + /para + /listitem +/varlistentry + +varlistentry termreplaceablenewname/replaceable/term listitem para Index: doc/src/sgml/ref/create_language.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/create_language.sgml,v retrieving revision 1.42 diff -c -r1.42 create_language.sgml *** doc/src/sgml/ref/create_language.sgml 16 Sep 2006 00:30:17 - 1.42 --- doc/src/sgml/ref/create_language.sgml 28 Jan 2007 00:04:10 - *** *** 36,42 database. Subsequently, functions and trigger procedures can be defined in this new language. The user must have the productnamePostgreSQL/productname superuser privilege to !register a new language. /para para --- 36,47 database. Subsequently, functions and trigger procedures can be defined in this new language. The user must have the productnamePostgreSQL/productname superuser privilege to !register a new language, unless the language is present in the !link linkend=catalog-pg-pltemplatestructnamepg_pltemplate/structname/link !system catalog, the language template is marked as trusted, and the !structfieldtmpldbaallowed/structfield column for the template is !true, in which case the database owner is also allowed to register !the language. /para para Index: src/backend/catalog/aclchk.c === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v retrieving revision 1.135 diff -c -r1.135 aclchk.c *** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135 --- src/backend/catalog/aclchk.c26 Jan 2007 23:53:03 - *** *** 1003,1013 /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. -* -* Note: for now, languages are treated as owned by the bootstrap -* user. We should add an owner column to pg_language instead. */ ! ownerId = BOOTSTRAP_SUPERUSERID; aclDatum = SysCacheGetAttr(LANGNAME, tuple, Anum_pg_language_lanacl, isNull); if (isNull) --- 1003,1010 /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper
Re: [pgsql-patches] [HACKERS] less privileged pl install
On Thu, 25 Jan 2007, Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: I think that an ALTER LANGUAGE OWNER TO is the proper response to these things, and unless I hear otherwise I will attempt to add this to my patch. Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME TO for the owner, which I missed before. I would appreciate someone with more knowledge of the permissions infrastructure to take a look at it since I am fairly new to it and may not fully understand its intricacies. I have refactored the owner checking of languages in the same manner as it is for other owned objects. I have changed to using standard permissions error messages (aclcheck_error) for the language permissions errors. I consider this patch ready for review, assuming the permissions rules outlined by Tom Lane on -hackers are valid. For reference, here are the rules that this patch is intended to implement: On Wed, 24 Jan 2007, Tom Lane wrote: In detail, it'd look something like: * For an untrusted language: must be superuser to either create or use the language (no change from current rules). Ownership of the pg_language entry is really irrelevant, as is its ACL. * For a trusted language: * if pg_pltemplate.something is ON: either a superuser or the current DB's owner can CREATE the language. In either case the pg_language entry will be marked as owned by the DB owner (pg_database.datdba), which means that subsequently he (or a superuser) can grant or deny USAGE within his DB. * if pg_pltemplate.something is OFF: must be superuser to CREATE the language; subsequently it will be owned by you, so only you or another superuser can grant or deny USAGE (same behavior as currently). The only difference from this is, that when superuser is required, the owner of the language is not the superuser who created it, but BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the same behavior as currently took precedence. The current behavior in cvs is that languages have no owner, and for purposes where one would be needed it is assumed to be BOOTSTRAP_SUPERUSERID. Is this valid, or should I instead set the owner to GetUserId() in those cases? -- Academic politics is the most vicious and bitter form of politics, because the stakes are so low. -- Wallace SayreIndex: doc/src/sgml/ref/alter_language.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/alter_language.sgml,v retrieving revision 1.6 diff -c -r1.6 alter_language.sgml *** doc/src/sgml/ref/alter_language.sgml16 Sep 2006 00:30:16 - 1.6 --- doc/src/sgml/ref/alter_language.sgml26 Jan 2007 01:01:40 - *** *** 21,26 --- 21,28 refsynopsisdiv synopsis ALTER LANGUAGE replaceablename/replaceable RENAME TO replaceablenewname/replaceable + + ALTER LANGUAGE replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable /synopsis /refsynopsisdiv *** *** 48,53 --- 50,64 /varlistentry varlistentry + termreplaceablenew_owner/replaceable/term + listitem + para + The new owner of the language. + /para + /listitem +/varlistentry + +varlistentry termreplaceablenewname/replaceable/term listitem para Index: src/backend/catalog/aclchk.c === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v retrieving revision 1.135 diff -c -r1.135 aclchk.c *** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135 --- src/backend/catalog/aclchk.c26 Jan 2007 23:53:03 - *** *** 1003,1013 /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. -* -* Note: for now, languages are treated as owned by the bootstrap -* user. We should add an owner column to pg_language instead. */ ! ownerId = BOOTSTRAP_SUPERUSERID; aclDatum = SysCacheGetAttr(LANGNAME, tuple, Anum_pg_language_lanacl, isNull); if (isNull) --- 1003,1010 /* * Get owner ID and working copy of existing ACL. If there's no ACL, * substitute the proper default. */ ! ownerId = pg_language_tuple-lanowner; aclDatum = SysCacheGetAttr(LANGNAME, tuple, Anum_pg_language_lanacl, isNull); if (isNull) *** *** 1770,1777 (errcode(ERRCODE_UNDEFINED_OBJECT
Re: [pgsql-patches] [HACKERS] less privileged pl install
On Sat, 27 Jan 2007, Tom Lane wrote:
I'd go with GetUserId() in the cases where you're not explicitly
assigning ownership to the datdba role. AFAIR the assumption that
languages are owned by BOOTSTRAP_SUPERUSERID was just a kluge to use in
some bits of code that had to have a notion of a specific owner. Now
in reality every superuser has the same privileges as every other one,
and so it doesn't matter much which one you use, but we might as well
record who actually did the deed.
Here is a version of the patch which does this. Very minor change from
the last version...
--
Begathon, n.:
A multi-day event on public television, used to raise money so
you won't have to watch commercials.Index: doc/src/sgml/ref/alter_language.sgml
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/alter_language.sgml,v
retrieving revision 1.6
diff -c -r1.6 alter_language.sgml
*** doc/src/sgml/ref/alter_language.sgml16 Sep 2006 00:30:16 -
1.6
--- doc/src/sgml/ref/alter_language.sgml26 Jan 2007 01:01:40 -
***
*** 21,26
--- 21,28
refsynopsisdiv
synopsis
ALTER LANGUAGE replaceablename/replaceable RENAME TO
replaceablenewname/replaceable
+
+ ALTER LANGUAGE replaceablename/replaceable OWNER TO
replaceablenew_owner/replaceable
/synopsis
/refsynopsisdiv
***
*** 48,53
--- 50,64
/varlistentry
varlistentry
+ termreplaceablenew_owner/replaceable/term
+ listitem
+ para
+ The new owner of the language.
+ /para
+ /listitem
+/varlistentry
+
+varlistentry
termreplaceablenewname/replaceable/term
listitem
para
Index: src/backend/catalog/aclchk.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.135
diff -c -r1.135 aclchk.c
*** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135
--- src/backend/catalog/aclchk.c26 Jan 2007 23:53:03 -
***
*** 1003,1013
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
-*
-* Note: for now, languages are treated as owned by the
bootstrap
-* user. We should add an owner column to pg_language instead.
*/
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
isNull);
if (isNull)
--- 1003,1010
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
*/
! ownerId = pg_language_tuple-lanowner;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
isNull);
if (isNull)
***
*** 1770,1777
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg(language with OID %u does not exist,
lang_oid)));
! /* XXX pg_language should have an owner column, but doesn't */
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
isNull);
--- 1767,1773
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg(language with OID %u does not exist,
lang_oid)));
! ownerId = ((Form_pg_language) GETSTRUCT(tuple))-lanowner;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
isNull);
***
*** 2148,2153
--- 2144,2177
}
/*
+ * Ownership check for a procedural language (specified by OID)
+ */
+ bool
+ pg_language_ownercheck(Oid lan_oid, Oid roleid)
+ {
+ HeapTuple tuple;
+ Oid ownerId;
+
+ /* Superusers bypass all permission checking. */
+ if (superuser_arg(roleid))
+ return true;
+
+ tuple = SearchSysCache(LANGOID,
+ ObjectIdGetDatum(lan_oid),
+ 0, 0, 0);
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+errmsg(language with OID %u does not exist,
lan_oid)));
+
+ ownerId = ((Form_pg_language) GETSTRUCT(tuple))-lanowner;
+
+ ReleaseSysCache(tuple);
+
+
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Jeremy Drake wrote:
On Wed, 24 Jan 2007, Tom Lane wrote:
In detail, it'd look something like:
* For an untrusted language: must be superuser to either create or use
the language (no change from current rules). Ownership of the
pg_language entry is really irrelevant, as is its ACL.
* For a trusted language:
* if pg_pltemplate.something is ON: either a superuser or the current
DB's owner can CREATE the language. In either case the pg_language
entry will be marked as owned by the DB owner (pg_database.datdba),
which means that subsequently he (or a superuser) can grant or deny
USAGE within his DB.
* if pg_pltemplate.something is OFF: must be superuser to CREATE the
language; subsequently it will be owned by you, so only you or another
superuser can grant or deny USAGE (same behavior as currently).
I think I have what is described here implemented in this patch, so that
it can be better understood. Thoughts?
This version of the patch creates a shared dependency on the language
owner.
I have thought of some other questions about the owner stuff which I will
send on -hackers...
--
Afternoon, n.:
That part of the day we spend worrying about how we wasted the
morning.Index: src/backend/catalog/aclchk.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.135
diff -c -r1.135 aclchk.c
*** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135
--- src/backend/catalog/aclchk.c25 Jan 2007 06:35:21 -
***
*** 1003,1013
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
-*
-* Note: for now, languages are treated as owned by the
bootstrap
-* user. We should add an owner column to pg_language instead.
*/
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
isNull);
if (isNull)
--- 1003,1010
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
*/
! ownerId = pg_language_tuple-lanowner;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
isNull);
if (isNull)
***
*** 1770,1777
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg(language with OID %u does not exist,
lang_oid)));
! /* XXX pg_language should have an owner column, but doesn't */
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
isNull);
--- 1767,1773
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg(language with OID %u does not exist,
lang_oid)));
! ownerId = ((Form_pg_language) GETSTRUCT(tuple))-lanowner;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
isNull);
Index: src/backend/commands/proclang.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/commands/proclang.c,v
retrieving revision 1.71
diff -c -r1.71 proclang.c
*** src/backend/commands/proclang.c 22 Jan 2007 01:35:20 - 1.71
--- src/backend/commands/proclang.c 25 Jan 2007 23:15:45 -
***
*** 17,22
--- 17,24
#include access/heapam.h
#include catalog/dependency.h
#include catalog/indexing.h
+ #include catalog/pg_authid.h
+ #include catalog/pg_database.h
#include catalog/pg_language.h
#include catalog/pg_namespace.h
#include catalog/pg_pltemplate.h
***
*** 27,32
--- 29,35
#include miscadmin.h
#include parser/gramparse.h
#include parser/parse_func.h
+ #include utils/acl.h
#include utils/builtins.h
#include utils/fmgroids.h
#include utils/lsyscache.h
***
*** 36,50
typedef struct
{
booltmpltrusted;/* trusted? */
char *tmplhandler;/* name of handler function */
char *tmplvalidator; /* name of validator function, or NULL
*/
char *tmpllibrary;/* path of shared library */
} PLTemplate;
static void create_proc_lang(const char *languageName,
!Oid handlerOid, Oid valOid, bool trusted);
static PLTemplate
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install (formerly tsearch
On Wed, 24 Jan 2007, Jeremy Drake wrote: On Wed, 24 Jan 2007, Martijn van Oosterhout wrote: Something I've wondered about before is the concept of having installed Modules in the system. Let's say for example that while compiling postgres it compiled the modules in contrib also and installed them in a modules directory. Once installed there, unpriviledged users could say INSTALL foo and it would install the module, even if they do not have the permissions to create them themselves. That would be great, and also it would be great to be able to CREATE LANGUAGE as a regular user for a trusted pl that is already compiled/installed. Something like the attached (simple) change to allow CREATE LANGUAGE by unprivileged users for trusted languages already present in pg_pltemplate. I'm not quite sure how one would go about doing the module thing, I think that would be more complex. Something simple like allowing creation of C language functions in libraries in $libdir would probably not be sufficient, because an unprivileged user could create functions that have the wrong paramters or return values and crash things pretty good that way. Any ideas how this would work? Perhaps a sql script in sharedir could be run by the backend as though by a superuser... -- Ed Sullivan will be around as long as someone else has talent. -- Fred AllenIndex: src/backend/commands/proclang.c === RCS file: /home/jeremyd/local/postgres/cvsuproot/pgsql/src/backend/commands/proclang.c,v retrieving revision 1.71 diff -c -r1.71 proclang.c *** src/backend/commands/proclang.c 22 Jan 2007 01:35:20 - 1.71 --- src/backend/commands/proclang.c 24 Jan 2007 23:50:49 - *** *** 61,74 Oid funcargtypes[1]; /* -* Check permission -*/ - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), -errmsg(must be superuser to create procedural language))); - - /* * Translate the language name and check that this language doesn't * already exist */ --- 61,66 *** *** 97,102 --- 89,103 (errmsg(using pg_pltemplate information instead of CREATE LANGUAGE parameters))); /* +* Check permission +*/ + if (!pltemplate-tmpltrusted !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), +errmsg(must be superuser to create untrusted procedural language))); + + + /* * Find or create the handler function, which we force to be in the * pg_catalog schema. If already present, it must have the correct * return type. *** *** 189,194 --- 190,203 errhint(The supported languages are listed in the pg_pltemplate system catalog.))); /* +* Check permission +*/ + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), +errmsg(must be superuser to create custom procedural language))); + + /* * Lookup the PL handler function and check that it is of the expected * return type */ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Tom Lane wrote: Jeremy Drake [EMAIL PROTECTED] writes: On Wed, 24 Jan 2007, Jeremy Drake wrote: That would be great, and also it would be great to be able to CREATE LANGUAGE as a regular user for a trusted pl that is already compiled/installed. Something like the attached (simple) change to allow CREATE LANGUAGE by unprivileged users for trusted languages already present in pg_pltemplate. If it were merely a matter of removing an error check I think we would have done it already. However, pltemplate will have all the languages in it whether the DBA wants to allow them to be used or not; so I'd say that there really needs to be *some* sort of privilege check here. What that is and how to implement it are the hard parts. So I guess it depends on what you mean by DBA. Perhaps the database owner? Or some new privilege type (GRANT CREATE ON LANGUAGE ...? Or GRANT CREATE LANGUAGE ON DATABASE...?) that the db owner has by default? -- 7:30, Channel 5: The Bionic Dog (Action/Adventure) The Bionic Dog drinks too much and kicks over the National Redwood Forest. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Tom Lane wrote: Not the DB owner. If you are worried about whether to allow use of PLs it's almost certainly an installation-wide security concern, so I'd say that the privilege has to flow from a superuser. GRANT CREATE ON LANGUAGE feeding into a flag bit in pltemplate would work, if restricted to superusers, but I suspect people would find this confusing because it'd work completely differently from GRANT USAGE ON LANGUAGE (eg, because the latter has only database-local effects). Might be better to use a different syntax. I had thought that it would be database-local, but I understand now that it makes more sense to be global. Note I'm not arguing against allowing it to be on by default, I just want to be sure there is a way for paranoid DBAs to turn it off. Maybe it'd be sufficient if the flag bit was there but UPDATE pg_pltemplate was the only way to manipulate it --- we've gotten along with treating datistemplate and datallowconn that way. That sounds reasonable to me. I'll try to put together a patch like this (adding a boolean column to pg_pltemplate) and see if this is acceptable. I assume that only superusers can modify pg_pltemplate already ;) Or we could go the full nine yards and add ACLs to pltemplate, but that's probably overkill. Agreed. -- He thought he saw an albatross That fluttered 'round the lamp. He looked again and saw it was A penny postage stamp. You'd best be getting home, he said, The nights are rather damp. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Tom Lane wrote:
In detail, it'd look something like:
* For an untrusted language: must be superuser to either create or use
the language (no change from current rules). Ownership of the
pg_language entry is really irrelevant, as is its ACL.
* For a trusted language:
* if pg_pltemplate.something is ON: either a superuser or the current
DB's owner can CREATE the language. In either case the pg_language
entry will be marked as owned by the DB owner (pg_database.datdba),
which means that subsequently he (or a superuser) can grant or deny
USAGE within his DB.
* if pg_pltemplate.something is OFF: must be superuser to CREATE the
language; subsequently it will be owned by you, so only you or another
superuser can grant or deny USAGE (same behavior as currently).
I think I have what is described here implemented in this patch, so that
it can be better understood. Thoughts?
--
Nobody said computers were going to be polite.Index: src/backend/catalog/aclchk.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.135
diff -c -r1.135 aclchk.c
*** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135
--- src/backend/catalog/aclchk.c25 Jan 2007 06:35:21 -
***
*** 1003,1013
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
-*
-* Note: for now, languages are treated as owned by the
bootstrap
-* user. We should add an owner column to pg_language instead.
*/
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
isNull);
if (isNull)
--- 1003,1010
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
*/
! ownerId = pg_language_tuple-lanowner;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
isNull);
if (isNull)
***
*** 1770,1777
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg(language with OID %u does not exist,
lang_oid)));
! /* XXX pg_language should have an owner column, but doesn't */
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
isNull);
--- 1767,1773
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg(language with OID %u does not exist,
lang_oid)));
! ownerId = ((Form_pg_language) GETSTRUCT(tuple))-lanowner;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
isNull);
Index: src/backend/commands/proclang.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/commands/proclang.c,v
retrieving revision 1.71
diff -c -r1.71 proclang.c
*** src/backend/commands/proclang.c 22 Jan 2007 01:35:20 - 1.71
--- src/backend/commands/proclang.c 25 Jan 2007 06:30:12 -
***
*** 17,22
--- 17,24
#include access/heapam.h
#include catalog/dependency.h
#include catalog/indexing.h
+ #include catalog/pg_authid.h
+ #include catalog/pg_database.h
#include catalog/pg_language.h
#include catalog/pg_namespace.h
#include catalog/pg_pltemplate.h
***
*** 27,32
--- 29,35
#include miscadmin.h
#include parser/gramparse.h
#include parser/parse_func.h
+ #include utils/acl.h
#include utils/builtins.h
#include utils/fmgroids.h
#include utils/lsyscache.h
***
*** 36,50
typedef struct
{
booltmpltrusted;/* trusted? */
char *tmplhandler;/* name of handler function */
char *tmplvalidator; /* name of validator function, or NULL
*/
char *tmpllibrary;/* path of shared library */
} PLTemplate;
static void create_proc_lang(const char *languageName,
!Oid handlerOid, Oid valOid, bool trusted);
static PLTemplate *find_language_template(const char *languageName);
/* -
* CREATE PROCEDURAL LANGUAGE
--- 39,56
typedef struct
{
booltmpltrusted;/* trusted? */
+ bool
[pgsql-patches] largeobject regression tests - updated patch
Given the recent changes to the regression scripts, the large object regression test patch I submitted quite a while ago, and is currently in the patch hold queue, no longer cleanly applies. This patch is functionally the same as the last one, but cleanly applies to current CVS head. Note that this patch requires the psql largeobject quiet mode patch which is also in the hold queue and still applies cleanly. largeobj-regress-v3.patch.gz Description: Binary data ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [pgsql-patches] largeobject regression tests - updated patch
Oops, that patch did not actually apply cleanly. Try this one instead, it should. Please disregard the previous copy. Sorry. On Fri, 19 Jan 2007, Jeremy Drake wrote: Given the recent changes to the regression scripts, the large object regression test patch I submitted quite a while ago, and is currently in the patch hold queue, no longer cleanly applies. This patch is functionally the same as the last one, but cleanly applies to current CVS head. Note that this patch requires the psql largeobject quiet mode patch which is also in the hold queue and still applies cleanly. -- Peace, n.: In international affairs, a period of cheating between two periods of fighting. -- Ambrose Bierce, The Devil's Dictionary largeobj-regress-v3.patch.gz Description: Binary data ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] psql \lo_* quiet mode patch
I sent this in rather late in the 8.2 cycle, so now that 8.3 development
is underway I thought I'd try sending it again. This patch was necessary
in the development of a large object regression test, but is logically
seperate and reasonable even without that test, so I'm sending it in
seperately for independant consideration. I'll save my pushing on the
large object test until this one gets in ;)
-- Forwarded message --
Date: Thu, 26 Oct 2006 15:58:07 -0700 (PDT)
From: Jeremy Drake [EMAIL PROTECTED]
To: PostgreSQL Patches pgsql-patches@postgresql.org
Subject: [PATCHES] psql \lo_* quiet mode patch
I sent this in a while back, but never heard anything about it.
This patch makes psql's \lo_* commands respect the -q flag (or other
methods of setting quiet mode) as well as HTML output mode. This came in
very handy when writing a regression test which uses the \lo_import
command since it would otherwise output the OID of the new large object
which would be different every run.
Please let me know if it is ok, or if I need to do it differently.
--
Let me assure you that to us here at First National, you're not just a
number. You're two numbers, a dash, three more numbers, another dash
and another number.
-- James EstesIndex: src/bin/psql/large_obj.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/bin/psql/large_obj.c,v
retrieving revision 1.46
diff -c -r1.46 large_obj.c
*** src/bin/psql/large_obj.c29 Aug 2006 15:19:51 - 1.46
--- src/bin/psql/large_obj.c25 Sep 2006 23:02:33 -
***
*** 12,17
--- 12,54
#include settings.h
#include common.h
+ static void
+ print_lo_result(const char *fmt,...)
+ __attribute__((format(printf, 1, 2)));
+
+ static void
+ print_lo_result(const char *fmt,...)
+ {
+ va_list ap;
+
+ if (!pset.quiet)
+ {
+ if (pset.popt.topt.format == PRINT_HTML)
+ {
+ fputs(p, pset.queryFout);
+ // XXX should probably use html_escaped_print here
+ // but I know the strings have no bad chars
+ }
+
+ va_start(ap, fmt);
+ vfprintf(pset.queryFout, fmt, ap);
+ va_end(ap);
+
+ if (pset.popt.topt.format == PRINT_HTML)
+ fputs(/p\n, pset.queryFout);
+ else
+ fputs(\n, pset.queryFout);
+ }
+
+ if (pset.logfile)
+ {
+ va_start(ap, fmt);
+ vfprintf(pset.logfile, fmt, ap);
+ va_end(ap);
+ fputs(\n, pset.logfile);
+ }
+ }
+
/*
* Prepare to do a large-object operation.We *must* be inside a
transaction
***
*** 129,135
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! fprintf(pset.queryFout, lo_export\n);
return true;
}
--- 166,172
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! print_lo_result(lo_export);
return true;
}
***
*** 189,195
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! fprintf(pset.queryFout, lo_import %u\n, loid);
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
--- 226,233
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! print_lo_result(lo_import %u, loid);
!
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
***
*** 225,231
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! fprintf(pset.queryFout, lo_unlink %u\n, loid);
return true;
}
--- 263,269
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! print_lo_result(lo_unlink %u, loid);
return true;
}
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
[PATCHES] psql \lo_* quiet mode patch
I sent this in a while back, but never heard anything about it.
This patch makes psql's \lo_* commands respect the -q flag (or other
methods of setting quiet mode) as well as HTML output mode. This came in
very handy when writing a regression test which uses the \lo_import
command since it would otherwise output the OID of the new large object
which would be different every run.
Please let me know if it is ok, or if I need to do it differently.
--
Let me assure you that to us here at First National, you're not just a
number. You're two numbers, a dash, three more numbers, another dash
and another number.
-- James EstesIndex: src/bin/psql/large_obj.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/bin/psql/large_obj.c,v
retrieving revision 1.46
diff -c -r1.46 large_obj.c
*** src/bin/psql/large_obj.c29 Aug 2006 15:19:51 - 1.46
--- src/bin/psql/large_obj.c25 Sep 2006 23:02:33 -
***
*** 12,17
--- 12,54
#include settings.h
#include common.h
+ static void
+ print_lo_result(const char *fmt,...)
+ __attribute__((format(printf, 1, 2)));
+
+ static void
+ print_lo_result(const char *fmt,...)
+ {
+ va_list ap;
+
+ if (!pset.quiet)
+ {
+ if (pset.popt.topt.format == PRINT_HTML)
+ {
+ fputs(p, pset.queryFout);
+ // XXX should probably use html_escaped_print here
+ // but I know the strings have no bad chars
+ }
+
+ va_start(ap, fmt);
+ vfprintf(pset.queryFout, fmt, ap);
+ va_end(ap);
+
+ if (pset.popt.topt.format == PRINT_HTML)
+ fputs(/p\n, pset.queryFout);
+ else
+ fputs(\n, pset.queryFout);
+ }
+
+ if (pset.logfile)
+ {
+ va_start(ap, fmt);
+ vfprintf(pset.logfile, fmt, ap);
+ va_end(ap);
+ fputs(\n, pset.logfile);
+ }
+ }
+
/*
* Prepare to do a large-object operation.We *must* be inside a
transaction
***
*** 129,135
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! fprintf(pset.queryFout, lo_export\n);
return true;
}
--- 166,172
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! print_lo_result(lo_export);
return true;
}
***
*** 189,195
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! fprintf(pset.queryFout, lo_import %u\n, loid);
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
--- 226,233
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! print_lo_result(lo_import %u, loid);
!
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
***
*** 225,231
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! fprintf(pset.queryFout, lo_unlink %u\n, loid);
return true;
}
--- 263,269
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! print_lo_result(lo_unlink %u, loid);
return true;
}
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
[PATCHES] large object regression tests, take two
This is the latest version of the large object regression test I have been
working on. Note that a prerequisite for this version of the test is the
patch I made to psql to make it not output on \lo_* commands in quiet mode
is required (also attached, it's small).
Sorry that it still makes use of the sed trickery like the copy test does,
but:
1) I think the server side lo_(import|export) functions need to be tested
as well as the psql variants
2) ISTM that making assumptions about the working directory of psql
during
the regression tests would open a can of worms, especially wrt VPATH
builds where the data files could be in a completely separate tree from
the regression tests.
Thoughts?
--
Why did the Roman Empire collapse?
What is the Latin for office automation?Index: src/bin/psql/large_obj.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/bin/psql/large_obj.c,v
retrieving revision 1.46
diff -c -r1.46 large_obj.c
*** src/bin/psql/large_obj.c29 Aug 2006 15:19:51 - 1.46
--- src/bin/psql/large_obj.c25 Sep 2006 23:02:33 -
***
*** 12,17
--- 12,54
#include settings.h
#include common.h
+ static void
+ print_lo_result(const char *fmt,...)
+ __attribute__((format(printf, 1, 2)));
+
+ static void
+ print_lo_result(const char *fmt,...)
+ {
+ va_list ap;
+
+ if (!pset.quiet)
+ {
+ if (pset.popt.topt.format == PRINT_HTML)
+ {
+ fputs(p, pset.queryFout);
+ // XXX should probably use html_escaped_print here
+ // but I know the strings have no bad chars
+ }
+
+ va_start(ap, fmt);
+ vfprintf(pset.queryFout, fmt, ap);
+ va_end(ap);
+
+ if (pset.popt.topt.format == PRINT_HTML)
+ fputs(/p\n, pset.queryFout);
+ else
+ fputs(\n, pset.queryFout);
+ }
+
+ if (pset.logfile)
+ {
+ va_start(ap, fmt);
+ vfprintf(pset.logfile, fmt, ap);
+ va_end(ap);
+ fputs(\n, pset.logfile);
+ }
+ }
+
/*
* Prepare to do a large-object operation.We *must* be inside a
transaction
***
*** 129,135
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! fprintf(pset.queryFout, lo_export\n);
return true;
}
--- 166,172
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! print_lo_result(lo_export);
return true;
}
***
*** 189,195
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! fprintf(pset.queryFout, lo_import %u\n, loid);
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
--- 226,233
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! print_lo_result(lo_import %u, loid);
!
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
***
*** 225,231
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! fprintf(pset.queryFout, lo_unlink %u\n, loid);
return true;
}
--- 263,269
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! print_lo_result(lo_unlink %u, loid);
return true;
}
largeobj-regress-v2.patch.gz
Description: Binary data
---(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
Re: [PATCHES] [HACKERS] large object regression tests
On Sun, 24 Sep 2006, Jeremy Drake wrote: On Thu, 21 Sep 2006, Tom Lane wrote: I think we could do without the Moby Dick extract too ... I am open to suggestions. I saw one suggestion that I use an image of an elephant, but I suspect that was tongue-in-cheek. I am not very fond of the idea of generating repetitious data, as I think it would be more difficult to determine whether or not the loseek/tell functions put me in the right place in the middle of the file. I just had the idea that I could use one of the existing data files which are used for testing COPY instead of the Moby Dick extract. They are already there, a few of them are pretty good sized, they have data in the file which is not just simple repetition so it would be pretty obvious if the seek function broke, and they are very unlikely to change. I am considering changing the test I put together to use tenk.data as the input file tomorrow and send in what I have again, since I also am doing a test of \lo_import (which also requires a patch to psql I sent in earlier to fix the output of the \lo_* commands to respect the output settings). -- When does summertime come to Minnesota, you ask? Well, last year, I think it was a Tuesday. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] large object regression tests
On Mon, 25 Sep 2006, Jeremy Drake wrote:
It looks like the large_obj.c output is missing much of the output
settings handling which is in the PrintQueryStatus function in common.c,
such as handling quiet mode, and html output. I will try to dig around
and try to put together a patch to make it respect the settings like other
commands...
I put together a patch for psql's large_obj.c to make it respect the
output settings. Is this reasonable?
--
For every complex problem, there is a solution that is simple, neat,
and wrong.
-- H. L. MenckenIndex: src/bin/psql/large_obj.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/bin/psql/large_obj.c,v
retrieving revision 1.46
diff -c -r1.46 large_obj.c
*** src/bin/psql/large_obj.c29 Aug 2006 15:19:51 - 1.46
--- src/bin/psql/large_obj.c25 Sep 2006 23:02:33 -
***
*** 12,17
--- 12,54
#include settings.h
#include common.h
+ static void
+ print_lo_result(const char *fmt,...)
+ __attribute__((format(printf, 1, 2)));
+
+ static void
+ print_lo_result(const char *fmt,...)
+ {
+ va_list ap;
+
+ if (!pset.quiet)
+ {
+ if (pset.popt.topt.format == PRINT_HTML)
+ {
+ fputs(p, pset.queryFout);
+ // XXX should probably use html_escaped_print here
+ // but I know the strings have no bad chars
+ }
+
+ va_start(ap, fmt);
+ vfprintf(pset.queryFout, fmt, ap);
+ va_end(ap);
+
+ if (pset.popt.topt.format == PRINT_HTML)
+ fputs(/p\n, pset.queryFout);
+ else
+ fputs(\n, pset.queryFout);
+ }
+
+ if (pset.logfile)
+ {
+ va_start(ap, fmt);
+ vfprintf(pset.logfile, fmt, ap);
+ va_end(ap);
+ fputs(\n, pset.logfile);
+ }
+ }
+
/*
* Prepare to do a large-object operation.We *must* be inside a
transaction
***
*** 129,135
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! fprintf(pset.queryFout, lo_export\n);
return true;
}
--- 166,172
if (!finish_lo_xact(\\lo_export, own_transaction))
return false;
! print_lo_result(lo_export);
return true;
}
***
*** 189,195
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! fprintf(pset.queryFout, lo_import %u\n, loid);
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
--- 226,233
if (!finish_lo_xact(\\lo_import, own_transaction))
return false;
! print_lo_result(lo_import %u, loid);
!
sprintf(oidbuf, %u, loid);
SetVariable(pset.vars, LASTOID, oidbuf);
***
*** 225,231
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! fprintf(pset.queryFout, lo_unlink %u\n, loid);
return true;
}
--- 263,269
if (!finish_lo_xact(\\lo_unlink, own_transaction))
return false;
! print_lo_result(lo_unlink %u, loid);
return true;
}
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] large object regression tests
On Thu, 21 Sep 2006, Tom Lane wrote: Jeremy Drake [EMAIL PROTECTED] writes: I put together a patch which adds a regression test for large objects, hopefully attached to this message. I would like some critique of it, to see if I have gone about it the right way. Also I would be happy to hear any additional tests which should be added to it. I'd prefer it if we could arrange not to need any absolute paths embedded into the test, because maintaining tests that require such is a real PITA --- instead of just committing the actual test output, one has to reverse-convert it to a .source file. I just copied how the test for COPY worked, since I perceived a similarity in what I needed to do (use external files to load data). I suggest that instead of testing the server-side lo_import/lo_export functions, perhaps you could test the psql equivalents and write and read a file in psql's working directory. I did not see any precedent for that when I was looking around in the existing tests for an example of how to do things. I am not even sure where the cwd of psql is, so I can put an input file there. Could you provide an example of how this might look, by telling me where to put a file in the src/test/regress tree and the path to give to \lo_import? Besides which, shouldn't both the server-side and psql versions be tested? When I was looking at the copy tests, it looked like the server-side ones were tested, and then the psql ones were tested by exporting and then importing data which was originally loaded from the server-side method. Am I correctly interpreting the precedent, or are you suggesting that the precedent be changed? I was trying to stay as close to the copy tests as possible since the functionality is so similar (transferring data to/from files in the filesystem, either via server-side functions which require absolute paths or via psql \ commands (which I forgot about for the lo funcs)). I think we could do without the Moby Dick extract too ... I am open to suggestions. I saw one suggestion that I use an image of an elephant, but I suspect that was tongue-in-cheek. I am not very fond of the idea of generating repetitious data, as I think it would be more difficult to determine whether or not the loseek/tell functions put me in the right place in the middle of the file. Perhaps if there was a way to generate deterministic pseudo-random data, that would work (has to be deterministic so the diffs of the output come out right). Anyone have a good example of seeding a random number generator and generating a bunch of bytea which is deterministic cross-platform? regards, tom lane In the mean time, I will alter the test to also test the psql backslash commands based on how the copy equivalents are tested, since I had forgotten them and they need to be tested also. -- Any sufficiently advanced technology is indistinguishable from a rigged demo. ---(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
Re: [PATCHES] [HACKERS] large object regression tests
On Sun, 24 Sep 2006, Jeremy Drake wrote: On Thu, 21 Sep 2006, Tom Lane wrote: I suggest that instead of testing the server-side lo_import/lo_export functions, perhaps you could test the psql equivalents and write and read a file in psql's working directory. I did not see any precedent for that when I was looking around in the existing tests for an example of how to do things. snip When I was looking at the copy tests, it looked like the server-side ones were tested, and then the psql ones were tested by exporting and then importing data which was originally loaded from the server-side method. I just went back and looked at the tests again. The only time the psql \copy command was used was in the (quite recent IIRC) copyselect test, and then only via stdout (never referring to psql working directory, or to files at all). Did I misunderstand, and you are proposing a completely new way of doing things in the regression tests? I am not particularly fond of the sed substitution stuff myself, but it seems to be the only currently supported/used method in the regression tests... I do think that making the large object test and the copy test consistent would make a lot of sense, since as I said before, the functionality of file access is so similar... -- We demand rigidly defined areas of doubt and uncertainty! -- Vroomfondel ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] large object regression tests
I put together a patch which adds a regression test for large objects, hopefully attached to this message. I would like some critique of it, to see if I have gone about it the right way. Also I would be happy to hear any additional tests which should be added to it. On Tue, 5 Sep 2006, Jeremy Drake wrote: I noticed when I was working on a patch quite a while back that there are no regression tests for large object support. I know, large objects are not the most sexy part of the code-base, and I think they tend to be ignored/forgotten most of the time. Which IMHO is all the more reason they should have some regression tests. Otherwise, if someone managed to break them somehow, it is quite likely not to be noticed for quite some time. So in this vein, I have recently found myself with some free time, and a desire to contribute something, and decided this would be the perfect place to get my feet wet without stepping on any toes. I guess what I should ask is, would a patch to add a test for large objects to the regression suite be well received? And, is there any advice for how to go about making these tests? I am considering, and I think that in order to get a real test of the large objects, I would need to load data into a large object which would be sufficient to be loaded into more than one block (large object blocks were 1 or 2K IIRC) so that the block boundary case could be tested. Is there any precedent on where to grab such a large chunk of data from? I was thinking about using an excerpt from a public domain text such as Moby Dick, but on second thought binary data may be better to test things with. My current efforts, and probably the preliminary portion of the final test, involves loading a small amount (less than one block) of text into a large object inline from a sql script and calling the various functions against it to verify that they do what they should. In the course of doing so, I find that it is necessary to stash certain values across statements (large object ids, large object 'handles'), and so far I am using a temporary table to store these. Is this reasonable, or is there a cleaner way to do that? -- One seldom sees a monument to a committee. largeobj-regress.patch.gz Description: Binary data ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] ADD/DROPS INHERIT (actually INHERIT / NO INHERIT)
On Sun, 2 Jul 2006, Tom Lane wrote: Nah, it was a false alarm: I was looking at the first post-patch report, http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mongoosedt=2006-07-02%2003:30:01 but apparently mongoose had managed to pick up a partially-updated snapshot. The later reports (including mongoose's own next try an hour later) were all OK. As the keeper of mongoose, is there anything I should do to prevent it from picking up a partially-updated snapshot? Or is this just a race condition that's bound to happen now and then? ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] patch to have configure check if CC is intel C compiler
On Sat, 22 Apr 2006, Tom Lane wrote: Jeremy Drake [EMAIL PROTECTED] writes: On Fri, 21 Apr 2006, Tom Lane wrote: Yeah. NaN == 0 is just silly ... From what I can tell from the instruction set docs and test programs, the actual bug/misoptimization is that NaN == anything. Which is even sillier. It's been a very long time since I studied the IEEE arithmetic spec, but my recollection is that comparisons involving NaN are supposed to yield the result unordered, which is not any of the normal possibilities less, equal, or greater. The problem for C compiler authors is how to wedge that behavior into a language that only admits the three normal possibilities. It sounds like the ICC authors have decided that it's OK to behave randomly, ie, not check for unordered at all, by default. I suspect that whether NaN appears to be equal or unequal or less or greater depends tremendously on the details of the assembly code the compiler chances to emit (ie, which arm of the IF it chooses to branch to instead of fall through to). So basically, the default behavior is completely unusable in programs that allow NaN to appear in their computations. Not quite. The C99 standard specifies that all compares with NaN are false (rather like sql null). But with the code the intel compiler generates, all three flags ZF, CF, and PF are set. This means that all numbers are greater AND equal to NaN unless the parity flag is checked (this indicates an unordered result). But that behavior is decidedly nonstandard, and thus completely unusable in portable code. It would probably be better described as arbitrary rather than random though. Given that we've already got a test for ICC in there as of today, I'd vote for adding -mp1 to CFLAGS if we see it's ICC. BTW, does anyone want to set up a buildfarm member running ICC? Yeah, I was planning to set one up once that patch was committed. I was just getting the stuff together on my box to run it. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend -- FORTUNE EXPLAINS WHAT JOB REVIEW CATCH PHRASES MEAN:#9 has management potential: Because of his intimate relationship with inanimate objects, the reviewee has been appointed to the critical position of department pencil monitor. inspirational: A true inspiration to others. (There, but for the grace of God, go I.) adapts to stress: Passes wind, water, or out depending upon the severity of the situation. goal oriented: Continually sets low goals for himself, and usually fails to meet them. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] patch to have configure check if CC is intel C compiler
If configure sees that the compiler specified by $CC looks like gcc (defines __GNUC__), then it puts some extra command line options into the CFLAGS (mostly -W*). The intel C compiler for linux emulates gcc by default, which means it defines that and looks very much like gcc to configure. However, it does not get along with the added -W flags very well. They don't seem to kill it, but some of them give warnings about unsupported command line options and others produce insane amounts of output from the compiler. This patch makes configure check for the __INTEL_COMPILER define (which is the recommended way to detect the intel compiler) before adding the extra CFLAGS if it thinks the compiler is GCC. I am not an autoconf hacker, so I may have done it wrong or something, but it appears to work for me (ICC 9.0.032 on gentoo i686 with latest packages). The patch is against the HEAD but I think it should be similar for other branches (I have been compiling 8.0 and 8.1 for some time with the intel compiler by manually massaging the makefiles after configure ran). -- We the unwilling, led by the ungrateful, are doing the impossible. We've done so much, for so long, with so little, that we are now qualified to do something with nothing.Index: configure.in === RCS file: /home/jeremyd/local/postgres/cvsuproot/pgsql/configure.in,v retrieving revision 1.455 diff -c -r1.455 configure.in *** configure.in6 Mar 2006 17:41:43 - 1.455 --- configure.in1 Apr 2006 17:43:19 - *** *** 249,260 fi if test $GCC = yes; then ! CFLAGS=$CFLAGS -Wall -Wmissing-prototypes -Wpointer-arith -Winline ! ! # Some versions of GCC support some additional useful warning flags. ! # Check whether they are supported, and add them to CFLAGS if so. ! PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) ! PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) # Disable strict-aliasing rules; needed for gcc 3.3+ PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing]) --- 249,265 fi if test $GCC = yes; then ! AC_TRY_COMPILE([], [EMAIL PROTECTED]:@ifndef __INTEL_COMPILER ! choke me ! @%:@endif], [ICC=[yes]], [ICC=[no]]) ! if test $ICC = no; then ! CFLAGS=$CFLAGS -Wall -Wmissing-prototypes -Wpointer-arith -Winline ! ! # Some versions of GCC support some additional useful warning flags. ! # Check whether they are supported, and add them to CFLAGS if so. ! PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) ! PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) ! fi # Disable strict-aliasing rules; needed for gcc 3.3+ PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing]) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] patch to have configure check if CC is intel C compiler
On Sun, 2 Apr 2006, Peter Eisentraut wrote: Jeremy Drake wrote: The intel C compiler for linux emulates gcc by default, which means it defines that and looks very much like gcc to configure. However, it does not get along with the added -W flags very well. They don't seem to kill it, but some of them give warnings about unsupported command line options and others produce insane amounts of output from the compiler. Details please. Which options are unsupported and what happens if you use them? For an example I grabbed the output from compiling nbtsearch.c using this configure line: CC=icc CFLAGS=-O3 -ip -parallel -xN ./configure --with-perl --with-python --with-openssl As you can tell, these make a lot of output. I only include the line(s) with each option which are added when the option is given. So, with the old configure, all of the warnings were concatenated (two instances of the argument required warning plus the inlining report and the collection of remark messages). -Wmissing-prototypes and -Wpointer-arith do not appear to make any difference, at least on this file. -Wendif-labels: iccbin: Command line warning: ignoring option '-W'; no argument required -Wdeclaration-after-statement: iccbin: Command line warning: ignoring option '-W'; no argument required -Winline: INLINING REPORT: (_bt_moveright) - elog_finish(EXTERN) - elog_start(EXTERN) - _bt_relandgetbuf(EXTERN) - ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) INLINING REPORT: (_bt_binsrch) - ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) INLINING REPORT: (_bt_next) - _bt_relbuf(EXTERN) - _bt_checkkeys(EXTERN) - ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) INLINING REPORT: (_bt_first) - ARGS_IN_REGS: _bt_endpoint(9) (isz = 475) (sz = 489 (176+313)) - INLINE: _bt_next(11) (isz = 59) (sz = 73 (27+46)) - ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) - _bt_checkkeys(EXTERN) - _bt_relbuf(EXTERN) - _bt_relbuf(EXTERN) - _bt_checkkeys(EXTERN) - ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) - ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) - BufferGetBlockNumber(EXTERN) - INLINE: _bt_binsrch(15) (isz = 104) (sz = 127 (41+86)) - ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) - _bt_freestack(EXTERN) - ARGS_IN_REGS: _bt_search.(0) (isz = 315) (sz = 335 (113+222)) - elog_finish(EXTERN) - elog_start(EXTERN) - memcpy(EXTERN) - memcpy(EXTERN) - ScanKeyEntryInitializeWithInfo(EXTERN) - index_getprocinfo(EXTERN) - ScanKeyEntryInitialize(EXTERN) - get_opclass_proc(EXTERN) - _bt_preprocess_keys(EXTERN) INLINING REPORT: (_bt_step) - BufferGetBlockNumber(EXTERN) - _bt_relbuf(EXTERN) - _bt_relandgetbuf(EXTERN) - INLINE: _bt_walk_left(10) (isz = 210) (sz = 222 (77+145)) - BufferGetBlockNumber(EXTERN) - _bt_relandgetbuf(EXTERN) - _bt_relandgetbuf(EXTERN) - _bt_relandgetbuf(EXTERN) - elog_start(EXTERN) - elog_finish(EXTERN) - elog_start(EXTERN) - elog_finish(EXTERN) - _bt_relandgetbuf(EXTERN) - _bt_relbuf(EXTERN) INLINING REPORT: (_bt_search) - _bt_relandgetbuf(EXTERN) - memcpy(EXTERN) - MemoryContextAlloc(EXTERN) - BufferGetBlockNumber(EXTERN) - INLINE: _bt_binsrch(14) (isz = 104) (sz = 127 (41+86)) - ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) - INLINE: _bt_moveright(16) (isz = 92) (sz = 110 (37+73)) - ARGS_IN_REGS: _bt_compare.(3) (isz = 249) (sz = 269 (77+192)) - _bt_relandgetbuf(EXTERN) - elog_start(EXTERN) - elog_finish(EXTERN) - _bt_getroot(EXTERN) INLINING REPORT: (_bt_compare) - FunctionCall2(EXTERN) - nocache_index_getattr(EXTERN) - nocache_index_getattr(EXTERN) INLINING REPORT: (_bt_endpoint) - INLINE: _bt_next(12) (isz = 59) (sz = 73 (27+46)) - ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) - _bt_checkkeys(EXTERN) - _bt_relbuf(EXTERN) - _bt_relbuf(EXTERN) - _bt_checkkeys(EXTERN) - ARGS_IN_REGS: _bt_step.(6) (isz = 486) (sz = 502 (171+331)) - elog_finish(EXTERN) - elog_start(EXTERN) - BufferGetBlockNumber(EXTERN) - INLINE: _bt_get_endpoint(13) (isz = 199) (sz = 213 (76+137)) - _bt_gettrueroot(EXTERN) - _bt_getroot(EXTERN) - elog_start(EXTERN) - elog_finish(EXTERN) - _bt_relandgetbuf(EXTERN) - elog_start(EXTERN) - elog_finish(EXTERN) - _bt_relandgetbuf(EXTERN) INLINING REPORT: (_bt_get_endpoint) - _bt_relandgetbuf(EXTERN) - elog_finish(EXTERN) - elog_start(EXTERN) - _bt_relandgetbuf(EXTERN) - elog_finish(EXTERN) - elog_start(EXTERN) - _bt_getroot(EXTERN) - _bt_gettrueroot(EXTERN) -Wall: ../../../../src/include/access/relscan.h(45): remark #1684: conversion from pointer to same-sized integral type (potential portability problem) OffsetNumber rs_vistuples[MaxHeapTuplesPerPage];/* their offsets
