RE: Log a sample of transactions
Dear Adrien, I understood the cost of randomizing is very low. Maybe it's OK.. I'll change the status to "Ready For Committer." Finally, I apologize for having delayed review. Thank you for your good opportunities. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Re: Log a sample of transactions
Dear David, I have a will and already read the patch, but I thought it's not my turn. Sorry. Adrien, > I did not find any test for log_min_duration that could help me. LCOV > indicate > there is no tests on this part (look check_log_duration()): > https://coverage.postgresql.org/src/backend/tcop/postgres.c.gcov.html I understand the unnecessarily of some test case. It's OK. Finally, how do you think about the deviation of randomness? If this parameter is set very low, nobody may be output because of the deviation. we can avoid this phenomenon by counting up internal parameter for each transactions and output to log file if the parameter becomes more than 1. After consideration for this case and rebasing, I think this patch is enough. Do I have to measure the change of throughput? Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Is PREPARE of ecpglib thread safe?
Dear Matsumura-san, Horiguchi-san, We should check the specfication of Pro*c before coding because this follows its feature. I'll test some cases and send you a simple report. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: ECPG regression with DECLARE STATEMENT support
Dear Matsumura-san, > I think that the 2nd argument of following ecpg_init() must be > real_connection_name. > Is it right? Yes, I think it should be real_connection_name for raising correct error message. This is also an leak of my code and I attached a patch. Best Regards, Hayato Kuroda Fujitsu LIMITED fix_argument.patch Description: fix_argument.patch
RE: ECPG regression with DECLARE STATEMENT support
Dear Rushabh, Michael, I attached a simple bug-fixing patch. Could you review it? An added logic is: 1. Send a close statement to a backend process regardless of the existence of a cursor. 2. If ecpg_do function returns false, raise “cursor is invalid” error. 3. Remove cursor from application. I already checked this patch passes regression tests and Rushabh’s test code. Best Regards, Hayato Kuroda Fujitsu LIMITED DeclareStmt_fix_close_bug.patch Description: DeclareStmt_fix_close_bug.patch
RE: ECPG regression with DECLARE STATEMENT support
Dear Rushabh, Michael, Thank you for reporting. I understood bugs had be embed in ecpg by me. I start checking codes and investigating solutions and other errors. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Prevent extension creation in temporary schemas
Dear Michael, Chris and Tom, > Adding special cases to extensions strikes me as adding more > funny corners to the behavior of the db in this regard. I understand your arguments and its utility. > For most of extensions, this can randomly finish with strange error > messages, say that: > =# create extension file_fdw with schema pg_temp_3; > ERROR: 42883: function file_fdw_handler() does not exist > LOCATION: LookupFuncName, parse_func.c:2088 I found that this strange error appears after making temporary tables. test=> CREATE TEMPORARY TABLE temp (id int); CREATE TABLE test=> CREATE EXTENSION file_fdw WITH SCHEMA pg_temp_3; ERROR: function file_fdw_handler() does not exist I would try to understand this problem for community and my experience. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Prevent extension creation in temporary schemas
Dear Michael, I seem this patch is enough, but could you explain the reason you drop initial proposal more detail? I'm not sure why extensions contained by temporary schemas are acceptable. > Anything depending on a temporary object will be dropped per > dependency links once the session is over. Extensions locate at pg_temp_* schemas are temporary objects IMO. How do you think? Would you implement this functionality in future? Hayato Kuroda Fujitsu LIMITED
RE: Log a sample of transactions
Dear Adrien, >> * xact_is_sampled is left at the end of a transaction. >> Should the parameter be set to false at the lowest layer of the transaction >> system? >> I understand it is unnecessary for the functionality, but it have more >> symmetry. > > Yes, it is not necessary. I wonder what is more important : keep some >kind of symmetry or avoid unnecessary code (which can be source of mistake)? >> >> * check_log_duration should be used only when postgres check the duration. >> But I'm not sure a new function such as check_is_sampled is needed because A >> processing in new function will be as almost same as check_log_duration. > I agree, I asked myself the same question and I preferred to keep code simple. I think your point is also correct. You should inquire superior reviewers or committers because I cannot judge which one is better. BTW, I give you a suggestion about a test. This parameter enables users to log statements randomly, hence adding some tests is very difficult. Perhaps Only three cases are available: * When log_transaction_sample_rate is set to 1, all statements are logged. * When the parameter is set to 0, they are never logged. * When the parameter change to 0 inside the transaction, logging is immediately stopped. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Log a sample of transactions
Dear Adrien, > Hum, I am not an english native speaker, I choosed "rate" because it is > already used for auto_explain.sample_rate and for log_statement_sample_rate If the community agree those name, renaming parameters are not needed. Consistency is the most important in a DBMS. :-) I read your source code and I thought it is generally good. Here are some minor suggestions, but you don't have to follow strictly: config.sgml You must document the behavior when users change the parameter during a transaction. あやしい・・・ postgres.c I give you three comments. > /* flag for logging statements in this transaction */ I think "a" or the plural form should be used instead of "this" * xact_is_sampled is left at the end of a transaction. Should the parameter be set to false at the lowest layer of the transaction system? I understand it is unnecessary for the functionality, but it have more symmetry. * check_log_duration should be used only when postgres check the duration. But I'm not sure a new function such as check_is_sampled is needed because A processing in new function will be as almost same as check_log_duration. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Log a sample of transactions
Dear Adrien, > Your messages looks normal to me, you will have same messages if you put > log_min_duration to 0. That was my lack of understanding. Sorry. By the way, I think the name of this parameter should be changed. In the Cambridge dictionary, the word "rate" means as follows: the speed at which something happens or changes, or the amount or number of times it happens or changes in a particular period. This parameter represents "probability" whether it log, therefore this name is inappropriate. You should use "probability" or "ratio", I think. Next week I will give you some comments about your good source. Best Regards, Hayato Kuroda Fujitsu LIMITED
RE: Log a sample of transactions
Dear Adrien,
I applied your file and faced a bug-candidate.
(I have not read your source yet. Sorry.)
When I tried to use tab-complition, some dirty messages were appeared.
Messages I faced are attached in this mail.
Best Regards,
Hayato Kuroda
Fujitsu LIMITED
postgres=# SET client_min_messages to LOG;
SET
postgres=# SET log_transaction_sample_rate to 1;
SET
postgres=# select * from LOG: duration: 4.906 ms statement: SELECT
pg_catalog.quote_ident(c.relname) FROM pg_catalog.pg_class c WHERE c.relkind IN
('r', 'S', 'v', 'm', 'f', 'p') AND
substring(pg_catalog.quote_ident(c.relname),1,0)='' AND
pg_catalog.pg_table_is_visible(c.oid) AND c.relnamespace <> (SELECT oid FROM
pg_catalog.pg_namespace WHERE nspname = 'pg_catalog')
UNION
SELECT pg_catalog.quote_ident(n.nspname) || '.' FROM pg_catalog.pg_namespace n
WHERE substring(pg_catalog.quote_ident(n.nspname) || '.',1,0)='' AND (SELECT
pg_catalog.count(*) FROM pg_catalog.pg_namespace WHERE
substring(pg_catalog.quote_ident(nspname) || '.',1,0) =
substring('',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) > 1
UNION
SELECT pg_catalog.quote_ident(n.nspname) || '.' ||
pg_catalog.quote_ident(c.relname) FROM pg_catalog.pg_class c,
pg_catalog.pg_namespace n WHERE c.relnamespace = n.oid AND c.relkind IN ('r',
'S', 'v', 'm', 'f', 'p') AND substring(pg_catalog.quote_ident(n.nspname) || '.'
|| pg_catalog.quote_ident(c.relname),1,0)='' AND
substring(pg_catalog.quote_ident(n.nspname) || '.',1,0) =
substring('',1,pg_catalog.length(pg_catalog.quote_ident(n.nspname))+1) AND
(SELECT pg_catalog.count(*) FROM pg_catalog.pg_namespace WHERE
substring(pg_catalog.quote_ident(nspname) || '.',1,0) =
substring('',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) = 1
LIMIT 1000
LOG: duration: 1.871 ms statement: SELECT pg_catalog.quote_ident(c.relname)
FROM pg_catalog.pg_class c WHERE c.relkind IN ('r', 'S', 'v', 'm', 'f', 'p')
AND substring(pg_catalog.quote_ident(c.relname),1,0)='' AND
pg_catalog.pg_table_is_visible(c.oid) AND c.relnamespace <> (SELECT oid FROM
pg_catalog.pg_namespace WHERE nspname = 'pg_catalog')
UNION
SELECT pg_catalog.quote_ident(n.nspname) || '.' FROM pg_catalog.pg_namespace n
WHERE substring(pg_catalog.quote_ident(n.nspname) || '.',1,0)='' AND (SELECT
pg_catalog.count(*) FROM pg_catalog.pg_namespace WHERE
substring(pg_catalog.quote_ident(nspname) || '.',1,0) =
substring('',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) > 1
UNION
SELECT pg_catalog.quote_ident(n.nspname) || '.' ||
pg_catalog.quote_ident(c.relname) FROM pg_catalog.pg_class c,
pg_catalog.pg_namespace n WHERE c.relnamespace = n.oid AND c.relkind IN ('r',
'S', 'v', 'm', 'f', 'p') AND substring(pg_catalog.quote_ident(n.nspname) || '.'
|| pg_catalog.quote_ident(c.relname),1,0)='' AND
substring(pg_catalog.quote_ident(n.nspname) || '.',1,0) =
substring('',1,pg_catalog.length(pg_catalog.quote_ident(n.nspname))+1) AND
(SELECT pg_catalog.count(*) FROM pg_catalog.pg_namespace WHERE
substring(pg_catalog.quote_ident(nspname) || '.',1,0) =
substring('',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) = 1
LIMIT 1000
foo pg_catalog. pg_toast.public.
information_schema. pg_temp_1. pg_toast_temp_1.
