Re: [HACKERS] Adding the optional clause 'AS' in CREATE TRIGGER
Hi Surafel, Thank you for your review! But I have not finish fixing a patch yet. > v Use tage in documentation ok. > v Don’t modified existing test case add new one instead These existing tests I modified are the results of commands "SELECT pg_get_triggerdef(oid, true) ... ". I modified them in order to match with pg_get_functiondef(). The keyword 'OR REPLACE' is always added in CREATE FUNCTION. Does not it make sense? > v Comment in pg_constraint.c is extended make it short > v Error message can be more guider if it tells about general rule ok. > v Wrong result in concurrency case Thank you for the detailed example case. I appreciate it. The issue does not seem to be resolved easily because I have to modify pg_get_constraintdef_worker() in ruleutils.c. And it takes some more time to modify pg_get_constraintdef_worker(). So, I am moving this patch to "Returned with Feedback". Regards, Okano Naoki Fujitsu -- 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] Adding the optional clause 'AS' in CREATE TRIGGER
Peter Eisentraut wrote: > On 3/8/17 04:12, Okano, Naoki wrote: > > Peter Eisentraut wrote: > >> I have a feeling that this was proposed a few times in the ancient past > >> but did not go through because of locking issues. I can't find any > >> emails about it through. Does anyone remember? Have you thought about > >> locking issues? > > Is this e-mail you are finding? > > https://www.postgresql.org/message-id/20140916124537.GH25887%40awork2.anarazel.de > > No, that's not the one I had in mind. I see. But I could only find it. Would anyone know e-mails discussed about locking issues? > > I am considering to add 'OR REPLACE' clause as a first step. > > At least, I think there is no need to change the locking level when > > replacing a trigger with 'EXECUTE PROCEDURE' clause. > > In PostgreSQL, we currently have ShareRowExclusiveLock lock on relation on > > which trigger is created. ShareRowExclusiveLock is enough to replace a > > trigger. > > Also, we currently have RowExclusiveLock on pg_trigger. RowExclusiveLock is > > enough to replace a trigger, too. > > I'm not saying it's not correct. I was just wondering. Thank you for your opinion! I think we do not need to change locking level in this case. If someone notice a mistake in my understanding, please point out it. Regards, Okano Naoki Fujitsu -- 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] Adding the optional clause 'AS' in CREATE TRIGGER
Peter Eisentraut wrote: > I have a feeling that this was proposed a few times in the ancient past > but did not go through because of locking issues. I can't find any > emails about it through. Does anyone remember? Have you thought about > locking issues? Is this e-mail you are finding? https://www.postgresql.org/message-id/20140916124537.GH25887%40awork2.anarazel.de I am considering to add 'OR REPLACE' clause as a first step. At least, I think there is no need to change the locking level when replacing a trigger with 'EXECUTE PROCEDURE' clause. In PostgreSQL, we currently have ShareRowExclusiveLock lock on relation on which trigger is created. ShareRowExclusiveLock is enough to replace a trigger. Also, we currently have RowExclusiveLock on pg_trigger. RowExclusiveLock is enough to replace a trigger, too. Regards, Okano Naoki Fujitsu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [WIP] RE: [HACKERS] DECLARE STATEMENT setting up a connection in ECPG
Hi I tried applying your patches. But it failed... The error messages are as below. $ git apply ../004_declareStmt_test_v4.patch error: patch failed: src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c:82 error: src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c: patch does not apply Regards, Okano Naoki
Re: [HACKERS] Keep ECPG comment for log_min_duration_statement
Michael Meskes wrote: > > Michael Meskes wrote: > > > The other option I can see, albeit without looking into details, is > > > allowing all comments and then testing it for the right syntax after > > > parsing. This could potentially also solve the above mentioned > > > option problem. > > > > This idea sounds great. But I am not sure that I can understand it > > correctly. > > > > I understood the concept of this idea as following. Is it right? > > 1. The parser ignores comments/*...*/ as usual. That is, parser does > > not > > identify comments as a token. > > I guess it'd be easier to accept each comment as a token and then parse the > token > text afterwards. > > > 2. After parsing, we parse again only to extract comments. > > Not sure if we can do that without creating a lot of overhead. I see. Based on your advice, I try to make a patch. I will attach a patch when I finish it. Regards, Okano Naoki Fujitsu -- 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] Keep ECPG comment for log_min_duration_statement
Thank you for your kind advise! Michael Meskes wrote: > The other option I can see, albeit without looking into details, is > allowing all comments and then testing it for the right syntax after > parsing. This could potentially also solve the above mentioned option > problem. This idea sounds great. But I am not sure that I can understand it correctly. I understood the concept of this idea as following. Is it right? 1. The parser ignores comments/*...*/ as usual. That is, parser does not identify comments as a token. 2. After parsing, we parse again only to extract comments. 3. comments are put back or forth in SQL statement. Regards, Okano Naoki Fujitsu -- 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] Keep ECPG comment for log_min_duration_statement
Hi, Michael wrote: > The reason for not keeping the comments in the statement was simply to > make the parser simpler. If you added this feature, do we still see a > reason for keeping the old version? Or in other words, shouldn't we > make the "enable-parse-comment" version the default without a new > option? Thank you for your feedback! As I said in the first e-mail, there are some restrictions of comment position in my implementation. I am concerned that an error will occur in .pgc files users made in old version. So, this feature should be a new option. When the pre-compiler(ECPG) converts C with embedded SQL to normal C code, gram.y is used for syntactic analysis. I need to change gram.y for comments in SQL statement. But I do not come up with better idea that gram.y is not affected. If you are interested in my implementation in detail, please check the [WIP]patch I attached. I am appreciated if you give me any idea or opinion. Regards, Okano Naoki Fujitsu [WIP]enable-parse-comment.patch Description: [WIP]enable-parse-comment.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Keep ECPG comment for log_min_duration_statement
Hi, I'd like to make it easier to analyze slow queries in ECPG using log_min_duration_statement. Especially when DBA and C application developer is different, DBA can feedback slow embedded query to the developer without difficulty and mistakes. When our customers log and look into slower queries in C programs with embedded SQL, they use log_min_duration_statement.Using this logging option, SQL statement slower than a threshold will be displayed, but comments won't. That's because the pre-compiler (ECPG) removes all the comments when the pre-compiler converts C with embedded SQL to normal C code. Without comments, DBA has difficulty with identifying to which C code the slow query belongs. And the exact slow query issue cannot be reported to the developer. So, I'd like to modify ecpg command not to remove some specific comments. [Use-cases] Here is a scenario: 1) Writing comments to embedded C file a) Describe the detailed usage of SQL in each comment. b) Allocating id to each SQL. - application developer need to create corresponding table between id and the detailed description of SQL 2) DBA takes advantage of comments especially when: a) Similar comments are displayed here and there. In such a case, each comment plays a role as an identifier and makes it easier for DBA to identify SQL he/she looking for. b) DBA and C application developer are different. DBA can tell an application developer which query is slow without mistakes. [Interface] add a new option "--enable-parse-comment" to ecpg command. ecpg --enable-parse-comment ,..., prog.pgc This option enables ecpg command to pass on block comments (/* 〜 */) to converted C file. The conditions to enable processing "block comments" as follows: - a block comment can be used with SELECT, INSERT, UPDATE, or DELETE - a block comment can be placed right after keywords: SELECT, INSERT, UPDATE, DELETE or With - other than those above error will occur - line comment(--) are ignored, which is same as log output when logging libpq application [Example] 1)[Correct comment position] this comment position is right after SELECT EXEC SQL SELECT /* qid=3, at line 30 in yourApp.ecpg */ * INTO :C1, :C2 FROM T1 WHERE C1=1; 2)[Incorrect comment position] this comment position is bad（error will occur) EXEC SQL /* qid=3, at line 30 in yourApp.ecpg */ SELECT * INTO :C1, :C2 FROM T1 WHERE C1=1; As far as I searched, there seems no discussion on this topic. Please let me know if exists. Regards, Okano Naoki Fujitsu -- 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] Adding the optional clause 'AS' in CREATE TRIGGER
On Thursday, February 9, 2017 4:41 AM Robert Haas wrote: > You should add this to the next CommitFest so it doesn't get missed. > > https://commitfest.postgresql.org/ Thank you for your kind advice! I have added this patch to the CommitFest 2017-03. Regards, Okano Naoki Fujitsu -- 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] Adding the optional clause 'AS' in CREATE TRIGGER
On Wednesday, November 16, 2016 4:31 PM Okano Naoki wrote: > > But in any case it would be a serious mistake to do this without first > > implementing CREATE OR REPLACE TRIGGER. I think that's an entirely > > separate > > proposal and you would be well advised to treat it as such. > I see. There are more problems than I expected... > Let me start with 'OR REPLACE' clause. I tried to cretae a patch for CREATE OR REPLACE TRIGGER. An example of execution is shown below. example) 1.define a new trigger CREATE TRIGGER regular_trigger AFTER UPDATE ON table_name REFERENCING OLD TABLE AS oldtable_1 NEW TABLE AS newtable_1 FOR EACH STATEMENT EXECUTE PROCEDURE func_1(); 2.redinfe a trigger in single command CREATE OR REPLACE TRIGGER regular_trigger AFTER UPDATE OR DELETE ON table_name REFERENCING OLD TABLE AS oldtable_2 NEW TABLE AS newtable_2 FOR EACH ROW EXECUTE PROCEDURE func_2(); If the named trigger does not exist. a new trigger is also created by using OR REPLACE clause. A regular trigger cannot be replaced by a constraint trigger and vice varsa. because a constraint trigger has a different role from regular triger. Please give me feedback. Regards, Okano Naoki Fujitsu OR_REPLACE_for_CREATE_TRIGGER_v1.patch Description: OR_REPLACE_for_CREATE_TRIGGER_v1.patch -- 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] pg_recvlogical --endpos
On Wednesday, November 30, 2016 10:34 AM Craig Ringer wrote: >On 30 November 2016 at 09:18, Okano, Naoki > wrote: >> >> On November 29, 2016 at 5:03 PM Craig Ringer wrote: >>> Would it be better rephrased as "--endpos can only be used with --start" ? >> OK. I think this phrase is better than the previous phrase. >> >>>> The patch should allow --endpos to work with --create-slot. >>> >>> How? It doesn't make sense with --create-slot . >> This patch is updated to incorporate Alvaro's changes shown below, isn't it? >> https://www.postgresql.org/message-id/20160503180658.GA59498%40alvherre.pgsql >> I thought that the consent in community was taken with this specification... >>> I could not find any mention that it did not make sense with --create-slot. > What would --endpos with --create-slot do? Sorry, I was misunderstanding. you are right. I have noticed that --endpos doesn't make sense unless it is with --start. I checked --endpos works with --create-slot and --start. So, there is no problem with this patch. Regards, Okano Naoki Fujitsu -- 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] pg_recvlogical --endpos
On November 29, 2016 at 5:03 PM Craig Ringer wrote: > Would it be better rephrased as "--endpos can only be used with --start" ? OK. I think this phrase is better than the previous phrase. >> The patch should allow --endpos to work with --create-slot. > > How? It doesn't make sense with --create-slot . This patch is updated to incorporate Alvaro's changes shown below, isn't it? https://www.postgresql.org/message-id/20160503180658.GA59498%40alvherre.pgsql I thought that the consent in community was taken with this specification... I could not find any mention that it did not make sense with --create-slot. But if exists, would you let me know? Regards, Okano Naoki Fujitsu -- 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] pg_recvlogical --endpos
On Monday, November 21, 2016 1:08 PM Craig Ringer wrote:
> I've updated the patch for this. It's already posted on the logical
> decoding timeline following thread, so I'll avoid repeating it here.
>
> https://www.postgresql.org/message-id/CAMsr%2BYGd5dv3zPNch6BU4UXX49NJDC9m3-Y%3DV5q%3DTNcE9QgSaQ%40mail.gmail.com
I checked the latest patch.
I think that the error message shown below is a typo.
> + if (endpos != InvalidXLogRecPtr && !do_start_slot)
> + {
> + fprintf(stderr,
> + _("%s: cannot use --create-slot or --drop-slot
> together with --endpos\n"),
The condition '!do_start_slot' is not reflected in the error message.
The patch should allow --endpos to work with --create-slot.
Also, the document explains as follows.
> +specified LSN. If specified when not in --start
> +mode, an error is raised.
So, it is better to output an error message that matches the document when
changing the error message.
Regards,
Okano Naoki
Fujitsu
--
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] Adding the optional clause 'AS' in CREATE TRIGGER
> But in any case it would be a serious mistake to do this without first > implementing CREATE OR REPLACE TRIGGER. I think that's an entirely separate > proposal and you would be well advised to treat it as such. I see. There are more problems than I expected... Let me start with 'OR REPLACE' clause. At least, adding only 'OR REPLACE' clause has the following advantages. * It enables users to redefine a trigger in single command. * The trigger can always be referenced when redefining a trigger. # But it is not so when using 'DROP' and 'CREATE' commands. It is useful when users change the function or change the condition of 'WHEN' clause. Regard, Okano Naoki Fujitsu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Adding the optional clause 'AS' in CREATE TRIGGER
Hi,
I would like to add the following support for a trigger.
This implementation enables to create a trigger efficiently
in single command.
It had been discussed before. The link is as shown below.
https://www.postgresql.org/message-id/CAA-aLv4m%3Df9cc1zcUzM49pE8%2B2NpytUDraTgfBmkTOkMN_wO2w%40mail.gmail.com
Currently, PostgreSQL requires two steps to create a trigger.
1. to create a function.
2. to define a trigger with action specified via already created function.
Supporting 'AS' clause in CREATE TRIGGER syntax will enable the option of
defining the trigger in single command.
As a bonus, it will be compatible with oracle.
Also, the optional clause 'OR REPLACE' is required as below.
https://www.postgresql.org/message-id/CAA-aLv6KYgVt2CwaRdcnptzWVngEm72Cp4mUFnF-MfeH0gS91g%40mail.gmail.com
Currently, to change the definition of a trigger, trigger needs to
be dropped first before creating it again with new definition.
To change the definition of a function in CREATE TRIGGER syntax,
trigger needs to be dropped first before creating it again with new definition,
too!
So, we need to add the optional clause 'OR REPLACE'.
Adding the optional clauses 'AS' and 'OR REPLACE' in CREATE TRIGGER syntax gives
the comfort of defining the trigger or redefining the trigger definition
which contains the function definition in single command.
Here is the syntax based on the previous discussion.
CREATE [ OR REPLACE ] [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD
OF }
{ event [ OR ... ] }
ON table_name
[ FROM referenced_table_name ]
[ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
DEFERRED } ]
[ FOR [ EACH ] { ROW | STATEMENT } ]
[ WHEN ( condition ) ]
{ EXECUTE PROCEDURE function_name ( arguments )
| AS 'trigger function definition' [ LANGUAGE lang_name ]
[ SET configuration_parameter { TO value | = value | FROM CURRENT }]
}
If you have your opinion on this concept, please give me it.
Regards,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
