RE: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented
>-Original Message-
>From: Fujii Masao
>Sent: Monday, October 4, 2021 1:59 PM
>To: bt21tanigaway ; RDH 加藤 慎也/Kato,
>Shinya (NTT DATA)
>Cc: pgsql-hackers@lists.postgresql.org
>Subject: Re: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet
>implemented
>
>
>
>On 2021/10/04 11:17, bt21tanigaway wrote:
else if (Matches("LOCK", MatchAny, "IN", "ACCESS|ROW") ||
- Matches("LOCK", "TABLE", MatchAny, "IN",
"ACCESS|ROW"))
+ Matches("LOCK", "TABLE", MatchAny, "IN",
>"ACCESS|ROW")
+||
+ Matches("LOCK", "ONLY", MatchAny, "IN",
>"ACCESS|ROW")
+||
+ Matches("LOCK", "TABLE", "ONLY", MatchAny, "IN",
+"ACCESS|ROW"))
>>> I think this code is redundant, so I change following.
>>> ---
>>> else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW"))
>>> ---
>>> I created the patch, and attached it. Do you think?
>> Thank you for update!
>> I think that your code is more concise than mine.
>> There seems to be no problem.
>
>The patch looks good to me, too. I applied cosmetic changes to it.
>Attached is the updated version of the patch. Barring any objection, I will
>commit
>it.
Thank you for the patch!
It looks good to me.
Regards,
Shinya Kato
RE: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented
>Thank you for your feedback.
>I might have added whitespace when I was checking the patch file.
>I attach a new patch to this mail.
Thank you for the update!
> else if (Matches("LOCK", MatchAny, "IN", "ACCESS|ROW") ||
>- Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW"))
>+ Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW")
>||
>+ Matches("LOCK", "ONLY", MatchAny, "IN", "ACCESS|ROW")
>||
>+ Matches("LOCK", "TABLE", "ONLY", MatchAny, "IN",
>"ACCESS|ROW"))
I think this code is redundant, so I change following.
---
else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW"))
---
I created the patch, and attached it. Do you think?
>> 2. The command "LOCK TABLE a, b;" can be executed, but tab-completion
>> doesn't work properly. Is it OK?
>It's OK for now.
>But it should be able to handle a case of multiple tables in the future.
OK. I agreed.
Regards,
Shinya Kato
fix_tab_completion_of_lock.patch
Description: fix_tab_completion_of_lock.patch
RE: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented
>-Original Message- >From: bt21tanigaway >Sent: Tuesday, September 28, 2021 5:06 PM >To: Fujii Masao ; >pgsql-hackers@lists.postgresql.org >Subject: Re: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet >implemented > >2021-09-28 17:03 に bt21tanigaway さんは書きました: >> 2021-09-28 16:36 に Fujii Masao さんは書きました: >>> On 2021/09/28 16:13, bt21tanigaway wrote: Hi, (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented in tab-complete. I made a patch for these options. >>> >>> Thanks for the patch! >>> The patch seems to forget to handle the tab-completion for "LOCK ONLY >>> IN". >> >> Thanks for your comment! >> I attach a new patch fixed to this mail. >> >> Regards, >> >> Koyu Tanigawa > >Sorry, I forgot to attach patch file. >"fix-tab-complete2.patch" is fixed! > >Regards, > >Koyu Tanigawa Thank you for your patch. I have two comments. 1. When I executed git apply, an error occured. --- $ git apply ~/Downloads/fix-tab-complete2.patch /home/penguin/Downloads/fix-tab-complete2.patch:14: indent with spaces. COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, " UNION SELECT 'TABLE'" " UNION SELECT 'ONLY'"); warning: 1 line adds whitespace errors. --- 2. The command "LOCK TABLE a, b;" can be executed, but tab-completion doesn't work properly. Is it OK? -- Regards, -- Shinya Kato Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
RE: [PATCH] New default role allowing to change per-role/database settings
>Thanks for letting me know, I've attached a rebased v4 of this patch, no other
>changes.
I tried it, but when I used set command, tab completion did not work properly
and an error occurred.
---
postgres=> \conninfo
You are connected to database "postgres" as user "aaa" via socket in "/tmp" at
port "5432".
postgres=> \du
List of roles
Role name | Attributes |
Member of
---++---
aaa ||
{pg_change_role_settings}
penguin | Superuser, Create role, Create DB, Replication, Bypass RLS | {}
postgres=> show log
log_autovacuum_min_durationlog_executor_stats
log_min_error_statementlog_replication_commands
log_timezone
log_checkpointslog_file_mode
log_min_messages log_rotation_age
log_transaction_sample_rate
log_connectionslog_hostname
log_parameter_max_length log_rotation_size
log_truncate_on_rotation
log_destinationlog_line_prefix
log_parameter_max_length_on_error log_statement
logging_collector
log_disconnections log_lock_waits
log_parser_stats log_statement_sample_rate
logical_decoding_work_mem
log_duration log_min_duration_sample
log_planner_stats log_statement_stats
log_error_verbositylog_min_duration_statement
log_recovery_conflict_waitslog_temp_files
postgres=> show log_duration ;
log_duration
--
off
(1 row)
postgres=> set log
log_parameter_max_length_on_error logical_decoding_work_mem
postgres=> set log_duration to on;
2021-09-08 16:23:39.216 JST [533860] ERROR: permission denied to set parameter
"log_duration"
2021-09-08 16:23:39.216 JST [533860] STATEMENT: set log_duration to on;
ERROR: permission denied to set parameter "log_duration"
---
Regards,
--
Shinya Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
fix DECLARE tab completion
Hi! In the discussion of [1], the option ASENSITIVE was added to DECLARE. I have improved its tab completion and attached a patch. Do you think? [1] https://www.postgresql.org/message-id/flat/96ee8b30-9889-9e1b-b053-90e10c050e85%40enterprisedb.com Regards, Shinya Kato fix_declare_tab_completion.patch Description: fix_declare_tab_completion.patch
RE: [PATCH] expand the units that pg_size_pretty supports on output
>I had not really looked at the patch, but if there's a cleanup portion to the >same >patch as you're adding the YB too, then maybe it's worth separating those out >into another patch so that the two can be considered independently. I agree with this opinion. It seems to me that we should think about units and refactoring separately. Sorry for the confusion. Best regards, Shinya Kato
RE: [PATCH] expand the units that pg_size_pretty supports on output
>> I don't see the need to extend the unit to YB. >> What use case do you have in mind? > >Practical or no, I saw no reason not to support all defined units. I assume >we’ll >get to a need sooner or later. :) Thank you for your reply! Hmmm, I didn't think YB was necessary, but what do others think? Best regards, Shinya Kato
RE: psql - factor out echo code
>> Wouldn't it be better to comment it like any other function? > >Sure. Attached. Thank you for your revision. I think this patch is good, so I will move it to ready for committer. Best regards, Shinya Kato
RE: [PATCH] expand the units that pg_size_pretty supports on output
>From: David Christensen >Sent: Friday, June 4, 2021 4:18 AM >To: PostgreSQL-development >Subject: Re: [PATCH] expand the units that pg_size_pretty supports on output > >New versions attached to address the initial CF feedback and rebase on HEAD as >of now. > >0001-Expand-the-units-that-pg_size_pretty-numeric-knows-a.patch > >- expands the units that pg_size_pretty() can handle up to YB. > >0002-Expand-the-supported-units-in-pg_size_bytes-to-cover.patch > >- expands the units that pg_size_bytes() can handle, up to YB. > I don't see the need to extend the unit to YB. What use case do you have in mind? Regards, Shinya Kato
RE: psql - factor out echo code
>-Original Message- >From: Fabien COELHO >Sent: Sunday, May 30, 2021 6:10 PM >To: PostgreSQL Developers >Subject: psql - factor out echo code > > >While working on something in "psql/common.c" I noticed some triplicated code, >including a long translatable string. This minor patch refactors this in one >function. > >-- >Fabien. Wouldn't it be better to comment it like any other function? Best regards, Shinya Kato
RE: Fix pg_checksums progress report
>-Original Message- >From: Fujii Masao >Sent: Friday, April 2, 2021 6:03 PM >To: Michael Paquier ; shinya11.k...@nttdata.com >Cc: pgsql-hack...@postgresql.org >Subject: Re: Fix pg_checksums progress report > > > >On 2021/04/02 16:47, Michael Paquier wrote: >> On Fri, Apr 02, 2021 at 07:30:32AM +, shinya11.k...@nttdata.com wrote: >>> I added a comment to the patch, and attached the new patch. > >Thanks for updating the patch! > >+ /* >+ * The current_size is calculated before checking if header is a >+ * new page, because total_size includes the size of new >pages. >+ */ >+ current_size += r; > >I'd like to comment more. What about the following? > >--- >Since the file size is counted as total_size for progress status information, >the >sizes of all pages including new ones in the file should be counted as >current_size. Otherwise the progress reporting calculated using those counters >may not reach 100%. >--- Thanks for your review! I updated the patch, and attached it. Regards, Shinya Kato fix_pg_checksums_progress_report_v3.patch Description: fix_pg_checksums_progress_report_v3.patch
RE: Fix pg_checksums progress report
>-Original Message- >From: Fujii Masao >Sent: Friday, April 2, 2021 2:39 PM >To: shinya11.k...@nttdata.com; pgsql-hack...@postgresql.org >Subject: Re: Fix pg_checksums progress report > > > >On 2021/04/02 14:23, shinya11.k...@nttdata.com wrote: >> Hi, >> >> I found a problem with the pg_checksums.c. >> >> The total_size is calculated by scanning the directory. >> The current_size is calculated by scanning the files, but the current_size >> does >not include the size of NewPages. >> >> This may cause pg_checksums progress report to not be 100%. >> I have attached a patch that fixes this. > >Thanks for the report and patch! > >I could reproduce this issue and confirmed that the patch fixes it. > >Regarding the patch, I think that it's better to add the comment about why >current_size needs to be counted including new pages. Thanks for your review. I added a comment to the patch, and attached the new patch. Regards, Shinya Kato fix_pg_checksums_progress_report_v2.patch Description: fix_pg_checksums_progress_report_v2.patch
Fix pg_checksums progress report
Hi, I found a problem with the pg_checksums.c. The total_size is calculated by scanning the directory. The current_size is calculated by scanning the files, but the current_size does not include the size of NewPages. This may cause pg_checksums progress report to not be 100%. I have attached a patch that fixes this. Regards, Shinya Kato fix_pg_checksums_progress_report.patch Description: fix_pg_checksums_progress_report.patch
RE: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
>-Original Message- >From: Fujii Masao >Sent: Monday, March 22, 2021 11:22 AM >To: shinya11.k...@nttdata.com; da...@pgmasters.net; movead...@highgo.ca >Cc: pgsql-hack...@postgresql.org; and...@anarazel.de; mich...@paquier.xyz; >ahsan.h...@highgo.ca; horikyota@gmail.com >Subject: Re: Wrong statistics for size of XLOG_SWITCH during pg_waldump. > > > >On 2021/03/19 18:27, Fujii Masao wrote: >> >> >> On 2021/03/19 15:06, shinya11.k...@nttdata.com wrote: >> But 0 value maybe looks strange, so in current version I show it like >>below: >> Type N (%) Record size (%) FPI size (%) Combined size (%) >> - --- --- --- --- - --- ... >> XLOG/SWITCH_JUNK - ( -) 11006248 ( 72.26) - ( -) 11006248 ( 65.78) >> Transaction/COMMIT 10 ( 0.03) 340 ( 0.00) 0 ( 0.00) 340 ( 0.00) > > I just wanted to know why the "count" and "fpi_len" fields 0 are. > So, it would be nice to have 0 values. Sorry for confusing you. Kato, it's not clear to me if you were asking for - to be changed back to 0? You marked the patch as Ready for Committer so I assume not, but it would be better to say clearly that you think this patch is ready for a >committer to look at. >>> >>> Yes, I don't think it needs to be changed back to 0. >>> I think this patch is ready for a committer to look at. >> >> What's the use case of this feature? I read through this thread >> briefly, but I'm still not sure how useful this feature is. >> >> Horiguchi-san reported one issue upthread; --stats=record shows two >> lines for Transaction/COMMIT record. Probably this should be fixed >> separately. >> >> Horiguchi-san, >> Do you have updated version of that bug-fix patch? >> Or you started another thread for that issue? > >I confirmed that only XACT records need to be processed differently. >So the patch that Horiguchi-san posted upthread looks good and enough to me. >I added a bit more detail information into the comment in the patch. >Attached is the updated version of the patch. Since this issue looks like a >bug, >I'm thinking to back-patch that. Thought? > >Barring any objection, I will commit this. I think it's good except for a typo "thoes four bits" Regards, Shinya Kato
RE: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
>>>But 0 value maybe looks strange, so in current version I show it like >below: >>>Type N (%) Record size (%) FPI size (%) Combined size (%) >>> - --- --- --- --- - --- ... >>>XLOG/SWITCH_JUNK - ( -) 11006248 ( 72.26) - ( -) 11006248 ( 65.78) >>>Transaction/COMMIT 10 ( 0.03) 340 ( 0.00) 0 ( 0.00) 340 ( 0.00) >> >> I just wanted to know why the "count" and "fpi_len" fields 0 are. >> So, it would be nice to have 0 values. Sorry for confusing you. > >Kato, it's not clear to me if you were asking for - to be changed back to 0? > >You marked the patch as Ready for Committer so I assume not, but it would be >better to say clearly that you think this patch is ready for a committer to >look at. Yes, I don't think it needs to be changed back to 0. I think this patch is ready for a committer to look at. Regards, Shinya Kato
RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
>On Thu, Jan 7, 2021 at 1:30 PM Fujii Masao
> wrote:
>>
>> On 2021/01/07 12:42, Masahiko Sawada wrote:
>> > On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao
>> > wrote:
>> >>
>> >> On 2021/01/07 10:01, Masahiko Sawada wrote:
>> >>> On Wed, Jan 6, 2021 at 3:37 PM
>> >>> wrote:
>>
>> > +#define Query_for_list_of_cursors \
>> > +" SELECT name FROM pg_cursors"\
>> >
>> > This query should be the following?
>> >
>> > " SELECT pg_catalog.quote_ident(name) "\
>> > " FROM pg_catalog.pg_cursors "\
>> > " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>> >
>> > +/* CLOSE */
>> > + else if (Matches("CLOSE"))
>> > + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> > + " UNION ALL SELECT 'ALL'");
>> >
>> > "UNION ALL" should be "UNION"?
>>
>> Thank you for your advice, and I corrected them.
>>
>> >> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> >> + " UNION SELECT 'ABSOLUTE'"
>> >> + " UNION SELECT 'BACKWARD'"
>> >> + " UNION SELECT 'FORWARD'"
>> >> + " UNION SELECT 'RELATIVE'"
>> >> + " UNION SELECT 'ALL'"
>> >> + " UNION SELECT 'NEXT'"
>> >> + " UNION SELECT 'PRIOR'"
>> >> + " UNION SELECT 'FIRST'"
>> >> + " UNION SELECT 'LAST'"
>> >> + " UNION SELECT 'FROM'"
>> >> + " UNION SELECT 'IN'");
>> >>
>> >> This change makes psql unexpectedly output "FROM" and "IN" just after
>> >> "FETCH".
>> >
>> > I think "FROM" and "IN" can be placed just after "FETCH". According to
>> > the documentation, the direction can be empty. For instance, we can do
>> > like:
>>
>> Thank you!
>>
>> > I've attached the patch improving the tab completion for DECLARE as an
>> > example. What do you think?
>> >
>> > BTW according to the documentation, the options of DECLARE statement
>> > (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>> >
>> > DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>> > CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>> >
>> > But I realized that these options are actually order-insensitive. For
>> > instance, we can declare a cursor like:
>> >
>> > =# declare abc scroll binary cursor for select * from pg_class;
>> > DECLARE CURSOR
>> >
>> > The both parser code and documentation has been unchanged from 2003.
>> > Is it a documentation bug?
>>
>> Thank you for your patch, and it is good.
>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO
>> SCROLL) are order-sensitive."
>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any
>> order." , according to the documentation.
>> >>>
>> >>> Thanks, you're right. I missed that sentence. I still don't think the
>> >>> syntax of DECLARE statement in the documentation doesn't match its
>> >>> implementation but I agree that it's order-insensitive.
>> >>>
>> I made a new patch, but the amount of codes was large due to
>> order-insensitive.
>> >>>
>> >>> Thank you for updating the patch!
>> >>>
>> >>> Yeah, I'm also afraid a bit that conditions will exponentially
>> >>> increase when a new option is added to DECLARE statement in the
>> >>> future. Looking at the parser code for DECLARE statement, we can put
>> >>> the same options multiple times (that's also why I don't think it
>> >>> matches). For instance,
>> >>>
>> >>> postgres(1:44758)=# begin;
>> >>> BEGIN
>> >>> postgres(1:44758)=# declare test binary binary binary cursor for
>> >>> select * from pg_class;
>> >>> DECLARE CURSOR
>> >>>
>> >>> So how about simplify the above code as follows?
>> >>>
>> >>> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int
>> >>> end)
>> >>> else if (Matches("DECLARE", MatchAny))
>> >>> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>> >>> "CURSOR");
>> >>> + /*
>> >>> + * Complete DECLARE with one of BINARY, INSENSITIVE, SCROLL,
>> >>> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
>> >>> + * DECLARE, assume we want options.
>> >>> + */
>> >>> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
>> >>> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
>> >>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>> >>> + "CURSOR");
>> >>
>> >> This change seems to cause "DECLARE ... CURSOR FOR SELECT " to
>> >> unexpectedly output BINARY, INSENSITIVE, etc.
>> >
>> > Indeed. Is the following not complete but much better?
>>
>> Yes, I think that's better.
>>
>> >
>> > @@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int
>> > end)
>> > " UNION SELECT 'ALL'");
>> >
>> > /* DECLARE */
>> > - else if (Matches("DECLARE", MatchAny))
>> > - COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>> > - "CURSOR");
>> > + /*
>> > + * Complete DECLARE with one of BINARY, INSENSITIVE, SCROLL,
>> > + * NO SCROLL, and CURSOR. If the tail is any one of
RE: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
>Thanks for review, and sorry for reply so later.
>
>>I reviewed the patch and found some problems.
>>>+ if(startSegNo != endSegNo)
>>>+ else if(record->ReadRecPtr / XLOG_BLCKSZ !=
>>>+ if(rmid == RM_XLOG_ID && info == XLOG_SWITCH)
>>>+ if(ri == RM_XLOG_ID)
>>>+ if(info == XLOG_SWITCH)
>>You need to put a space after the "if".
>All fix and thanks for point the issue.
>
>>>@@ -24,6 +24,7 @@
>>>#include "common/logging.h"
>>>#include "getopt_long.h"
>>>#include "rmgrdesc.h"
>>>+#include "catalog/pg_control.h"
>>I think the include statements should be arranged in alphabetical order.
>Fix.
Thank you for your revision!
>>>+ info = (rj << 4) & ~XLR_INFO_MASK;
>>>+ if(info == XLOG_SWITCH)
>>>+ XLogDumpStatsRow(psprintf("XLOG/SWITCH_JUNK"),
>>>+ 0, total_count, stats->junk_size, total_rec_len,
>>>+ 0, total_fpi_len, stats->junk_size, total_len);
>
>>Can't be described in the same way as "XLogDumpStatsRow(psprintf("%s/%s",
>>desc->rm_name, id)..."?
>>Only this part looks strange.
>>Why are the "count" and "fpi_len" fields 0?
>The 'SWITCH_JUNK' is not a real record and it relys on 'XLOG_SWITCH' record,
>so I think we can't count
>'SWITCH_JUNK', so the "count" is 0. And it never contain FPI, so the "fpi_len"
>is 0.
>
>But 0 value maybe looks strange, so in current version I show it like below:
>Type N (%) Record size (%) FPI size (%) Combined size (%)
> - --- --- --- --- - ---
>...
>XLOG/SWITCH_JUNK - ( -) 11006248 ( 72.26) - ( -) 11006248 ( 65.78)
>Transaction/COMMIT 10 ( 0.03) 340 ( 0.00) 0 ( 0.00) 340 ( 0.00)
>
I just wanted to know why the "count" and "fpi_len" fields 0 are.
So, it would be nice to have 0 values. Sorry for confusing you.
Regards,
Shinya Kato
RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
>+#define Query_for_list_of_cursors \
>+" SELECT name FROM pg_cursors"\
>
>This query should be the following?
>
>" SELECT pg_catalog.quote_ident(name) "\
>" FROM pg_catalog.pg_cursors "\
>" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>
>+/* CLOSE */
>+ else if (Matches("CLOSE"))
>+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>+ " UNION ALL SELECT
>'ALL'");
>
>"UNION ALL" should be "UNION"?
Thank you for your advice, and I corrected them.
>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> + " UNION SELECT
>> 'ABSOLUTE'"
>> + " UNION SELECT
>> 'BACKWARD'"
>> + " UNION SELECT
>> 'FORWARD'"
>> + " UNION SELECT
>> 'RELATIVE'"
>> + " UNION SELECT 'ALL'"
>> + " UNION SELECT
>> 'NEXT'"
>> + " UNION SELECT
>> 'PRIOR'"
>> + " UNION SELECT
>> 'FIRST'"
>> + " UNION SELECT
>> 'LAST'"
>> + " UNION SELECT
>> 'FROM'"
>> + " UNION SELECT
>> 'IN'");
>>
>> This change makes psql unexpectedly output "FROM" and "IN" just after
>> "FETCH".
>
>I think "FROM" and "IN" can be placed just after "FETCH". According to
>the documentation, the direction can be empty. For instance, we can do
>like:
Thank you!
>I've attached the patch improving the tab completion for DECLARE as an
>example. What do you think?
>
>BTW according to the documentation, the options of DECLARE statement
>(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>
>DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>
>But I realized that these options are actually order-insensitive. For
>instance, we can declare a cursor like:
>
>=# declare abc scroll binary cursor for select * from pg_class;
>DECLARE CURSOR
>
>The both parser code and documentation has been unchanged from 2003.
>Is it a documentation bug?
Thank you for your patch, and it is good.
I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are
order-sensitive."
I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order."
, according to the documentation.
I made a new patch, but the amount of codes was large due to order-insensitive.
If you know of a better way, please let me know.
Regards,
Shinya Kato
fix_tab_complete_close_fetch_move_v3.patch
Description: fix_tab_complete_close_fetch_move_v3.patch
RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
Thank you for your review! I fixed some codes and attach a new patch. >When I applied the patch, I got the following whitespace warnings: > >$ git apply ~/patches/fix_tab_complete_close_fetch_move.patch >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:40: >indent with spaces. >COMPLETE_WITH_QUERY(Query_for_list_of_cursors >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:41: >indent with spaces. >" UNION SELECT 'ABSOLUTE'" >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:42: >indent with spaces. >" UNION SELECT 'BACKWARD'" >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:43: >indent with spaces. >" UNION SELECT 'FORWARD'" >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:44: >indent with spaces. >" UNION SELECT 'RELATIVE'" >warning: squelched 19 whitespace errors >warning: 24 lines add whitespace errors. > >I recommend you checking whitespaces or running pgindent. Thank you for your advice and I have corrected it. >Here are some comments: > >/* >-* Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, >RELATIVE, ALL, >-* NEXT, PRIOR, FIRST, LAST >+* Complete FETCH with a list of cursors and one of ABSOLUTE, >BACKWARD, FORWARD, RELATIVE, ALL, >+* NEXT, PRIOR, FIRST, LAST, FROM, IN > */ > >Maybe I think the commend should say: > >+* Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, >RELATIVE, ALL, >+* NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors > >Other updates of the comment seem to have the same issue. > >Or I think we can omit the details from the comment since it seems redundant >information. We can read it from the arguments of the following >COMPLETE_WITH_QUERY(). It certainly seems redundant, so I deleted them. >--- >- /* >-* Complete FETCH with "FROM" or "IN". These are equivalent, >-* but we may as well tab-complete both: perhaps some users prefer one >-* variant or the other. >-*/ >+ /* Complete FETCH with a list of cursors and "FROM" or >+ "IN" */ > >Why did you remove the second sentence in the above comment? I had misunderstood the meaning and deleted it. I deleted it as well as above, but would you prefer it to be there? >--- >The patch improves tab completion for CLOSE, FETCH, and MOVE but is there >any reason why you didn't do that for DECLARE? I think DECLARE also can be >improved and it's a good timing for that. I wanted to improve tab completion for DECLARE, but I couldn't find anything to improve. Please let me know if there are any codes that can be improved. Regards, Shinya Kato fix_tab_complete_close_fetch_move_v2.patch Description: fix_tab_complete_close_fetch_move_v2.patch
RE: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
Thanks for the reply. > Mr.Horiguchi.
I reviewed the patch and found some problems.
>+ if(startSegNo != endSegNo)
>+ else if(record->ReadRecPtr / XLOG_BLCKSZ !=
>+ if(rmid == RM_XLOG_ID && info == XLOG_SWITCH)
>+ if(ri == RM_XLOG_ID)
>+ if(info == XLOG_SWITCH)
You need to put a space after the "if".
>@@ -24,6 +24,7 @@
>#include "common/logging.h"
>#include "getopt_long.h"
>#include "rmgrdesc.h"
>+#include "catalog/pg_control.h"
I think the include statements should be arranged in alphabetical order.
>+ info = (rj << 4) & ~XLR_INFO_MASK;
>+ if(info == XLOG_SWITCH)
>+ XLogDumpStatsRow(psprintf("XLOG/SWITCH_JUNK"),
>+ 0, total_count, stats->junk_size, total_rec_len,
>+ 0, total_fpi_len, stats->junk_size, total_len);
Can't be described in the same way as "XLogDumpStatsRow(psprintf("%s/%s",
desc->rm_name, id)..."?
Only this part looks strange.
Why are the "count" and "fpi_len" fields 0?
I think you need to improve the duplicate output in column "XLOG/SWITCH_JUNK".
Regards,
Shinya Kato
[PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
Hi! I created a patch for improving CLOSE, FETCH, MOVE tab completion. Specifically, I add CLOSE, FETCH, MOVE tab completion for completing a predefined cursors. Regards, Shinya Kato fix_tab_complete_close_fetch_move.patch Description: fix_tab_complete_close_fetch_move.patch
RE: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
When I execute pg_waldump, I found that XLOG/SWITCH_JUNK appears twice. Is this problem solved by the way of correcting the previously discussed Transaction/COMMIT? $ ../bin/pg_waldump --stats=record ../data/pg_wal/00010001 Type N (%) Record size (%) FPI size (%)Combined size (%) - --- --- --- ---- --- XLOG/CHECKPOINT_SHUTDOWN 5 ( 0.01) 570 ( 0.01)0 ( 0.00) 570 ( 0.01) XLOG/CHECKPOINT_ONLINE 6 ( 0.02) 684 ( 0.02)0 ( 0.00) 684 ( 0.01) XLOG/NEXTOID 3 ( 0.01) 90 ( 0.00)0 ( 0.00) 90 ( 0.00) XLOG/FPI 290 ( 0.80)14210 ( 0.34) 638216 ( 40.72) 652426 ( 11.30) Transaction/COMMIT12 ( 0.03) 408 ( 0.01)0 ( 0.00) 408 ( 0.01) Transaction/COMMIT 496 ( 1.36) 134497 ( 3.20)0 ( 0.00) 134497 ( 2.33) Storage/CREATE13 ( 0.04) 546 ( 0.01)0 ( 0.00) 546 ( 0.01) CLOG/ZEROPAGE 1 ( 0.00) 30 ( 0.00)0 ( 0.00) 30 ( 0.00) Database/CREATE2 ( 0.01) 84 ( 0.00)0 ( 0.00) 84 ( 0.00) Standby/LOCK 142 ( 0.39) 5964 ( 0.14)0 ( 0.00) 5964 ( 0.10) Standby/RUNNING_XACTS 13 ( 0.04) 666 ( 0.02)0 ( 0.00) 666 ( 0.01) Standby/INVALIDATIONS136 ( 0.37)12416 ( 0.30)0 ( 0.00)12416 ( 0.22) Heap2/CLEAN 132 ( 0.36) 8994 ( 0.21)0 ( 0.00) 8994 ( 0.16) Heap2/FREEZE_PAGE245 ( 0.67) 168704 ( 4.01)0 ( 0.00) 168704 ( 2.92) Heap2/CLEANUP_INFO 2 ( 0.01) 84 ( 0.00)0 ( 0.00) 84 ( 0.00) Heap2/VISIBLE424 ( 1.16)25231 ( 0.60) 352256 ( 22.48) 377487 ( 6.54) XLOG/SWITCH_JUNK 0 ( 0.00)0 ( 0.00)0 ( 0.00)0 ( 0.00) Heap2/MULTI_INSERT 1511 ( 4.15) 287727 ( 6.84)12872 ( 0.82) 300599 ( 5.21) Heap2/MULTI_INSERT+INIT 46 ( 0.13)71910 ( 1.71)0 ( 0.00)71910 ( 1.25) Heap/INSERT 8849 ( 24.31) 1288414 ( 30.62)25648 ( 1.64) 1314062 ( 22.76) Heap/DELETE 25 ( 0.07) 1350 ( 0.03)0 ( 0.00) 1350 ( 0.02) Heap/UPDATE 173 ( 0.48)55238 ( 1.31) 5964 ( 0.38)61202 ( 1.06) Heap/HOT_UPDATE 257 ( 0.71)27585 ( 0.66) 1300 ( 0.08)28885 ( 0.50) XLOG/SWITCH_JUNK 0 ( 0.00)0 ( 0.00)0 ( 0.00)0 ( 0.00) Heap/LOCK180 ( 0.49) 9800 ( 0.23) 129812 ( 8.28) 139612 ( 2.42) Heap/INPLACE 214 ( 0.59)44520 ( 1.06)40792 ( 2.60)85312 ( 1.48) Heap/INSERT+INIT 171 ( 0.47) 171318 ( 4.07)0 ( 0.00) 171318 ( 2.97) Regards, Shinya Kato
