[PATCHES] pg_typeof() (was: Mysterious Bus Error with get_fn_expr_argtype())
Hi folks, As discussed on -hackers [1], here is a patch to add a pg_typeof() builtin function to core. The function accepts one argument (type any) and returns the regtype of that argument. This can be helpful in various circumstances, including troubleshooting cast/coercion behaviour in a query, or constructing dynamic SQL statements. It's declared in builtins.h and defined in utils/adt/misc.c. The patch includes a small documentation update; I added pg_typeof() to Table 9-47. System Catalog Information Functions, and a brief descriptive paragraph underneath the table. This didn't seem like it warranted any additional regression tests. Added to the November commitfest. Cheers, BJ doc/src/sgml/func.sgml | 15 +++ src/backend/utils/adt/misc.c |9 + src/include/catalog/catversion.h |2 !! src/include/catalog/pg_proc.h|2 ++ src/include/utils/builtins.h |1 + 5 files changed, 27 insertions(+), 2 modifications(!) [1] http://archives.postgresql.org/message-id/[EMAIL PROTECTED] pg_typeof.diff Description: Binary data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] to_date() validation
Hi all,
Well, it's been a long time coming, but I've finally got a patch to
improve the error handling of to_date() and to_timestamp(), as
previously discussed on -hackers. [1][2]
The patch is against HEAD, and compiles cleanly and passes all
regression tests on gentoo amd64.
I did some testing to see whether the extra error checks had an
adverse effect on performance. It turns out that my patch actually
performs materially *better*, somehow. About 14.5% better over 1M
calls to to_date in a single statement. Go figure (C compiler
optimisations are completely arcane to me).
What the patch does
With this patch I've tried to make DCH_from_char a bit smarter with
regard to parsing and validation of the user input. The goal here is
that if the user makes a mistake and enters something invalid,
to_date() will throw an meaningful error instead of just acting like
everything is okay and then producing an absurd answer.
I've modified the code to error out usefully in a few different scenarios:
* Non-integer value for an integer field
* Out-of-range value for an integer field
* Not enough characters in the input string to provide for all the
format fields
* Two conflicting values given for a field
* Illegal mixture of date conventions (ISO week date and Gregorian)
I've included new regression tests to check that these errors are
occurring when appropriate.
How I went about it
In order to detect an illegal mixture of Gregorian and ISO week date
conventions, the code needs to know which fomatting fields are
Gregorian, which are ISO WD and which are date-convention-neutral. To
get this working I added a new enum called FromCharDateMode, and added
a field to both KeyWord and TmFromChar to hold the date mode. In
KeyWord, it tells you which convention the keyword belongs to ( is
Gregorian, IYYY is ISO WD and HH24 has nothing to do with dates). In
TmFromChar it keeps track of which convention is currently in use.
Because TmFromChar now tracks the date mode independently, it is not
necessary to maintain separate fields for the ISO WD values, so
'iyear' and 'iw' are removed.
In order to get some consistent treatment on pulling integers out of a
string and stuffing them into a TmFromChar, I had to refactor
DCH_from_char somewhat. The function is basically a giant switch
statement, with branches for each of the formatting fields. In HEAD
these branches are highly duplicative. The integer fields all perform
a sscanf() on the source string, putting the result straight into the
TmFromChar struct, and then skipping over the number of characters
consumed by sscanf(). I moved this logic into functions:
* from_char_parse_int() uses strtol() rather than sscanf() to parse
an integer from the string, and produces meaningful errors if
something is wrong with the input,
* from_char_set_int() saves an integer into a TmFromChar field, but
throws an error if the field has already been set to some other
non-zero value.
What the patch doesn't do
It doesn't consider to_number() at all. I don't have any interest in
the number formatting stuff.
There is plenty of room to educate do_to_timestamp() further about
dates; it's still very naive. Currently you can do something like
to_date('2008-50', '-MM') and it won't even bat an eyelid. It'll
just advance the date by 50 months. I suppose you could consider that
a bug or a feature, depending on your point of view. Personally, I
think of it as a bug. If you're giving a month value outside of 1-12
to to_date(), chances are good that it's a user error in the query,
not a deliberate attempt to perform some date arithmetic.
do_to_timestamp() also doesn't try to detect logical mistakes like
'-MM-DD DDD' where the DDD part conflicts with the MM-DD part. I
have some thoughts about how I would improve on that, but I think it's
best left for a separate patch.
The patch has been added to the September commitfest. Thanks for your time.
Cheers,
BJ
[1] http://archives.postgresql.org/pgsql-hackers/2007-02/msg00915.php
[2] http://archives.postgresql.org/message-id/[EMAIL PROTECTED]
to-date-validation-1.diff.gz
Description: GNU Zip compressed data
--
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] printTable API (was: Show INHERIT in \du)
Hi guys, Here's the latest version of the printTable API. This version is against the current HEAD and merges in the changes made by the recently committed psql wrap patch. This version also includes Alvaro's fix for the issue of pg_strdup not being available to programs in scripts/ (as quoted below). Clean compile and all regression tests passed on amd64 gentoo. Cheers, BJ On Thu, May 8, 2008 at 7:55 AM, Alvaro Herrera [EMAIL PROTECTED] wrote: FWIW I just noticed something else. With this patch we add pg_strdup calls into print.c. pg_strdup lives in common.c. This is fine as psql is concerned, but we have another problem which is that in bin/scripts there are two programs that want to use printQuery(). The problem is that there's no pg_strdup there :-( The easy solution is to add pg_strdup to bin/scripts/common.c, but there we don't have a global progname, so the error report in the out of memory case cannot carry the name of the program crashing. I don't like that, but I don't see any other solution. Ideas welcome. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support print-table-5.diff.bz2 Description: BZip2 compressed data -- 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] [HACKERS] Multiline privileges in \z
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sun, May 4, 2008 at 10:55 AM, Tom Lane wrote: Andrew Dunstan writes: Wouldn't this expression: pg_catalog.array_to_string(c.relacl, chr(10)) be better expressed as pg_catalog.array_to_string(c.relacl, E'\n') +1 ... it's minor, but knowing that ASCII LF is 10 is probably not wired into too many people's brains anymore. (Besides, some of us remember it as octal 12, anyway...) Fair enough. I just wanted a non-messy way of saying 1 newline, please, and I wasn't too sure whether backslash escapes were considered kosher for internal queries. But you're right, readability is important. No objections to using the escape syntax. Please note that I used chr(10) in my \du attributes patch as well. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIHjNA5YBsbHkuyV0RAuNSAJ0ZDLxhHaPj4CBsBCILnxHy+5Jf5ACfQHMH 4XZxczc+YEow3AFdayn9fGs= =+TSV -END PGP SIGNATURE- -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] ADD COLUMN with PRIMARY KEY bug (was: I think this is a BUG?)
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Hi,
The attached patch resolves the bug reported by Kaloyan Iliev [1] on -general.
The problem occurred when executing ALTER TABLE ... ADD COLUMN ...
PRIMARY KEY with no default clause, on a table with rows already
present. The NOT NULL constraint was not enforced. The bug has been
observed on (at least) 8.2, 8.3 and CVS HEAD.
The fix was to force evaluation of the NOT NULL constraint in
ATExecAddColumn in all cases where the parser indicated that the
column ought to be NOT NULL.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org
iD8DBQFIENpl5YBsbHkuyV0RApP6AKDdnZAA0LjSzh9XdiQt7K/cx4YOcQCgydHr
6BYSD2kVX2DP7LYgDHvzNdE=
=Yrmp
-END PGP SIGNATURE-
On Fri, Apr 25, 2008 at 3:46 AM, Tom Lane [EMAIL PROTECTED] wrote:
alter table t1 add column f2 int not null;
transformAlterTableStmt will produce an AT_AddColumn subcommand
containing a ColumnDef with is_not_null = false, followed by an
AT_SetNotNull subcommand. But for
I realised that there's no reason for preparing a separate SetNotNull
subcommand anymore, now that ATExecAddColumn takes care of enforcing
the constraint, so I removed this special case.
I've also added a regression test for ADD COLUMN PRIMARY KEY.
Cheers,
BJ
[1] http://archives.postgresql.org/message-id/[EMAIL PROTECTED]
*** src/backend/commands/tablecmds.c
--- src/backend/commands/tablecmds.c
***
*** ,3338 ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
--- ,3345
}
/*
+* Tell Phase 3 to perform the check for NULL values in the new column
if
+* its attnotnull bit has been set to true.
+*/
+ if (attribute-attnotnull)
+ tab-new_notnull = true;
+
+ /*
* Add needed dependency entries for the new column.
*/
add_column_datatype_dependency(myrelid, i, attribute-atttypid);
*** src/backend/parser/parse_utilcmd.c
--- src/backend/parser/parse_utilcmd.c
***
*** 1726,1753 transformAlterTableStmt(AlterTableStmt *stmt, const char
*queryString)
* If the column has a non-null
default, we can't skip
* validation of foreign keys.
*/
! if (((ColumnDef *)
cmd-def)-raw_default != NULL)
skipValidation = false;
newcmds = lappend(newcmds, cmd);
/*
-* Convert an ADD COLUMN ... NOT NULL
constraint to a
-* separate command
-*/
- if (def-is_not_null)
- {
- /* Remove NOT NULL from
AddColumn */
- def-is_not_null = false;
-
- /* Add as a separate
AlterTableCmd */
- newcmd =
makeNode(AlterTableCmd);
- newcmd-subtype = AT_SetNotNull;
- newcmd-name =
pstrdup(def-colname);
- newcmds = lappend(newcmds,
newcmd);
- }
-
- /*
* All constraints are processed in
other ways. Remove the
* original list
*/
--- 1726,1737
* If the column has a non-null
default, we can't skip
* validation of foreign keys.
*/
! if (def-raw_default != NULL)
skipValidation = false;
newcmds = lappend(newcmds, cmd);
/*
* All constraints are processed in
other ways. Remove the
* original list
*/
*** src/test/regress/expected/alter_table.out
--- src/test/regress/expected/alter_table.out
***
*** 505,510 create table atacc1 ( test int );
--- 505,522
alter table atacc1 add constraint atacc_test1 primary key (test1);
ERROR: column test1 named in key does not exist
drop table atacc1;
+ -- Adding a new column as primary key to a non-empty table. Should fail
unless
+ -- the column has a default value.
+ create table atacc1 ( test int );
+ insert into atacc1
Re: [PATCHES] printTable API (was: Show INHERIT in \du)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 9:53 AM, Alvaro Herrera wrote: Well, consider that there are going to be 1 or 2 entries in the arrays in most cases anyway :-) Well, as far as footers go anyway ... I just realized that in all other cases it will certainly be the wrong thing to do :-) Still, perhaps a integer count is better? I oscillated between using an integer and a pointer for -header and - -cell while I was writing the code. In the end I went with pointer simply because it makes the iterations nicer looking, and there's a symmetry in having the first item and last item pointers be the same type. If there's some technical reason that integers would be better, I'll all ears, but from an aesthetic/code readability perspective I like the pointer. Brendan Jurd escribió: What is it about the extra fields that makes you unhappy? I don't know if unnecessarity is a word, but I hope you get what I mean :-) Since the footers list is usually pretty short, you could make an argument for dropping the last footer pointer and just iterating through the list to find the last element every time you want to do something with footers. And to do that, you'd have to declare a temporary pointer inside the AddFooter function anyway, so you're not getting rid of the pointer so much as making it slightly more transient. All that having been said, C isn't my native language. So if the pointer is indeed wasteful I'm happy to cede to the wisdom of more experienced C coders. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBw7m5YBsbHkuyV0RAv4IAJ0cKrziZpNWkVV7LxFhlV/V5L0pJACfUtZ+ cVbcjL1e89j21JDZJBVdBqw= =Nzr+ -END PGP SIGNATURE- -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Multiline privileges in \z
Hi hackers,
It occurred to me that psql's \z command could benefit from the
addition of some newlines. With any more than one grantee per object,
the output of \z rapidly becomes extremely wide, and very hard to
read.
I'd like to split the output onto one line per grantee. So, instead of this:
Schema | Name | Type |Access privileges
+--+---+-
public | a| table |
{brendanjurd=arwdxt/brendanjurd,foo=arwd/brendanjurd,bar=r/brendanjurd}
public | b| table | {brendanjurd=arwdxt/brendanjurd,foo=arwd/brendanjurd}
(2 rows)
You would get this:
Schema | Name | Type | Access privileges
+--+---+
public | a| table | brendanjurd=arwdxt/brendanjurd
: foo=arwd/brendanjurd
: bar=r/brendanjurd
public | b| table | brendanjurd=arwdxt/brendanjurd
: foo=arwd/brendanjurd
(2 rows)
Because the -BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
ACLs
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org
iD8DBQFIB3kL5YBsbHkuyV0RAgWQAJ9bcl3bOFozvi9LxRAQN1OwT3t+QgCcCGVq
dcMw3wIBQVPv1nYDBCSRpDA=
=s1eD
-END PGP SIGNATURE-
are stored as an array, the patch to achieve this is trivial (see attached).
Looking forward to your comments.
Added to wiki.
Cheers,
BJ
*** src/bin/psql/describe.c
--- src/bin/psql/describe.c
***
*** 482,488 permissionsList(const char *pattern)
SELECT n.nspname as \%s\,\n
c.relname as \%s\,\n
CASE c.relkind WHEN 'r' THEN '%s'
WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \%s\,\n
! c.relacl as \%s\\n
FROM pg_catalog.pg_class c\n
LEFT JOIN pg_catalog.pg_namespace n ON n.oid =
c.relnamespace\n
WHERE c.relkind IN ('r', 'v',
'S')\n,
--- 482,488
SELECT n.nspname as \%s\,\n
c.relname as \%s\,\n
CASE c.relkind WHEN 'r' THEN '%s'
WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \%s\,\n
! array_to_string(c.relacl, chr(10))
as \%s\\n
FROM pg_catalog.pg_class c\n
LEFT JOIN pg_catalog.pg_namespace n ON n.oid =
c.relnamespace\n
WHERE c.relkind IN ('r', 'v',
'S')\n,
*** src/test/regress/expected/dependency.out
--- src/test/regress/expected/dependency.out
***
*** 68,86 NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
deptest_pkey fo
GRANT ALL ON deptest1 TO regression_user2;
RESET SESSION AUTHORIZATION;
\z deptest1
! Access privileges
for database regression
! Schema | Name | Type |
Access privileges
!
+--+---+
! public | deptest1 | table |
{regression_user0=arwdxt/regression_user0,regression_user1=a*r*w*d*x*t*/regression_user0,regression_user2=arwdxt/regression_user1}
(1 row)
DROP OWNED BY regression_user1;
-- all grants revoked
\z deptest1
! Access privileges for database regression
! Schema | Name | Type | Access privileges
! +--+---+
! public | deptest1 | table | {regression_user0=arwdxt/regression_user0}
(1 row)
-- table was dropped
--- 68,88
GRANT ALL ON deptest1 TO regression_user2;
RESET SESSION AUTHORIZATION;
\z deptest1
! Access privileges for database regression
! Schema | Name | Type | Access privileges
! +--+---+
! public | deptest1 | table | regression_user0=arwdxt/regression_user0
!: regression_user1=a*r*w*d*x*t*/regression_user0
!: regression_user2=arwdxt/regression_user1
(1 row)
DROP OWNED BY regression_user1;
-- all grants revoked
\z deptest1
! Access privileges for database regression
! Schema | Name | Type |Access privileges
! +--+---+--
! public | deptest1 | table | regression_user0=arwdxt/regression_user0
(1 row)
-- table was dropped
--
Sent
Re: [PATCHES] [HACKERS] Multiline privileges in \z
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On Fri, Apr 18, 2008 at 2:37 AM, Tom Lane wrote:
The function names in the patch need schema-qualification in case
pg_catalog is not first in the search path.
Ah, yes. I should have seen that. Thanks Tom.
New version attached with schema-qualification.
Cheers,
BJ
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org
iD8DBQFIB4Ae5YBsbHkuyV0RAqJVAJ9+h6wZrLT9YFRw3s2E742sg7Yr4wCgvtcq
xK7cTnbiGtfpGGYw5WP4asI=
=hf8W
-END PGP SIGNATURE-
*** src/bin/psql/describe.c
--- src/bin/psql/describe.c
***
*** 482,488 permissionsList(const char *pattern)
SELECT n.nspname as \%s\,\n
c.relname as \%s\,\n
CASE c.relkind WHEN 'r' THEN '%s'
WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \%s\,\n
! c.relacl as \%s\\n
FROM pg_catalog.pg_class c\n
LEFT JOIN pg_catalog.pg_namespace n ON n.oid =
c.relnamespace\n
WHERE c.relkind IN ('r', 'v',
'S')\n,
--- 482,488
SELECT n.nspname as \%s\,\n
c.relname as \%s\,\n
CASE c.relkind WHEN 'r' THEN '%s'
WHEN 'v' THEN '%s' WHEN 'S' THEN '%s' END as \%s\,\n
!
pg_catalog.array_to_string(c.relacl, chr(10)) as \%s\\n
FROM pg_catalog.pg_class c\n
LEFT JOIN pg_catalog.pg_namespace n ON n.oid =
c.relnamespace\n
WHERE c.relkind IN ('r', 'v',
'S')\n,
*** src/test/regress/expected/dependency.out
--- src/test/regress/expected/dependency.out
***
*** 68,86 NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
deptest_pkey fo
GRANT ALL ON deptest1 TO regression_user2;
RESET SESSION AUTHORIZATION;
\z deptest1
! Access privileges
for database regression
! Schema | Name | Type |
Access privileges
!
+--+---+
! public | deptest1 | table |
{regression_user0=arwdxt/regression_user0,regression_user1=a*r*w*d*x*t*/regression_user0,regression_user2=arwdxt/regression_user1}
(1 row)
DROP OWNED BY regression_user1;
-- all grants revoked
\z deptest1
! Access privileges for database regression
! Schema | Name | Type | Access privileges
! +--+---+
! public | deptest1 | table | {regression_user0=arwdxt/regression_user0}
(1 row)
-- table was dropped
--- 68,88
GRANT ALL ON deptest1 TO regression_user2;
RESET SESSION AUTHORIZATION;
\z deptest1
! Access privileges for database regression
! Schema | Name | Type | Access privileges
! +--+---+
! public | deptest1 | table | regression_user0=arwdxt/regression_user0
!: regression_user1=a*r*w*d*x*t*/regression_user0
!: regression_user2=arwdxt/regression_user1
(1 row)
DROP OWNED BY regression_user1;
-- all grants revoked
\z deptest1
! Access privileges for database regression
! Schema | Name | Type |Access privileges
! +--+---+--
! public | deptest1 | table | regression_user0=arwdxt/regression_user0
(1 row)
-- table was dropped
--
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] [HACKERS] Text - C string
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 - -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sat, Mar 29, 2008 at 5:40 AM, Brendan Jurd wrote: On 29/03/2008, Tom Lane wrote: I intentionally didn't touch xml.c, nor anyplace that is not dealing in text, even if it happens to be binary-compatible with text. Hmm, okay. My original submission did include a few such changes; for example, in xml_in and xml_out_internal I saw that the conversion between xmltype and cstring was identical to the text conversion, so I went ahead and used the text functions. Feedback upthread suggested that it was okay to go ahead with casting in identical cases. [1] I saw that these changes made it into the commit, so I assumed that it was the right call. If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Still not sure where we stand on the above. To cast, or not to cast? Cheers, BJ - -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBaFy5YBsbHkuyV0RAsMmAKDHaEb7aMudKJgVxcf5RRcOtAJ+bwCgivLI 5B3xJze46NGWjEyOpq/TSaY= =BObd - -END PGP SIGNATURE- -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBaIl5YBsbHkuyV0RArDDAJ0QThLXAhzy+NX+2YsF+q4z/sIy1QCeJPiW s/rVns3FFQVP5g9DTOshDfQ= =4Tdh -END PGP SIGNATURE- -- 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] Reference by output in : \d table_name
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Wed, Apr 16, 2008 at 10:00 PM, kenneth d'souza wrote: However,Why does the word FOREIGN KEY appear in the last line of your output. My original patch had the output like this. Referenced by: bar_foo_fkey IN public.bar(foo) REFERENCES foo(a) The keyword FOREIGN KEY was removed by me as it would further cause a confusion. Hi Kenneth, Tom reinstated the FOREIGN KEY part of the definition when he committed your patch. I think it's fine with FOREIGN KEY left in there; I don't find it confusing. I actually think it makes the line more descriptive and obvious. Secondly, since the table foo is altered with an addition of a new column bar, it doesn't display in your output. Please double check. You're right; it was just a copy-paste error I made when I was composing my email. The actual output from psql shows all columns as expected. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBixv5YBsbHkuyV0RArJSAJ0eDes2V0nwlgQuNE0GjJxwW4Ey8gCgiWTw UjSjF8EJaTDSTQnkTfgSasY= =xdOl -END PGP SIGNATURE- -- 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] [HACKERS] Text - C string
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 1:16 AM, Tom Lane wrote: Brendan Jurd writes: If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Still not sure where we stand on the above. To cast, or not to cast? I dunno. I know there was previously some handwaving about representing XML values in some more intelligent fashion than a plain text string, but I have no idea if anyone is likely to do anything about it in the foreseeable future. Well ... if somebody does want to change the representation of xml down the road, he's going to have to touch all the sites where the code converts to and from cstring anyway, right? So the only real difference this will make is that, instead of having to replace four lines of VARDATA/memcpy per site, he'll have to replace a single function call. That doesn't seem like a negative. With that in mind, please find attached my followup patch. It cleans up another 21 sites manually copying between cstring and varlena, for a net reduction of 115 lines of code. I didn't attempt to work through every reference to VARDATA, but I did look at every hit from a `grep -Rn VARDATA . | grep memcpy`. All regression tests passed (contrib tests included) on gentoo. Patch added to commitfest queue. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBm105YBsbHkuyV0RAmqfAKCfyNyFdciqX4QV81sG9MhPt+KXuACfe694 3d/ICZF6yqV6K20X3TVX+So= =CvRM -END PGP SIGNATURE- text-cstring-followup.diff.bz2 Description: BZip2 compressed data -- 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] printTable API (was: Show INHERIT in \du)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 7:27 AM, Alvaro Herrera wrote: Brendan Jurd escribió: The second version of the patch is attached. Thanks. I looked the patch over and did some minor changes. Modified version attached. Cool, I had a look through your changes and they all seemed fine to me. In particular, moving the header comments to the ... header ... file seemed like a smart move. =) One thing I'm not entirely happy about is the fact that we need a pointer to the last footer/cell/header, so two pointers for each element kind. Well, the alternative is iterating through the array each time you want to add something until you hit a NULL pointer, and adding the new item at that point. Considering we're only chewing up an extra 4 * sizeof(pointer) = 16 bytes in the struct, it seems like a reasonable price to pay for the convenience. What is it about the extra fields that makes you unhappy? Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBo2y5YBsbHkuyV0RAmLhAJ9CP/9L1Nv7WbCtrCYu6tyoGhQItQCeMRm0 HegSSBq8tXw43Kj2xeQ2RCs= =RvzP -END PGP SIGNATURE- -- 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] [HACKERS] Show INHERIT in \du
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sun, Apr 13, 2008 at 6:11 PM, Brendan Jurd wrote: I've written a patch which implements the same \du behaviour as my previous patch, but using the new printTable API New versions of this patch, including changes made to the printTable API in [1] As before, I've included a patch against HEAD which includes the printTable changes (du-attributes-print-table_2.diff.bz2), and a patch against my printTable branch which shows just the changes to describeRoles (du-attributes_2.diff.bz2). Cheers, BJ [1] http://archives.postgresql.org/message-id/[EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBGZP5YBsbHkuyV0RAsb9AKDrFJ/8V00t2XCwIihzEZYcPQZKiQCg3q6L RkiMfjqLay/JLk8phnniYLs= =oW6S -END PGP SIGNATURE- du-attributes-print-table_2.diff.bz2 Description: BZip2 compressed data du-attributes_2.diff.bz2 Description: BZip2 compressed data -- 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] Reference by output in : \d table_name
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Mon, Apr 14, 2008 at 11:42 PM, Tom Lane wrote: Brendan Jurd writes: Sorry Kenneth, I didn't quite follow your explanation. How does the position of FOREIGN KEY affect the indentation at the beginning of the footer? I changed that patch around a bit while applying it, and very possibly fat-fingered the indentation FWIW, I think the deviant indentation was present in the original patch submitted by Kenneth. I compared your commit with the patch, and the initial two-space indent is clearly there in both diffs. --- I don't recall having explicitly compared it to the other cases. It surely should be consistent with everything else. Yeah, that's what I figured. The patch I attached to my previous email should fix it up. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIA2J55YBsbHkuyV0RAjpxAKCy+pzCV8h88k5PcdwD8ik86Ka1PACffVOO K2rItDy0yWlvTxZpArpXU0o= =vLyi -END PGP SIGNATURE- -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Removing typename from A_Const (was: Empty arrays with ARRAY[])
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Fri, Mar 21, 2008 at 7:47 AM, Tom Lane wrote: I didn't do anything about removing A_Const's typename field, but I'm thinking that would be a good cleanup patch. Here's my attempt to remove the typename field from A_Const. There were a few places (notably flatten_set_variable_args() in guc.c, and typenameTypeMod() in parse_type.c) where the code expected to see an A_Const with a typename, and I had to adjust for an A_Const within a TypeCast. Nonetheless, there was an overall net reduction of 34 lines of code, so I think this was a win. All regression tests passed on x86_64 gentoo. Added to May CommitFest. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIA8s/5YBsbHkuyV0RAlMdAJ0dWdoZd5cypvInAR2msO8XA8qqxACeILSw bCI2TGAQI3m3TBoJspvV4OQ= =dGP9 -END PGP SIGNATURE- aconst-no-typename_0.diff.bz2 Description: BZip2 compressed data -- 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] [HACKERS] Show INHERIT in \du
On Tue, Mar 25, 2008 at 2:41 AM, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: This makes me wonder whether print.c could offer something a bit more helpful to callers wishing to DIY a table; we could have a table-building struct with methods like addHeader and addCell. What do you think? Overkill, or worthy pursuit? Once you have two occurrences of a pattern, it's reasonable to assume there will be more later. +1 for building a little bit of infrastructure. I've written a patch which implements the same \du behaviour as my previous patch, but using the new printTable API I submitted in [1]. If the printTable API patch is rejected or substantially changed, we will need to revisit this patch. The new patch constructs a table manually, in the same manner as describeOneTableDetails, so that we get the same outputs as the original patch but without any of the localisation issues identified by Tom and Alvaro. I have attached a patch against my printTable code, containing only the changes I made to describeRoles() (du-attributes_1.diff.bz2), and a combined patch against HEAD containing the full printTable API changes as well as the changes to describeRoles() (du-attributes-print-table_1.diff.bz2). No memory problems detected by valgrind, and all regression tests passed on x86_64 gentoo. I've added this item to the May CommitFest wiki page. Cheers, BJ [1[ http://archives.postgresql.org/message-id/[EMAIL PROTECTED] du-attributes_1.diff.bz2 Description: BZip2 compressed data du-attributes-print-table_1.diff.bz2 Description: BZip2 compressed data -- 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] Reference by output in : \d table_name
On Mon, Mar 31, 2008 at 3:50 AM, Tom Lane [EMAIL PROTECTED] wrote: kenneth d'souza [EMAIL PROTECTED] writes: With reference to the post http://archives.postgresql.org/pgsql-patches/2008-02/msg00104.phpand as stated by -hackers and -patchers, I am submitting the diff -c output as an attachment. Thanks, Kenneth Applied with some revisions. While working on my printTable patch, I noticed that this patch only has an indent of two spaces for incoming foreign keys, while all the other table footers have an indent of four spaces. Was this deliberate? And if so, why the change in indentation for this particular listing? Cheers, BJ -- 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] printTable API (was: Show INHERIT in \du)
On Sun, Mar 30, 2008 at 9:39 AM, Brendan Jurd [EMAIL PROTECTED] wrote: On 25/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: This makes me wonder whether print.c could offer something a bit more helpful to callers wishing to DIY a table; we could have a table-building struct with methods like addHeader and addCell. Once you have two occurrences of a pattern, it's reasonable to assume there will be more later. +1 for building a little bit of infrastructure. Per the above discussion, I'm looking at creating a nice API in print.c for callers wishing to build their own psql output table. Currently, if you want to build your own table you need to implement your own local version of the logic in printQuery. describeOneTableDetails is the only place this happens right now, but due to some localisation issues it looks like my \du patch will have to build its table manually as well. I'd like to submit my first version of this patch for review. I have introduced a new struct in print.h called printTableContent, which is used to compose the contents of a psql table. The methods exposed for this struct are as follows: void printTableInit(printTableContent *const content, const char *title, const int ncolumns, const int nrows); void printTableAddHeader(printTableContent *const content, const char *header, const int encoding, const bool translate, const char align); void printTableAddCell(printTableContent *const content, const char *cell, const int encoding, const bool translate); void printTableAddFooter(printTableContent *const content, const char *footer); void printTableSetFooter(printTableContent *const content, const char *footer); void printTableCleanUp(printTableContent *const content); -= Notes The column headers and cells are implemented as simple arrays of pointers to existing strings. It's necessary for the calling site to ensure that these strings survive at least until the table has been printed. That's usually not a problem since the headers and cells tend to be inside a PGresult. Footers are a bit different. I've implemented them as a very primitive singly-linked list which copies its contents with strdup. A lot of the complexity in the pre-patch describeOneTableDetails comes from the fact that it needs to know the number of footers in advance. The singly-linked list allows callers to incrementally add an arbitrary number of footers, and doesn't require them to preserve the strings, which is nice when you're working with a temporary PQExpBuffer to produce a footer. -= QA Not having written much C, I'm concerned about memory management errors. But I've run my own tests with describeOneTableDetails on tables with every kind of footer there is, and put the patch through valgrind, and I haven't encountered any segfaults or memory leaks. Both the serial and parallel regression tests passed perfectly. -= Documentation None required as far as I can tell, aside from code comments. -= Patch tracking I'll add this to the May CommitFest wiki page myself -- Bruce, you don't need to do anything at all! =) Looking forward to your comments. Cheers, BJ print-table.diff_0.bz2 Description: BZip2 compressed data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Typo in README
Hi there, Noticed a typo in the root README file. Patch below. Regards, BJ --- a/README +++ b/README @@ -20,7 +20,7 @@ PHP - http://www.php.net Python - http://www.initd.org/ Ruby - http://ruby.scripting.ca/postgres/ -Other language binding are available from a variety of contributing +Other language bindings are available from a variety of contributing parties. PostgreSQL also has a great number of procedural languages available, -- 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] psql command aliases support
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am Donnerstag, 3. April 2008 schrieb Gregory Stark: #= \oldd #= \old #= select 'where is all my output going?' #= select 'what happened to my ldd file?' psql allows you to omit the space between the command and argument? Does anybody else find that weird? What is the virtue of allowing such a syntax in the first place? I can't think of any other context where it's okay to issue a command together with arguments without some kind of delimiter, for the obvious reason that it introduces ambiguities such as those Greg described. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFH9OWk5YBsbHkuyV0RAmZ4AKC9qjU+KqgLJxQSJ4sD7X4YaPEbHgCg8ipW BRedeq4kG/FpqZBoptrKlRw= =out8 -END PGP SIGNATURE- -- 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] Consistent \d commands in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/04/2008, Greg Sabino Mullane wrote: \df Lists all user functions \df [pattern] Lists both system and user functions matching [pattern] \df * Lists all system and user functions I don't like this for two reasons: the items returned changes based on the existence of args, rather than on the command itself, and more importantly, this would make it inconsistent with the other backslash commands. To address your first complaint, I think it makes perfect sense to behave differently based on whether an argument has been supplied. The existence of a pattern tells you something about the user's intention in issuing the command. If they provide a pattern, they probably want to /search/ for something. If they don't provide a pattern they probably want to get a /listing/ of some set. Regards, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFH86u15YBsbHkuyV0RAmcMAKCy44zQaEcPA+QDpXr2+3vrSPucDwCgg3FG 8cq7P2DvI/ogqrHwM9Zpzx0= =IEvi -END PGP SIGNATURE- -- 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] Consistent \d commands in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/04/2008, Tom Lane wrote: David Fetter writes: When we have a bad default--and I'd argue that for anyone not developing PostgreSQL itself, showing system functions is a bad default--we should change it to something sane. I disagree with your parenthetical argument here, mainly on the strength of Greg's point about how that might hide the existence of conflicts. But in any case the discussion here is first about what set of behaviors we need to provide, and only second about which one should be default. If I read Greg's latter proposal correctly, he was suggesting \df Lists all user functions \df [pattern] Lists both system and user functions matching [pattern] \df * Lists all system and user functions This doesn't provide is all system functions only, but: 1. That list is way too long to be of much use in a psql context 2. You can still do a \df pg_catalog.* if you're really that keen. It also doesn't provide only user functions matching [pattern], but is that really a problem? I suppose you could conceive of a situation where somebody is looking for all the user funcs matching int* and getting annoyed by having to scroll past ~200 system funcs, but you can always refine your pattern, or clamp it to a particular schema. Regards, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFH8qtm5YBsbHkuyV0RAkXlAKCH8lL9H8XInLRvlbKnh84XafXyZwCg2Qom a3TuUMKHH7Yq/zZaA4MI7hk= =yLQJ -END PGP SIGNATURE- -- 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] Consistent \d commands in psql
On 31/03/2008, Gregory Stark [EMAIL PROTECTED] wrote: It might be cute to see if the pattern matches any user functions and if not try again with system functions. So you would still get results if you did \df rtrim for example. Nice idea. +1 for this behaviour. Cheers, BJ -- 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] Consistent \d commands in psql
On 01/04/2008, Tom Lane [EMAIL PROTECTED] wrote: ... That means there'd be no way to replicate the all-functions-of-both-types behavior that has been the default in every prior release. \dfS- sys functions only \dfU- user functions only \dfSU - all functions (should allow \dfUS spelling too) \df - behavior proposed by Greg How about \df* rather than (or in addition to) \dfSU \dfUS? -- 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] [HACKERS] Text - C string
On 26/03/2008, Tom Lane [EMAIL PROTECTED] wrote: There are no textout/textin calls left, but I may have missed some places that were doing it the hard way with direct palloc/memcpy manipulations. It might be worth trolling all the VARDATA() references to see if any more are easily replaceable. I think there are perhaps a dozen such sites, and I hope to submit a cleanup patch that gets rid of these soon. I did come across one site I'm not sure about in utils/adt/xml.c:291 /* * We need a null-terminated string to pass to parse_xml_decl(). Rather * than make a separate copy, make the temporary result one byte bigger * than it needs to be. */ result = palloc(nbytes + 1 + VARHDRSZ); SET_VARSIZE(result, nbytes + VARHDRSZ); memcpy(VARDATA(result), str, nbytes); str = VARDATA(result); str[nbytes] = '\0'; The author seemed pretty sure he didn't want to make a separate copy of the string, so I'm thinking there's not a whole lot I can do to improve this site. I notice in particular that xfunc.sgml contains sample C functions to copy and concatenate text. While these aren't directly replaceable with the new functions, I wonder whether we ought to change the examples to make them less certain to break if we ever change text's representation. Yes, these sample C functions are shown in tutorial/funcs.c and funcs_new.c as well. I agree that the examples could do with changing, but don't have any keen insight on what to change them to. -- 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] [HACKERS] Text - C string
On 29/03/2008, Tom Lane [EMAIL PROTECTED] wrote: I intentionally didn't touch xml.c, nor anyplace that is not dealing in text, even if it happens to be binary-compatible with text. Hmm, okay. My original submission did include a few such changes; for example, in xml_in and xml_out_internal I saw that the conversion between xmltype and cstring was identical to the text conversion, so I went ahead and used the text functions. Feedback upthread suggested that it was okay to go ahead with casting in identical cases. [1] I saw that these changes made it into the commit, so I assumed that it was the right call. If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Regards, BJ [1] http://archives.postgresql.org/pgsql-hackers/2007-09/msg01094.php -- 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] [HACKERS] quote_literal with NULL
On 23/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Applied with some revisions to sync it with CVS HEAD --- primarily, since we now have a quote_literal(anyelement) function, it seemed important to add a quote_nullable(anyelement) variant. I also editorialized on the documentation example a bit. Thanks Tom, good call on the (anyelement) version. I like the changes to the documentation too. I didn't know that the DISTINCT FROM operator was relatively slow. Regards, BJ - 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] [HACKERS] Function structure in formatting.c
On 23/03/2008, Tom Lane [EMAIL PROTECTED] wrote:
Working through this patch now. I found one thing that seems to be
a mistake (probably an overenthusiastic searchreplace): the patch
changes
- {iy, 2, dch_date, DCH_IY, TRUE},
to
+ {iyear, 2, DCH_IY, TRUE},
The removal of dch_date is intended, but surely the keyword should
still be iy. I'm proceeding on that assumption, but if this change
was actually intended, please explain.
Nice catch. Not sure how that got in there, but your theory about a
search replace gone awry seems the most likely.
Now that the functions have been refactored, I'm looking forward to
getting back into improving the sanity checking in to_date.
Cheers,
BJ
-
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] [HACKERS] Show INHERIT in \du
On 21/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: We can't just build the output table by hand like describeOneTableDetails does? Admittedly it's kludgy, but it's not an unprecedented kludge. Oh, I had forgotten the existence of that entry point in print.c. Yeah, it might be workable --- want to have a go at it? I've had a chance to look at this now, and although it certainly does seem workable, there's a lot of duplication of code that I feel uneasy about. describeOneTableDetails essentially already duplicates the table buildling code in printQuery, so I would be creating a third copy of the same logic. This makes me wonder whether print.c could offer something a bit more helpful to callers wishing to DIY a table; we could have a table-building struct with methods like addHeader and addCell. What do you think? Overkill, or worthy pursuit? - 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] [GENERAL] Empty arrays with ARRAY[]
On 21/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: As discussed on -hackers, this patch allows the construction of an empty array if an explicit cast to an array type is given (as in, ARRAY[]::int[]). Applied with minor fixes; mostly, ensuring that the cast action would propagate down to sub-arrays, as in Great, thanks Tom. I was interested to realize that this fix validates the decision to pass down the type information on-the-fly during transformExpr recursion. It would have been a lot more painful to do it if we'd taken the A_Const approach. Indeed. I didn't do anything about removing A_Const's typename field, but I'm thinking that would be a good cleanup patch. I'd be happy to take this on. My day job is pretty busy at the moment but I should be able to submit something in a week or so. Cheers, BJ -- 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] [HACKERS] Show INHERIT in \du
On 21/03/2008, Tom Lane [EMAIL PROTECTED] wrote: The code is now set up so that it can pass an entire field value through gettext(), but if gettext recognizes the strings foo and bar that doesn't mean it will do anything useful with foo\nbar, which is what this patch would require. Ouch! I suspect that to solve this in a non-kluge fashion we'd need to make \du pull over the plain boolean and integer values, then build a new PGresult data structure on its own. Ugh. (Actually, without any support from libpq for building PGresults, it's hard to imagine doing that in a way that wouldn't be a kluge itself.) Or we could go back to the drawing board on what the output ought to look like. We can't just build the output table by hand like describeOneTableDetails does? Admittedly it's kludgy, but it's not an unprecedented kludge. -- 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] [HACKERS] Show INHERIT in \du
On 21/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: We can't just build the output table by hand like describeOneTableDetails does? Admittedly it's kludgy, but it's not an unprecedented kludge. Oh, I had forgotten the existence of that entry point in print.c. Yeah, it might be workable --- want to have a go at it? Sure. It will be at least a few days (Easter holidays) before I can submit anything. -- 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] [GENERAL] Empty arrays with ARRAY[]
A quick recap: I submitted a patch for empty ARRAY[] syntax back in November, and as far as I can see it never made it to the patches list. Gregory suggested a different way of approaching the problem (quoted below), but nobody commented further about how it might be made to work. I'd like to RFC again on Gregory's idea, and if that doesn't bear any fruit I'd like to submit the patch as-is for review. Regards, BJ On 01/12/2007, Brendan Jurd [EMAIL PROTECTED] wrote: On Nov 30, 2007 9:09 PM, Gregory Stark [EMAIL PROTECTED] wrote: I'm sorry to suggest anything at this point, but... would it be less invasive if instead of requiring the immediate cast you created a special case in the array code to allow a placeholder object for empty array of unknown type. The only operation which would be allowed on it would be to cast it to some specific array type. That way things like UPDATE foo SET col = array[]; INSERT INTO foo (col) VALUES (array[]); could be allowed if they could be contrived to introduce an assignment cast. Not sure it would be less invasive, but I do like the outcome of being able to create an empty array pending assignment. In addition to your examples, it might also make it possible to do things like this in plpgsql DECLARE a text[] := array[]; Whereas my patch requires you to write a text[]: =array[]::text[]; ... which seems pretty stupid. ... Any suggestions about how you would enforce the only allow casts to array types restriction on the empty array? -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your Subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches
Re: [PATCHES] Reference by output in : \d table_name
On Mon, Feb 25, 2008 at 5:05 PM, kenneth d'souza [EMAIL PROTECTED] wrote: Refrenced by : htest_child_ctest_cust_id_fkey IN public.htest_child(ctest_cust_id) REFERENCES htest(new_id) htest_child1_ctest_cust_id_fkey IN public.htest_child1(ctest_cust_id) REFERENCES htest(new_id) Very cool! Cheers, BJ ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] Show INHERIT in \du
I've done up a patch per Tom's idea of combining the binary role
attributes into a single column.
Each attribute which differs from the default is listed on a separate
line, like so:
List of roles
Role name | Attributes | Member of
-++---
bob || {readers,writers}
brendanjurd | Superuser | {}
: Create role
: Create DB
harry | No inherit | {}
jim | 10 connections | {readers}
readers | No login | {}
writers | No login | {}
(6 rows)
Notes:
* The patch relies on array_to_string's current treatment of NULL
values in the array; they are ignored. If that behaviour changes in
the future, the \du output will become very ugly indeed.
* I'm not sure whether No login and No inherit are the best
phrases to use. I took my cue from the SQL setting names NOLOGIN and
NOINHERIT, but maybe something more grammatically sensible with
Cannot login and No inheritance would be preferable.
* If accepted, this patch would supercede the earlier patch mentioned
by Bernd Helmle upthread, which adds LOGIN to the output as a new
column: http://archives.postgresql.org/pgsql-patches/2007-11/msg00014.php
Cheers,
BJ
*** src/bin/psql/describe.c
--- src/bin/psql/describe.c
***
*** 1611,1638 describeRoles(const char *pattern, bool verbose)
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
! static const bool trans_columns[] = {false, true, true, true, true,
false, false};
initPQExpBuffer(buf);
printfPQExpBuffer(buf,
SELECT r.rolname AS \%s\,\n
! CASE WHEN r.rolsuper THEN '%s' ELSE '%s' END
AS \%s\,\n
!CASE WHEN r.rolcreaterole THEN '%s' ELSE '%s' END AS
\%s\,\n
! CASE WHEN r.rolcreatedb THEN '%s' ELSE '%s' END AS
\%s\,\n
! CASE WHEN r.rolconnlimit 0 THEN CAST('%s' AS
pg_catalog.text)\n
!ELSE CAST(r.rolconnlimit AS
pg_catalog.text)\n
! END AS \%s\, \n
ARRAY(SELECT b.rolname FROM
pg_catalog.pg_auth_members m JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)
WHERE m.member = r.oid) as \%s\,
gettext_noop(Role name),
- gettext_noop(yes),
gettext_noop(no),
gettext_noop(Superuser),
! gettext_noop(yes),
gettext_noop(no),
gettext_noop(Create role),
- gettext_noop(yes),
gettext_noop(no),
gettext_noop(Create DB),
! gettext_noop(no limit),
! gettext_noop(Connections),
gettext_noop(Member of));
if (verbose)
--- 1611,1639
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
! static const bool trans_columns[] = {false, true, false, false};
initPQExpBuffer(buf);
printfPQExpBuffer(buf,
SELECT r.rolname AS \%s\,\n
! array_to_string(ARRAY[\n
! CASE WHEN r.rolsuper THEN '%s' ELSE NULL
END,\n
! CASE WHEN NOT r.rolinherit THEN '%s' ELSE NULL END,\n
! CASE WHEN r.rolcreaterole THEN '%s' ELSE NULL END,\n
!CASE WHEN r.rolcreatedb THEN '%s' ELSE NULL
END,\n
!CASE WHEN NOT r.rolcanlogin THEN '%s' ELSE NULL END,\n
! CASE WHEN r.rolconnlimit = 0 THEN
r.rolconnlimit::pg_catalog.text || ' %s' ELSE NULL END\n
! ], chr(10)) AS \%s\,\n
ARRAY(SELECT b.rolname FROM
pg_catalog.pg_auth_members m JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)
WHERE m.member = r.oid) as \%s\,
gettext_noop(Role name),
gettext_noop(Superuser),
! gettext_noop(No inherit),
gettext_noop(Create role),
gettext_noop(Create DB),
! gettext_noop(No login),
! gettext_noop(connections),
! gettext_noop(Attributes),
gettext_noop(Member of));
if (verbose)
---(end of
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
On Nov 30, 2007 9:09 PM, Gregory Stark [EMAIL PROTECTED] wrote: I'm sorry to suggest anything at this point, but... would it be less invasive if instead of requiring the immediate cast you created a special case in the array code to allow a placeholder object for empty array of unknown type. The only operation which would be allowed on it would be to cast it to some specific array type. That way things like UPDATE foo SET col = array[]; INSERT INTO foo (col) VALUES (array[]); could be allowed if they could be contrived to introduce an assignment cast. Hi Gregory. Not sure it would be less invasive, but I do like the outcome of being able to create an empty array pending assignment. In addition to your examples, it might also make it possible to do things like this in plpgsql DECLARE a text[] := array[]; Whereas my patch requires you to write a text[]: =array[]::text[]; ... which seems pretty stupid. So, I like your idea a lot from a usability point of view. But I really, really hate it from a just spent half a week on this patch point of view =/ Any suggestions about how you would enforce the only allow casts to array types restriction on the empty array? Cheers BJ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
As discussed on -hackers, this patch allows the construction of an
empty array if an explicit cast to an array type is given (as in,
ARRAY[]::int[]).
postgres=# select array[]::int[];
array
---
{}
postgres=# select array[];
ERROR: no target type for empty array
HINT: Empty arrays must be explictly cast to the desired array type,
e.g. ARRAY[]::int[]
A few notes on the implementation:
* The syntax now allows an ARRAY constructor with an empty expression
list (array_expr_list may be empty).
* I've added a new parsenode for arrays, A_ArrayExpr (previously the
parser would create ArrayExpr primnodes).
* transformArrayExpr() now takes two extra arguments, a type oid and
a typmod. When transforming a typecast which casts an A_ArrayExpr to
an array type, transformExpr passes these type details down to
transformArrayExpr, and skips the typecast.
* transformArrayExpr() behaves slightly differently when passed type
information. The overall type of the array is set to the given type,
and all elements are explictly coerced to the equivalent element type.
If it was not passed a type, then the behaviour is as previous; the
function looks for a common type among the elements, and coerces them
to that type. The overall type of the array is derived from the
common element type.
The patch is very invasive (at least compared to any of my previous
patches), but so far I haven't managed to find any broken behaviour.
All regression tests pass, and the regression tests for arrays seem to
be quite comprehensive. I did add a couple of new tests for the empty
array behaviours, but the rest I've left alone.
I look forward to your comments -- although given the length of the
8.4 patch review queue, that will probably be an exercise in extreme
patience!
Major thanks go out to Tom for all his guidance on -hackers while I
developed the patch.
Regards,
BJ
*** ./doc/src/sgml/syntax.sgml.orig Fri Nov 30 19:31:29 2007
--- ./doc/src/sgml/syntax.sgml Fri Nov 30 19:32:11 2007
***
*** 1497,1503
array value from values for its member elements. A simple array
constructor
consists of the key word literalARRAY/literal, a left square bracket
! literal[/, one or more expressions (separated by commas) for the
array element values, and finally a right square bracket literal]/.
For example:
programlisting
--- 1497,1503
array value from values for its member elements. A simple array
constructor
consists of the key word literalARRAY/literal, a left square bracket
! literal[/, a list of expressions (separated by commas) for the
array element values, and finally a right square bracket literal]/.
For example:
programlisting
***
*** 1507,1515
{1,2,7}
(1 row)
/programlisting
! The array element type is the common type of the member expressions,
! determined using the same rules as for literalUNION/ or
! literalCASE/ constructs (see xref linkend=typeconv-union-case).
/para
para
--- 1507,1516
{1,2,7}
(1 row)
/programlisting
! If the array is not explictly cast to a particular type, the array
element
! type is the common type of the member expressions, determined using the
! same rules as for literalUNION/ or literalCASE/ constructs (see
! xref linkend=typeconv-union-case).
/para
para
***
*** 1554,1559
--- 1555,1573
/para
para
+You can construct an empty array, but since it's impossible to have an
array
+with no type, you must explictly cast your empty array to the desired
type. For example:
+ programlisting
+ SELECT ARRAY[]::int[];
+ int4
+ --
+ {}
+ (1 row)
+ /programlisting
+For more on casting, see xref linkend=sql-syntax-type-casts.
+ /para
+
+ para
It is also possible to construct an array from the results of a
subquery. In this form, the array constructor is written with the
key word literalARRAY/literal followed by a parenthesized (not
*** ./src/backend/nodes/copyfuncs.c.origFri Nov 30 19:29:16 2007
--- ./src/backend/nodes/copyfuncs.c Fri Nov 30 19:32:11 2007
***
*** 1704,1709
--- 1704,1719
return newnode;
}
+ static A_ArrayExpr *
+ _copyA_ArrayExpr(A_ArrayExpr *from)
+ {
+ A_ArrayExpr *newnode = makeNode(A_ArrayExpr);
+
+ COPY_NODE_FIELD(elements);
+
+ return newnode;
+ }
+
static ResTarget *
_copyResTarget(ResTarget *from)
{
***
*** 3538,3543
--- 3548,3556
case T_A_ArrayExpr:
retval = _copyA_ArrayExpr(from);
break;
+ case T_A_ArrayExpr:
+ retval = _copyA_ArrayExpr(from);
+ break;
case T_ResTarget:
retval = _copyResTarget(from);
break;
*** ./src/backend/nodes/outfuncs.c.orig
Re: [PATCHES] [HACKERS] quote_literal with NULL
On 10/12/07, Simon Riggs [EMAIL PROTECTED] wrote: I think you should add some examples to show how we would handle an INSERT or an UPDATE SET with quite_nullable() and a SELECT WHERE clause with quote_literal. The difference is a subtle one, which is why nobody mentioned it before, so it needs some better docs too. A cross-ref to the functions page would help also. Alright, I've improved the documentation along the lines suggested by Simon. There's a full example on doing a null-safe dynamic UPDATE, as well as a brief discussion about being wary of using comparison operators with NULLs (e.g., in WHERE clauses). Cross references abound. I did make a version of the patch which has the pg_proc entries for quote_literal and quote_nullable both pointing to the same internal function. I thought this was a tidier solution, but it failed regression test #5 in opr_sanity; apparently two entries in pg_proc can't have the same prosrc and differing proisstrict? Cheers, BJ quote-nullable_1.diff.bz2 Description: BZip2 compressed 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
Re: [PATCHES] [HACKERS] quote_literal with NULL
On 10/15/07, Simon Riggs [EMAIL PROTECTED] wrote: I did make a version of the patch which has the pg_proc entries for quote_literal and quote_nullable both pointing to the same internal function. I thought this was a tidier solution, but it failed regression test #5 in opr_sanity; apparently two entries in pg_proc can't have the same prosrc and differing proisstrict? Sanity prevails, I guess. :-) I'm all for the prevalance of sanity, but I'm not really clear on what about the above scenario is not sane. Suspect I'm missing something about the workings of pg_proc, but from over here it seems like having a strict and a non-strict version of the same function would be okay. As long as the internal function is rigged to handle null input properly, what's the problem? It's only tangential to the patch itself, and I'm not challenging the regression test. Just curious about it. Cheers, BJ ---(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] quote_literal with NULL
On 10/12/07, Simon Riggs [EMAIL PROTECTED] wrote: I think you should add some examples to show how we would handle an INSERT or an UPDATE SET with quite_nullable() and a SELECT WHERE clause with quote_literal. The difference is a subtle one, which is why nobody mentioned it before, so it needs some better docs too. A cross-ref to the functions page would help also. Thanks for your comments Simon. I agree about the doco, and will send in an updated patch soon. Looking at the patch again, I was thinking; is there actually any point having separate underlying C functions for quote_nullable and quote_literal? If I merged the functions together, and pointed both pg_proc entries at the one combined function wouldn't it have the same effect? Perhaps having the separate function makes the intent of the code more obvious, but looking at the patch with fresh eyes I'm thinking it's mostly a waste of space. Cheers, BJ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Use of postmaster
On 10/4/07, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: Now that we've renamed the server binary to postgres, what is the status on use of the name postmaster? Is it now deprecated? And if not, is there any point in keeping it around? We should replace it by terms like server in contexts where it's not actually important to the reader which process is involved, but I think Peter's hit most of them already ... Looks like Peter got the sgml sources pretty well cleaned up, but didn't touch the FAQs. The attached patch replaces some more references to postmaster in the FAQs. Per Tom's guidance, I only replaced those references where I felt a distinction between the postmaster and its children wasn't important to the reader. Thanks for your time, BJ Index: doc/FAQ === RCS file: /projects/cvsroot/pgsql/doc/FAQ,v retrieving revision 1.433 diff -c -r1.433 FAQ *** doc/FAQ 27 Sep 2007 06:14:46 - 1.433 --- doc/FAQ 3 Oct 2007 23:43:08 - *** *** 423,432 3.5) Why do I get Sorry, too many clients when trying to connect? !You have reached the default limit is 100 database sessions. You need !to increase the postmaster's limit on how many concurrent backend processes it can start by changing the max_connections value in !postgresql.conf and restarting the postmaster. 3.6) What is the upgrade process for PostgreSQL? --- 423,432 3.5) Why do I get Sorry, too many clients when trying to connect? !You have reached the default limit of 100 database sessions. You need !to increase the server's limit on how many concurrent backend processes it can start by changing the max_connections value in !postgresql.conf and restarting the server. 3.6) What is the upgrade process for PostgreSQL? *** *** 753,759 You probably have run out of virtual memory on your system, or your kernel has a low limit for certain resources. Try this before starting !postmaster: ulimit -d 262144 limit datasize 256m --- 753,759 You probably have run out of virtual memory on your system, or your kernel has a low limit for certain resources. Try this before starting !the server: ulimit -d 262144 limit datasize 256m Index: doc/FAQ_AIX === RCS file: /projects/cvsroot/pgsql/doc/FAQ_AIX,v retrieving revision 1.21 diff -c -r1.21 FAQ_AIX *** doc/FAQ_AIX 6 Dec 2006 15:45:30 - 1.21 --- doc/FAQ_AIX 3 Oct 2007 23:43:10 - *** *** 301,307 The overall cause of all these problems is the default bittedness and ! memory model used by the postmaster process. By default, all binaries built on AIX are 32-bit. This does not depend upon hardware type or kernel in use. These 32-bit processes --- 301,307 The overall cause of all these problems is the default bittedness and ! memory model used by the server process. By default, all binaries built on AIX are 32-bit. This does not depend upon hardware type or kernel in use. These 32-bit processes *** *** 327,336 build, but not run, 64-bit binaries. If a 32-bit binary is desired, set LDR_CNTRL to MAXDATA=0xn000, ! where 1 = n = 8, before starting the postmaster and try different values and postgresql.conf settings to find a configuration that works satisfactorily. This use of LDR_CNTRL tells AIX that you want the ! postmaster to have $MAXDATA bytes set aside for the heap, allocated in 256MB segments. When you find a workable configuration, ldedit can be used to modify --- 327,336 build, but not run, 64-bit binaries. If a 32-bit binary is desired, set LDR_CNTRL to MAXDATA=0xn000, ! where 1 = n = 8, before starting the postgres server and try different values and postgresql.conf settings to find a configuration that works satisfactorily. This use of LDR_CNTRL tells AIX that you want the ! server to have $MAXDATA bytes set aside for the heap, allocated in 256MB segments. When you find a workable configuration, ldedit can be used to modify Index: doc/FAQ_CYGWIN === RCS file: /projects/cvsroot/pgsql/doc/FAQ_CYGWIN,v retrieving revision 1.2 diff -c -r1.2 FAQ_CYGWIN *** doc/FAQ_CYGWIN 15 Oct 2004 16:18:35 - 1.2 --- doc/FAQ_CYGWIN 3 Oct 2007 23:43:10 - *** *** 30,37 3a. Start cygserver for shared memory support. To do this, enter the command /usr/sbin/cygserver . This program ! needs to be running anytime you start the PostgreSQL server ! (postmaster) or initialize a database (initdb). 3b. Use the initdb
Re: [PATCHES] [HACKERS] Function structure in formatting.c
I noticed an editing error in the patch I originally submitted; it
defined the same debugging macro twice.
I've attached a fresh copy of the patch against the current HEAD with
the fix included.
Cheers,
BJ
On 8/11/07, Brendan Jurd [EMAIL PROTECTED] wrote:
Hello,
As discussed on -hackers, I've done some refactoring work on
backend/utils/adt/formatting.c, in an attempt to make the code a bit
more intelligible before improving handling of bogus formats.
This is purely a refactor. The functionality of the file hasn't
changed; it does the same job as before, but it does it in ~200 fewer
lines and ~3.5k fewer characters. The clarity of code is greatly
improved. Sadly, performance appears to be unchanged.
Summary of changes:
* Did away with dch_global, dch_date and dch_time.
* Replaced DCH_processor with two new functions DCH_to_char and
DCH_from_char, which now do all the work previously done by
dch_{global,date,time}.
* Removed the 'action' field from the KeyWord struct as it is no longer
useful.
* Changed the type of the 'character' field in the FormatNode struct
to char, because ... that's what it is. The original choice of 'int'
seems to have been an error.
* Removed commented-out function declaration for is_acdc. According
to CVS annotate, this hasn't been in use since sometime in the early
Cretaceous period, and in any case I don't know why you'd want to
check whether a string was the rock band AC/DC. =)
* Reworded some of the comments for clarity.
* Didn't touch any of the number formatting routines.
This compiles cleanly on x86 gentoo and passes check, installcheck and
installcheck-parallel.
Thanks for your time,
BJ
formatting-refactor_1.diff.gz
Description: GNU Zip compressed data
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] Text - C string
As discussed on -hackers, I'm trying to get rid of some redundant code by creating a widely useful set of functions to convert between text and C string in the backend. The new extern functions, declared in include/utils/builtins.h and defined in backend/utils/adt/varlena.c, are: char * text_cstring(const text *t) char * text_cstring_limit(const text *t, int len) text * cstring_text(const char *s) text * cstring_text_limit(const char *s, int len) Within varlena.c, the actual conversions are performed by: char * do_text_cstring(const text *t, const int len) text * do_cstring_text(const char *s, const int len) These functions now do the work for the fmgr functions textin and textout, as well as being directly accessible by backend code. I've searched through the backend for any code which converted between text and C string manually (with memcpy and VARDATA), replacing with calls to one of the four new functions as appropriate. I came across some areas which were using the same, or similar, conversion technique on other varlena data types, such as bytea or xmltype. In cases where the conversion was completely identical I used the new functions. In cases with any differences (even if they seemed minor) I played it safe and left them alone. I'd now like to submit my work so far for review. This patch compiled cleanly on Linux and passed all parallel regression tests. It appears to be performance-neutral based on a few rough tests; I haven't tried to profile the changes in detail. There is still a lot of code out there using DirectFunctionCall1 to call text(in|out)). I've decided to wait for some community feedback on the patch as it stands before replacing those calls. There are a great many, and it would be a shame to have to go through them more than once. I would naively expect that replacing fmgr calls with direct calls would lead to a performance gain (no fmgr overhead), but honestly I'm not sure whether that would actually make a difference. Thanks for your time, BJ text-cstring_1.diff.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] Add function for quote_qualified_identifier?
On 9/29/07, Bruce Momjian [EMAIL PROTECTED] wrote: I think we need more than one person's request to add this function. Well, I don't expect it would get requested. Most DBAs would likely look for the function in the docs, see it's not there and then just implement it themselves. Obviously it's not critical. But anticipating those little requirements and providing for them is one of the things that makes a piece of software a pleasure to use. Batteries included and all that. Anyway, I seem to be flogging a horse which, if not dead, is surely mortally wounded. If quote_qualified_ident isn't desired, perhaps you can still use the regression test I included for quote_ident in the patch. The test is functional as a standalone item, and seems to fill a gap. Thanks for your time, BJ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Add function for quote_qualified_identifier?
On 9/29/07, Bruce Momjian [EMAIL PROTECTED] wrote: Has anyone every asked for this functionality? I searched the list archives for previous mentions of the topic, and didn't find any. So the answer to your question is yes, but so far it seems to be just me. Cheers, BJ ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Eliminate more detoast copies for packed varlenas
On 9/22/07, Gregory Stark [EMAIL PROTECTED] wrote: Ok, this removes what should be most if not all of the call sites where we're detoasting text or byteas. In particular it gets all the regexp/like functions and all the trim/pad functions. It also gets hashtext and hash_any. Looks like there's some more of this in src/tutorial/funcs.c and funcs_new.c. On a related note, while I was trawling through header files trying to wrap my head around all this toast and varlena business, I found the following comment, in fmgr.h and reiterated in postgres.h: WARNING: It is only safe to use PG_DETOAST_DATUM_UNPACKED() and VARDATA_ANY() if you really don't care about the alignment. / Shouldn't this be PG_DETOAST_DATUM_PACKED()? I'm emboldened by the fact that there is no macro called PG_TOAST_DATUM_UNPACKED defined anywhere in postgres. Patch attached, in case I've got the right idea. Regards, BJ Index: src/include/fmgr.h === --- src/include/fmgr.h (revision 29144) +++ src/include/fmgr.h (working copy) @@ -158,11 +158,11 @@ * The resulting datum can be accessed using VARSIZE_ANY() and VARDATA_ANY() * (beware of multiple evaluations in those macros!) * - * WARNING: It is only safe to use PG_DETOAST_DATUM_UNPACKED() and - * VARDATA_ANY() if you really don't care about the alignment. Either because - * you're working with something like text where the alignment doesn't matter - * or because you're not going to access its constituent parts and just use - * things like memcpy on it anyways. + * WARNING: It is only safe to use PG_DETOAST_DATUM_PACKED() and VARDATA_ANY() + * if you really don't care about the alignment. Either because you're working + * with something like text where the alignment doesn't matter or because + * you're not going to access its constituent parts and just use things like + * memcpy on it anyways. * * Note: it'd be nice if these could be macros, but I see no way to do that * without evaluating the arguments multiple times, which is NOT acceptable. Index: src/include/postgres.h === --- src/include/postgres.h (revision 29144) +++ src/include/postgres.h (working copy) @@ -237,7 +237,7 @@ * code that specifically wants to work with still-toasted Datums. * * WARNING: It is only safe to use VARDATA_ANY() -- typically with - * PG_DETOAST_DATUM_UNPACKED() -- if you really don't care about the alignment. + * PG_DETOAST_DATUM_PACKED() -- if you really don't care about the alignment. * Either because you're working with something like text where the alignment * doesn't matter or because you're not going to access its constituent parts * and just use things like memcpy on it anyways. ---(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] Add function for quote_qualified_identifier?
I had some spare cycles so I went ahead and patched this.
Patch includes documentation and new regression tests. While I was in
there I also added regression tests for quote_ident(), which appeared
to be absent.
quote_literal doesn't seem to have any regression tests either, but I
decided to leave that for another patch.
With thanks to Neil Conway for his assistance on IRC.
Cheers
BJ
On 9/15/07, Bruce Momjian [EMAIL PROTECTED] wrote:
This has been saved for the 8.4 release:
Brendan Jurd wrote:
Hi hackers,
I note that we currently expose the usefulness of the quote_identifier
function to the user with quote_ident(text).
Is there any reason we shouldn't do the same with
quote_qualified_identifier?
We could just add a quote_qualified_ident(text, text) ... it would
make forming dynamic queries more convenient in databases that use
multiple schemas.
Clearly a DBA could just create this function himself in SQL (and it
wouldn't be difficult), but is that a good reason not to have it in
our standard set of functions?
Would be happy to cook up a patch for this.
Cheers,
BJ
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
--
Bruce Momjian [EMAIL PROTECTED] http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/func.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.397
diff -c -r1.397 func.sgml
*** doc/src/sgml/func.sgml 19 Sep 2007 03:13:57 - 1.397
--- doc/src/sgml/func.sgml 22 Sep 2007 03:07:26 -
***
*** 1276,1281
--- 1276,1284
primaryquote_ident/primary
/indexterm
indexterm
+ primaryquote_qualified_ident/primary
+/indexterm
+indexterm
primaryquote_literal/primary
/indexterm
indexterm
***
*** 1541,1546
--- 1544,1563
/row
row
+
entryliteralfunctionquote_qualified_ident/function(parameterschema/parameter
typetext/type, parameteridentifier/parameter
typetext/type)/literal/entry
+entrytypetext/type/entry
+entry
+ Return the given schema and identifier suitably quoted to be
used as a
+ fully qualified identifier in an acronymSQL/acronym
statement
+ string. Quoting is performed as for
functionquote_ident/function,
+ but parameterschema/parameter and
parameteridentifier/parameter
+ are quoted separately.
+/entry
+entryliteralquote_ident('Some schema','A table')/literal/entry
+entryliteralSome schema.A table/literal/entry
+ /row
+
+ row
entryliteralfunctionquote_literal/function(parameterstring/parameter)/literal/entry
entrytypetext/type/entry
entry
Index: src/backend/utils/adt/quote.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/quote.c,v
retrieving revision 1.22
diff -c -r1.22 quote.c
*** src/backend/utils/adt/quote.c 27 Feb 2007 23:48:08 - 1.22
--- src/backend/utils/adt/quote.c 22 Sep 2007 03:07:26 -
***
*** 46,51
--- 46,77
}
/*
+ * quote_qualified_ident -
+ *returns a properly quoted, schema-qualified identifier
+ */
+ Datum
+ quote_qualified_ident(PG_FUNCTION_ARGS)
+ {
+ text*schema = PG_GETARG_TEXT_P(0);
+ text*ident = PG_GETARG_TEXT_P(1);
+ text*result;
+ const char *quoted;
+ char*schema_s;
+ char*ident_s;
+
+ schema_s = DatumGetCString(DirectFunctionCall1(textout,
+
PointerGetDatum(schema)));
+ ident_s = DatumGetCString(DirectFunctionCall1(textout,
+
PointerGetDatum(ident)));
+
+ quoted = quote_qualified_identifier(schema_s, ident_s);
+
+ result = DatumGetTextP(DirectFunctionCall1(textin,
+
CStringGetDatum(quoted)));
+ PG_RETURN_TEXT_P(result);
+ }
+
+ /*
* quote_literal -
* returns a properly quoted literal
*
Index: src/include/catalog/pg_proc.h
===
RCS file: /projects/cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.471
diff -c -r1.471 pg_proc.h
*** src/include/catalog/pg_proc.h 20 Sep 2007 17:56:32
Re: [PATCHES] [HACKERS] Add function for quote_qualified_identifier?
On 9/23/07, Tom Lane [EMAIL PROTECTED] wrote: This seems rather pointless, since it's equivalent to quote_ident(schemaname) || '.' || quote_ident(relname). Yes it is, and I brought that up in the OP: I wrote: Clearly a DBA could just create this function himself in SQL (and it wouldn't be difficult), but is that a good reason not to have it in our standard set of functions? But since nobody arced up about it I thought I might as well move things along and produce a patch. Many of the functions provided by postgres are easy to write yourself. That doesn't mean they shouldn't be there. After all, there is *exactly* one way to do quote_qualified_ident. Why require every DBA who needs this functionality to go through the motions? I'll admit that it's a minor improvement, but that seems reasonable given it has a miniscule cost. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] Linkage for escape strings
Just a minor doc upgrade. I've linked a couple of the more prominent mentions of escape string syntax in Functions and Operators / Pattern Matching back to the section on SQL string literals, which explains how escape syntax works. I considering linking all mentions of escape syntax, but thought that might be overkill since there are so many of them. Thanks for your time, BJ Index: doc/src/sgml/func.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/func.sgml,v retrieving revision 1.392 diff -c -r1.392 func.sgml *** doc/src/sgml/func.sgml 31 Aug 2007 21:33:48 - 1.392 --- doc/src/sgml/func.sgml 1 Sep 2007 17:09:45 - *** *** 2929,2942 /para para ! Note that the backslash already has a special meaning in string ! literals, so to write a pattern constant that contains a backslash ! you must write two backslashes in an SQL statement (assuming escape ! string syntax is used). Thus, writing a pattern ! that actually matches a literal backslash means writing four backslashes ! in the statement. You can avoid this by selecting a different escape ! character with literalESCAPE/literal; then a backslash is not special ! to functionLIKE/function anymore. (But it is still special to the string literal parser, so you still need two of them.) /para --- 2929,2942 /para para ! Note that the backslash already has a special meaning in string literals, ! so to write a pattern constant that contains a backslash you must write two ! backslashes in an SQL statement (assuming escape string syntax is used, see ! xref linkend=sql-syntax-strings). Thus, writing a pattern that ! actually matches a literal backslash means writing four backslashes in the ! statement. You can avoid this by selecting a different escape character ! with literalESCAPE/literal; then a backslash is not special to ! functionLIKE/function anymore. (But it is still special to the string literal parser, so you still need two of them.) /para *** *** 3549,3555 meaning in productnamePostgreSQL/ string literals. To write a pattern constant that contains a backslash, you must write two backslashes in the statement, assuming escape ! string syntax is used. /para /note --- 3549,3555 meaning in productnamePostgreSQL/ string literals. To write a pattern constant that contains a backslash, you must write two backslashes in the statement, assuming escape ! string syntax is used (see xref linkend=sql-syntax-strings). /para /note ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] Default location for docbook stylesheets in Gentoo
This patch adds the default location for the DocBook DSSSL stylesheets in gentoo's package system to the configure script. The package in question is app-text/docbook-dsssl-stylesheets. I'll understand if we don't want to include the location for every single distribution under the sun, but figured it's more help than harm to include it. Cheers BJ Index: configure === RCS file: /projects/cvsroot/pgsql/configure,v retrieving revision 1.556 diff -r1.556 configure 24263a24264 sgml/stylesheets/dsssl/docbook \ ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Default location for docbook stylesheets in Gentoo
On 8/9/07, Neil Conway [EMAIL PROTECTED] wrote: FYI, patches should typically be submitted as context diffs. For the specific case of configure, you should submit patches against the source file (configure.in), not the generated file (configure). Apologies. I haven't done much dev involving autoconf, so it didn't occur to me that configure wasn't a source. Is the following a reasonable way to test my changes? $ rm configure $ autoconf $ ./configure New (context) diff attached. The actual change is in config/docbook.m4, not configure.in. Cheers BJ Index: config/docbook.m4 === RCS file: /projects/cvsroot/pgsql/config/docbook.m4,v retrieving revision 1.7 diff -c -r1.7 docbook.m4 *** config/docbook.m4 13 Dec 2003 20:25:18 - 1.7 --- config/docbook.m4 8 Aug 2007 19:00:26 - *** *** 60,66 for pgac_postfix in \ sgml/stylesheets/nwalsh-modular \ sgml/stylesheets/docbook \ ! sgml/docbook-dsssl \ sgml/docbook/dsssl/modular \ sgml/docbook/stylesheet/dsssl/modular \ sgml/docbook/dsssl-stylesheets --- 60,67 for pgac_postfix in \ sgml/stylesheets/nwalsh-modular \ sgml/stylesheets/docbook \ ! sgml/stylesheets/dsssl/docbook \ ! sgml/docbook-dsssl \ sgml/docbook/dsssl/modular \ sgml/docbook/stylesheet/dsssl/modular \ sgml/docbook/dsssl-stylesheets ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [GENERAL] ISO week dates
On 2/17/07, Alvaro Herrera [EMAIL PROTECTED] wrote: Thanks for the clarification. Would you have a look at the tests as they are now and confirm that that's what you wanted? Yes, the tests in HEAD right now are what I wanted. Although, while I was in there I did notice a minor thing (another copy-paste error): the timestamptz test is using a column alias that's not consistent with other tests in the vicinity. Patch attached. iso_regress_fix.patch Description: Binary data ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [GENERAL] ISO week dates
On 2/15/07, Bruce Momjian [EMAIL PROTECTED] wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Peter Eisentraut wrote: I don't think Oracle even has that. But personally I'd like to see errors for invalid pattern combinations. What do we do with other invalid pattern combinations in to_char() now? Mostly, we return bogus results :-(. The formatting.c code in general doesn't seem very robust. Yep, seems every release I am in there cleaning up some mistake repeated multiple times in the code. It needs a good cleaning. I'm happy to volunteer to do something about the invalid field combinations, but I suspect an overhaul of formatting.c is more than I can currently chew. I figure it would be a bit misguided of me to put together a patch for invalid field combinations if somebody is about to do a rewrite of much of the code? ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [GENERAL] ISO week dates
On 2/15/07, Bruce Momjian [EMAIL PROTECTED] wrote: Bruce Momjian wrote: Brendan Jurd wrote: On 2/15/07, Bruce Momjian [EMAIL PROTECTED] wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Peter Eisentraut wrote: I don't think Oracle even has that. But personally I'd like to see errors for invalid pattern combinations. What do we do with other invalid pattern combinations in to_char() now? Mostly, we return bogus results :-(. The formatting.c code in general doesn't seem very robust. Yep, seems every release I am in there cleaning up some mistake repeated multiple times in the code. It needs a good cleaning. I'm happy to volunteer to do something about the invalid field combinations, but I suspect an overhaul of formatting.c is more than I can currently chew. I figure it would be a bit misguided of me to put together a patch for invalid field combinations if somebody is about to do a rewrite of much of the code? Yea, I was just throwing out a note that someday if someone has time, that file need a good sweeping. Sorry, I wasn't clear. No one is currently working on overhauling formatting.c, so if you want to submit _any_ patch to improve the file, please do. :-) It'll be a pleasure Bruce. I think it would be best to wait until the existing ISO week date patch has gone through before working on it though. Perhaps a TODO item is in order? ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [GENERAL] ISO week dates
On 11/9/06, Guillaume Lelarge [EMAIL PROTECTED] wrote: Brendan Jurd a écrit : I will take a look at implementing 'isoyear' for extract(), and also start putting together a patch for the documentation. If Guillaume is still interested in adding the IDDD field to to_char(), wonderful, if not I will pick up from his ID patch and add IDDD to it. Sorry for the late answer. I'm still interested but, to be honest, I don't think I will have the time to do it. Perhaps in a month or so. No problem Guillaume. I'm actually nearly done adding in all these features. Thank you for getting the ball rolling! ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Indicate disabled triggers in \d
As discussed briefly on pgsql-hackers, the current psql \d command does not make any distinction between enabled and disabled triggers. The attached patch modifies psql's describeOneTableDetails() such that triggers and disabled triggers are displayed as two separate footer lists, for example: Triggers: y AFTER DELETE ON x FOR EACH ROW EXECUTE PROCEDURE do_something() Disabled triggers: z BEFORE INSERT ON x FOR EACH ROW EXECUTE PROCEDURE input_stuff() The patch compiled and tested cleanly on my machine, and passed all regression tests. I didn't find any relevant documentation that needed patching, so this feature add should work fine as a standalone patch. Regards, BJ describe.c.diff Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [GENERAL] ISO week dates
On 10/13/06, Guillaume Lelarge [EMAIL PROTECTED] wrote: Peter Eisentraut a écrit : There is an inconsistency here: 'IYYY' is the four-digit ISO year, 'IW' is the two-digit ISO week, but 'ID' would be the one-digit ISO day-of-the-week. I'm not sure we can fix that, but I wanted to point it out. Is there a two digit ISO day of the week ? If not, we should use ID. As you say, I don't know what we can do about that. I used Brendan Jurd's idea, perhaps he can tell us more on this matter. Thanks for your work so far Guillaume. I agree with Peter, it is inconsistent to have a one-digit field represented by a two-character code. However, I don't see a way around it. 'D' is already taken to mean the non-ISO day-of-week, and 'I' is taken to mean the last digit of the ISO year (although to be honest I don't see where this would be useful). This sort of thing is not unprecedented in to_char(). For example, the codes 'HH24' and 'HH12' are four characters long, but resolve to a two-digit result. 'DAY' resolves to nine characters, and so on. Basically I think we're stuck with ID for day-of-week and IDDD for day-of-year. I will take a look at implementing 'isoyear' for extract(), and also start putting together a patch for the documentation. If Guillaume is still interested in adding the IDDD field to to_char(), wonderful, if not I will pick up from his ID patch and add IDDD to it. Regards, BJ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] gettime() - a timeofday() alternative
On 8/7/05, Brendan Jurd [EMAIL PROTECTED] wrote: Hi all, I propose to add an internal function gettime() that transparently returns the current system time, as a timestamptz with maximum precision. Here's the patch. The changes to pg_proc.h, timestamp.h and timestamp.c are trivial. The changes to func.sgml are more comprehensive; I've split the section on Current Date/Time into two subsections, one that explains the transaction time functions and one for the system time functions. -- BJ timestamp.h.diff Description: Binary data pg_proc.h.diff Description: Binary data timestamp.c.diff Description: Binary data func.sgml.diff 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
Re: [PATCHES] Increased error verbosity when querying row-returning
Tom Lane wrote: Brendan Jurd [EMAIL PROTECTED] writes: Thanks Alvaro, changes made and new patch attached. I submitted this patch about 5 days ago and I haven't heard anything since. I don't wish to be rude, but I'm not familiar with the pgsql-patches etiquette yet, and I noticed most submissions and questions are getting responses very quickly. This is in the category of stuff that has to wait for 8.1, so nothing will be done with it until after we fork the CVS tree (which should happen any day now). If we weren't busy with getting 8.0 out the door, there'd probably be more response, but right now small patches are just going into the to-look-at-later folder... regards, tom lane That's cool, I've got nothing against it being in the to-look-at-later folder. Just glad it didn't end up in /dev/null by mistake =) Thanks for the clarification. BJ ---(end of broadcast)--- TIP 8: explain analyze is your friend
[PATCHES] Increased error verbosity when querying row-returning functions
This patch to src/backend/executor/nodeFunctionscan.c is intended to
make life a little easier for people using row-returning functions, by
increasing the level of detail in the error messages thrown when
tupledesc_match fails.
Cheers
BJ
Index: src/backend/executor/nodeFunctionscan.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.29
diff -c -r1.29 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c 31 Dec 2004 21:59:45 -
1.29
--- src/backend/executor/nodeFunctionscan.c 11 Jan 2005 22:17:16 -
***
*** 87,96
* need to do this for functions returning RECORD, but might as
* well do it always.
*/
! if (funcTupdesc !tupledesc_match(node-tupdesc, funcTupdesc))
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
!errmsg(query-specified return row and
actual function return row do not match)));
}
/*
--- 87,94
* need to do this for functions returning RECORD, but might as
* well do it always.
*/
! if( funcTupdesc )
! tupledesc_match(node-tupdesc, funcTupdesc);
}
/*
***
*** 363,369
--- 361,373
int i;
if (dst_tupdesc-natts != src_tupdesc-natts)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+errmsg(function return row and query-specified return
row do not match),
+errdetail(function-returned row contains %d
attributes, but query expects %d., src_tupdesc-natts, dst_tupdesc-natts)));
return false;
+ }
for (i = 0; i dst_tupdesc-natts; i++)
{
***
*** 373,382
--- 377,401
if (dattr-atttypid == sattr-atttypid)
continue; /* no worries */
if (!dattr-attisdropped)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+errmsg(function return row and
query-specified return row do not match),
+errdetail(function returned type %s at
ordinal position %d, but query expects %s.,
+format_type_be(sattr-atttypid),
+i+1,
+format_type_be(dattr-atttypid;
return false;
+ }
if (dattr-attlen != sattr-attlen ||
dattr-attalign != sattr-attalign)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+errmsg(function return row and
query-specified return row do not match),
+errdetail(physical storage mismatch on
dropped attribute at ordinal position %d., i+1)));
return false;
+ }
}
return true;
---(end of broadcast)---
TIP 8: explain analyze is your friend
Re: [PATCHES] Increased error verbosity when querying row-returning
Alvaro Herrera wrote:
On Wed, Jan 12, 2005 at 09:23:26AM +1100, Brendan Jurd wrote:
This patch to src/backend/executor/nodeFunctionscan.c is intended to
make life a little easier for people using row-returning functions, by
increasing the level of detail in the error messages thrown when
tupledesc_match fails.
You should get rid of the returns, because ereport(ERROR) will never
return control to the function and they are thus dead code. And make
the function return void rather than bool.
Also follow the style: use if (foo) rather than if( foo ). And
message style stipulates that the errdetail() message should start with
a capital (upper case?) letter.
Thanks Alvaro, changes made and new patch attached.
Index: src/backend/executor/nodeFunctionscan.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.29
diff -c -r1.29 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c 31 Dec 2004 21:59:45 -
1.29
--- src/backend/executor/nodeFunctionscan.c 12 Jan 2005 06:36:53 -
***
*** 36,42
static TupleTableSlot *FunctionNext(FunctionScanState *node);
! static bool tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
/*
*Scan Support
--- 36,42
static TupleTableSlot *FunctionNext(FunctionScanState *node);
! static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
/*
*Scan Support
***
*** 87,96
* need to do this for functions returning RECORD, but might as
* well do it always.
*/
! if (funcTupdesc !tupledesc_match(node-tupdesc, funcTupdesc))
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
!errmsg(query-specified return row and
actual function return row do not match)));
}
/*
--- 87,94
* need to do this for functions returning RECORD, but might as
* well do it always.
*/
! if (funcTupdesc)
! tupledesc_match(node-tupdesc, funcTupdesc);
}
/*
***
*** 357,369
* destination type, so long as the physical storage matches. This is
* helpful in some cases involving out-of-date cached plans.
*/
! static bool
tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
{
int i;
if (dst_tupdesc-natts != src_tupdesc-natts)
! return false;
for (i = 0; i dst_tupdesc-natts; i++)
{
--- 355,370
* destination type, so long as the physical storage matches. This is
* helpful in some cases involving out-of-date cached plans.
*/
! static void
tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
{
int i;
if (dst_tupdesc-natts != src_tupdesc-natts)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
!errmsg(function return row and query-specified return
row do not match),
!errdetail(Function-returned row contains %d
attributes, but query expects %d., src_tupdesc-natts, dst_tupdesc-natts)));
for (i = 0; i dst_tupdesc-natts; i++)
{
***
*** 373,382
if (dattr-atttypid == sattr-atttypid)
continue; /* no worries */
if (!dattr-attisdropped)
! return false;
if (dattr-attlen != sattr-attlen ||
dattr-attalign != sattr-attalign)
! return false;
}
return true;
--- 374,393
if (dattr-atttypid == sattr-atttypid)
continue; /* no worries */
if (!dattr-attisdropped)
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
!errmsg(function return row and
query-specified return row do not match),
!errdetail(Function returned type %s at
ordinal position %d, but query expects %s.,
!format_type_be(sattr-atttypid),
!i+1,
!format_type_be(dattr-atttypid;
!
if (dattr-attlen != sattr-attlen ||
dattr-attalign != sattr-attalign)
! ereport(ERROR
