Re: [HACKERS] Pretty printed trigger in psql
On Tuesday 12 January 2010 01:06:22 Takahiro Itagaki wrote:
Psql shows too many parentheses when it prints triggers with WHEN clause.
postgres=# \d t1
Table public.t1
Column | Type | Modifiers
+-+---
c1 | integer |
Triggers:
mytrig AFTER UPDATE ON t1 FOR EACH ROW
WHEN ((old.c1 new.c1)) EXECUTE PROCEDURE myfunc()
^^
The attached patch eliminates unneeded parentheses by using
pg_get_triggerdef(pretty = true) in psql.
Triggers:
mytrig AFTER UPDATE ON t1 FOR EACH ROW
WHEN (old.c1 new.c1) EXECUTE PROCEDURE myfunc()
snip
Greetings,
I tried to apply this patch to the latest version of PostgreSQL in git
(bbfc96e). Some of the patch did not apply. Please
find attached the output from patch. The full path of the ruleutils.c.rej is
src/backend/utils/adt/ruleutils.c.rej
Thanks,
--bts
Hmm... Looks like a new-style context diff to me...
The text leading up to this was:
--
|diff -cprN head/src/backend/utils/adt/ruleutils.c
work/src/backend/utils/adt/ruleutils.c
|*** head/src/backend/utils/adt/ruleutils.c 2010-01-04 09:10:26.638773000
+0900
|--- work/src/backend/utils/adt/ruleutils.c 2010-01-12 17:51:27.595666819
+0900
--
Patching file src/backend/utils/adt/ruleutils.c using Plan A...
Hunk #1 failed at 518.
Hunk #2 failed at 572.
Hunk #3 succeeded at 636.
2 out of 3 hunks failed--saving rejects to src/backend/utils/adt/ruleutils.c.rej
Hmm... The next patch looks like a new-style context diff to me...
The text leading up to this was:
--
|diff -cprN head/src/bin/psql/describe.c work/src/bin/psql/describe.c
|*** head/src/bin/psql/describe.c 2010-01-04 09:10:26.638773000 +0900
|--- work/src/bin/psql/describe.c 2010-01-12 17:51:27.597646243 +0900
--
Patching file src/bin/psql/describe.c using Plan A...
Hunk #1 succeeded at 1854 with fuzz 2.
Hmm... The next patch looks like a new-style context diff to me...
The text leading up to this was:
--
|diff -cprN head/src/test/regress/expected/triggers.out
work/src/test/regress/expected/triggers.out
|*** head/src/test/regress/expected/triggers.out2009-11-24
10:04:57.883822000 +0900
|--- work/src/test/regress/expected/triggers.out2010-01-12
17:53:21.142635393 +0900
--
Patching file src/test/regress/expected/triggers.out using Plan A...
Hunk #1 succeeded at 375.
Hunk #2 succeeded at 387.
Hunk #3 succeeded at 416.
Hmm... The next patch looks like a new-style context diff to me...
The text leading up to this was:
--
|diff -cprN head/src/test/regress/sql/triggers.sql
work/src/test/regress/sql/triggers.sql
|*** head/src/test/regress/sql/triggers.sql 2009-11-24 10:04:57.883822000
+0900
|--- work/src/test/regress/sql/triggers.sql 2010-01-12 17:51:27.597646243
+0900
--
Patching file src/test/regress/sql/triggers.sql using Plan A...
Hunk #1 succeeded at 304.
done
***
*** 518,527
initStringInfo(buf);
tgname = NameStr(trigrec-tgname);
! appendStringInfo(buf, CREATE %sTRIGGER %s,
! trigrec-tgisconstraint ? CONSTRAINT : ,
quote_identifier(tgname));
- appendStringInfoString(buf, pretty ? \n : );
if (TRIGGER_FOR_BEFORE(trigrec-tgtype))
appendStringInfo(buf, BEFORE);
--- 518,526
initStringInfo(buf);
tgname = NameStr(trigrec-tgname);
! appendStringInfo(buf, CREATE %sTRIGGER %s ,
! trigrec-tgisconstraint ? CONSTRAINT : ,
quote_identifier(tgname));
if (TRIGGER_FOR_BEFORE(trigrec-tgtype))
appendStringInfo(buf, BEFORE);
***
*** 573,605
appendStringInfo(buf, TRUNCATE);
findx++;
}
! appendStringInfo(buf, ON %s,
generate_relation_name(trigrec-tgrelid, NIL));
- appendStringInfoString(buf, pretty ? \n : );
if (trigrec-tgisconstraint)
{
if (OidIsValid(trigrec-tgconstrrelid))
! {
! appendStringInfo(buf, FROM %s,
generate_relation_name(trigrec-tgconstrrelid, NIL));
- appendStringInfoString(buf, pretty ? \n : );
- }
if (!trigrec-tgdeferrable)
appendStringInfo(buf, NOT );
appendStringInfo(buf, DEFERRABLE INITIALLY );
if (trigrec-tginitdeferred)
! appendStringInfo(buf, DEFERRED);
else
! appendStringInfo(buf, IMMEDIATE);
! appendStringInfoString(buf, pretty ? \n : );
}
if (TRIGGER_FOR_ROW(trigrec-tgtype))
! appendStringInfo(buf, FOR EACH ROW);
else
! appendStringInfo(buf, FOR EACH STATEMENT);
! appendStringInfoString(buf, pretty ? \n : );
/* If the trigger has a WHEN qualification, add that */
value = fastgetattr(ht_trig, Anum_pg_trigger_tgqual,
--- 572,598
appendStringInfo(buf, TRUNCATE);
findx++;
}
! appendStringInfo(buf, ON %s ,
Re: [HACKERS] Pretty printed trigger in psql
Brad T. Sliger b...@sliger.org wrote: I tried to apply this patch to the latest version of PostgreSQL in git (bbfc96e). Some of the patch did not apply. Please find attached the output from patch. The full path of the ruleutils.c.rej is src/backend/utils/adt/ruleutils.c.rej The attached patch is rebased to current CVS. Regards, --- Takahiro Itagaki NTT Open Source Software Center pretty-printed-trigger_20100119.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pretty printed trigger in psql
On Monday 18 January 2010 16:40:07 Takahiro Itagaki wrote: Brad T. Sliger b...@sliger.org wrote: I tried to apply this patch to the latest version of PostgreSQL in git (bbfc96e). Some of the patch did not apply. Please find attached the output from patch. The full path of the ruleutils.c.rej is src/backend/utils/adt/ruleutils.c.rej The attached patch is rebased to current CVS. That patch applies, builds and installs. `gmake check` and `gmake distcheck` pass. The code style looks fine and the patch doesn't seem to add additional lint. The patch does remove extra ()'s from trigger descriptions with \d in psql. As far as I can tell, everything looks reasonable. Regards, --- Takahiro Itagaki NTT Open Source Software Center Thanks, --bts -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pretty printed trigger in psql
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: The attached patch eliminates unneeded parentheses by using pg_get_triggerdef(pretty = true) in psql. Is this patch reversed? It seems so but the listed file timestamps don't match that idea ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pretty printed trigger in psql
Tom Lane t...@sss.pgh.pa.us wrote: Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: The attached patch eliminates unneeded parentheses by using pg_get_triggerdef(pretty = true) in psql. Is this patch reversed? It seems so but the listed file timestamps don't match that idea ... Sorry, I cannot understand what you mean... My English might be broken. May I explain what I did again? 1. psql has been used pg_get_triggerdef(oid). 2. I added pg_get_triggerdef(oid, pretty = false) at the last commit fest for pg_dump to support dumping triggeres with WHEN cluase. In that time, PRETTYFLAG_PAREN and PRETTYFLAG_INDENT are used when pretty = true. 3 psql still uses pg_get_triggerdef(oid [, pretty=false] ). Also, pg_dump should use (pretty=false) for safer migration. No programs use pg_get_triggerdef(pretty=true) is for now. 4. psql will be better to use pg_get_triggerdef(oid, true) to display trigger definitions cleanly, but it also should print them in one line. For the purpose, the patch changes two things: - Modify psql to use pg_get_triggerdef(oid, true) when server version = 8.5. - Remove PRETTYFLAG_INDENT from pg_get_triggerdef(). It will partially revert the changes in 2. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pretty printed trigger in psql
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: Tom Lane t...@sss.pgh.pa.us wrote: Is this patch reversed? It seems so but the listed file timestamps don't match that idea ... Sorry, I cannot understand what you mean... The patch looks like it removes, rather than adds, your intended changes. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Pretty printed trigger in psql
Psql shows too many parentheses when it prints triggers with WHEN clause. postgres=# \d t1 Table public.t1 Column | Type | Modifiers +-+--- c1 | integer | Triggers: mytrig AFTER UPDATE ON t1 FOR EACH ROW WHEN ((old.c1 new.c1)) EXECUTE PROCEDURE myfunc() ^^ The attached patch eliminates unneeded parentheses by using pg_get_triggerdef(pretty = true) in psql. Triggers: mytrig AFTER UPDATE ON t1 FOR EACH ROW WHEN (old.c1 new.c1) EXECUTE PROCEDURE myfunc() I think this change is harmless because we don't use pg_get_triggerdef(pretty = true) in any programs, including pg_dump. Is this change ok? Regards, --- Takahiro Itagaki NTT Open Source Software Center pretty-printed-trigger_20100112.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
