using index or check in ALTER TABLE SET NOT NULL

Hello I write patch to speed up ALTER TABLE SET NOT NULL by check existed check constraints or indexes. Huge phase 3 with verify table data will be skipped if table has valid check constraint cover "alteredfield IS NOT NULL" condition or by SPI query if found index with compatible condition or

Re: using index or check in ALTER TABLE SET NOT NULL

Here is new patch with check only existed valid constraints and without SPI at all. Thanksdiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d979ce2..7ab7580 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -370,6 +370,8

Re: using index or check in ALTER TABLE SET NOT NULL

Thank you for review! My apologies for my errors. It seems i read developers wiki pages not enough carefully. I will reread wiki, code style and then update patch with all your remarks. > The comment /* T if we added new NOT NULL constraints */ should > probably be changed to /* T if we should

Re: proposal: alternative psql commands quit and exit

Why not just use ctrl+D shortcut? This EOF signal works both in bash, mysql, psql, any CLI tool which I remember

Re: using index or check in ALTER TABLE SET NOT NULL

Hello I update patch and also rebase to current head. I hope now it is better aligned with project stylediff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index dd4a8d3..36bcb3f 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@

Re: Allow reload recovery.conf during recovery

Hello Michael Thank you, i understand your opinion. I really tried to find a discussion about reload recovery.conf (or walreceiver conninfo changing and similar stuff), not about GUC. regards, Sergei

Re: Having query cache in core

> Having been bitten by the feature on MySQL, I think it's not a good thing. Even in MySQL itself this feature was already removed.

Allow reload recovery.conf during recovery

Hello all I want propose patch to make possible change primary_conninfo, primary_slot_name, restore_command and trigger_file in recovery.conf without restart postgresql. Startup process will reread recovery.conf on SIGHUP. My primary usecase is postgresql cluster with streaming replication.

Re: pg_basebackup -k option

Hi I think having only long option is enough regards, Sergei

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

Hello I found SELECT pg_stat_statements_reset(NULL,NULL,s.queryid) in tests and it pass tests, but i wonder how it works. Should not we check the NULL through PG_ARGISNULL macro before any PG_GETARG_*? According src/include/fmgr.h > * If function is not marked "proisstrict" in pg_proc, it must

Log query parameters for terminated execute

Hello all We already have feature to logging query parameters. If we use log_statement = 'all' we write parameters before execution and all is fine here. If we use log_min_duration_statement statement is logged obviously after execution, but currently we have no parameters if query was

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

Hello > key.userid = GetUserId(); We can remove query id only from current user, right? Maybe better provide optional argument for userid? Typically user with pg_read_all_stats and user for application queries are different users. At least it should be documented. regards, Sergei

Re: Online enabling of checksums

Hello I tried build this patch and got error during make docs > postgres.sgml:19626: element xref: validity error : IDREF attribute linkend > references an unknown ID "runtime-checksumhelper-cost-limit" > postgres.sgml:19625: element xref: validity error : IDREF attribute linkend > references

Re: Anyone keep mirrors of old packages from apt.postgresql.org?

Hello Do you know http://atalia.postgresql.org/morgue/ repository? > copy of postgresql-9.6-dbg_9.6.5-1.pgdg80+1_amd64.deb anywhere Here is http://atalia.postgresql.org/morgue/p/postgresql-9.6/postgresql-9.6-dbg_9.6.5-1.pgdg80%2b1_amd64.deb regards, Sergei

Re: numeric regression test passes, but why?

Hello I am surprised, but i can confirm error on versions prior 9.6: on 9.5, 9.4, 9.3 same error. On 9.6 and 10 query works correctly All checked postgresql are x86_64 from postgresql debian repository: http://apt.postgresql.org/pub/repos/apt/ jessie-pgdg main postgres=# select 0.5678::numeric

Re: [HACKERS] Restricting maximum keep segments by repslots

Hello >> patch -p1 gives some "Stripping trailing CRs from patch" >> messages for me, but applied to current HEAD and builds. After > > Hmm. I wonder why I get that complaint so often. (It's rather > common? or caused by the MIME format of my mail?) I'd say with > confidence that it is because

Re: Identifying ALTER TABLE "sub-command"

HelloYou can start with functions ATPrepCmd and ATExecCmd in src/backend/commands/tablecmds.cAlso note, one alter table statement can have multiple different actions. Regards, Sergei11.01.2018, 18:56, "Jeremy Finzel" :Hello -I have found that in leveraging the parser code to

Re: Is this a bug?

Hello No, this is not bug. https://www.postgresql.org/docs/10/static/hot-standby.html#HOT-STANDBY-CONFLICT Message in DETAIL describe conflict reason. > Application of a vacuum cleanup record from WAL conflicts with queries > accessing the target page on the standby, whether or not the data to

Re: Using scalar function as set-returning: bug or feature?

Hello > select into b from my_insert('from func atx'); You missed select something into b. For example, select ret into b from my_insert('from func atx') as ret; Using scalar function in from is not bug. Silent assigning NULL for variables in "into" not matches same in "select"... I think

Re: [HACKERS] Restricting maximum keep segments by repslots

Hello I think limit wal in replication slots is useful in some cases. But first time i was confused with proposed terminology secured/insecured/broken/unknown state. patch -p1 gives some "Stripping trailing CRs from patch" messages for me, but applied to current HEAD and builds. After little

Continue work on changes to recovery.conf API

Hello all I would like to continue work on new recovery api proposed in thread [1]. We have some form of consensus but thread has been inactive for a long time and i hope i can help. I start from last published patch [2] and make some changes: - updated to current HEAD - made the patch pass

Re: Log query parameters for terminated execute

Hello > Changed the status as per last email of Tom Lane. I have no idea what can i change. I received no feedback how i can print parameters in other cases. Should i mark CF as rejected? regards, Sergei

Re: Standby trying "restore_command" before local WAL

Hello > As mentioned by others, it sounds like we could have an option to try > contacting the primary before running restore_commnad Why about primary? If we have restore_command on slave (or during point in time recovery) - we force using XLOG_FROM_ARCHIVE, even if XLOG_FROM_PG_WAL source can

Re: Online enabling of checksums

Hello I think one restart is acceptable for such feature. I doubt user want often disable-enable checksums. In many cases checksums will be enabled one time during all cluster life. We need more downtimes for minor updates (4/year) and changes in config PGC_POSTMASTER (max_connections or

Re: Online enabling of checksums

Hi > This doesn't test the consequences of the restart being skipped, nor > does it review on a code level the correctness. I check not only one stuff during review. I look code too: bgworker checksumhelper.c registered with: > bgw.bgw_start_time = BgWorkerStart_RecoveryFinished; And then

Re: Online enabling of checksums

Hi > They fail over to a secondary to do maintenance on a primary. But this is not problem even in current patch state. We can restart replica before failover and it works. I tested this behavior during my review. We can: - call pg_enable_data_checksums() on master - wait change data_checksums

Re: Indicate anti-wraparound autovacuum in log_autovacuum_min_duration

Hello > Yes, a bit more verbosity would be nice for that. Using the term > "anti-wraparound" directly in the logs makes the most sense? I used "(to prevent wraparound)" to looks like reporting in pg_stat_activity. As far i can see currently we not using "anti-wraparound" for user messages. On

Re: [FEATURE PATCH] pg_stat_statements with plans (v02)

Hello I notice latest patch version (v06) not applied after da6f3e45ddb68ab3161076e120e7c32cfd46d1db commit in april. We have some consensus with design of this feature? I will mark CF entry as waiting on author regards, Sergei

Re: Online enabling of checksums

The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: not tested Spec compliant: not tested Documentation:tested, failed Hello As i wrote few weeks ago i can not build documentation due

Re: Log query parameters for terminated execute

Hello Thank you for review! Well, i can miss some cases. I'm not sure about overall design of this patch. Is acceptable add errdetail_params to statement_timeout ereport in such way? After shutdown signal we must be in aborted state, so we mustn't call user-defined I/O functions. (quotation

Indicate anti-wraparound autovacuum in log_autovacuum_min_duration

Hello Currently log_autovacuum_min_duration log message has no difference between regular autovacuum and to prevent wraparound autovacuum. There are important differences, for example, backend can automatically cancel regular autovacuum, but not anti-wraparound. I think it is useful indicate

Re: Online enabling of checksums

ith actual start background worker only at recovery end (or promote). I think better using DEBUG ereport in postmaster and LOG in checksumhelper. regards, Sergei 25.07.2018, 12:35, "Daniel Gustafsson" : >>  On 24 Jul 2018, at 11:05, Sergei Kornilov wrote: >> >>  The

Re: Log query parameters for terminated execute

Hello 23.07.2018, 17:08, "Tom Lane" : > Sergei Kornilov writes: >>  Please test with logging command tag %i in log_line_prefix. Extended >> protocol has three different messages, each can be canceled by timeout. But >> here is completely no parameters in

Re: Continue work on changes to recovery.conf API

Hello Current patch moves recovery.conf settings into GUC system: - if startup process found recovery.conf - it throw error - recovery mode is turned on if new special file recovery.signal found - standby_mode setting was removed. Standby mode can be enabled if startup found new special file

Re: Can I just reload the slave to change primary_conninfo?

Hi > This parameter is defined in postgresql.conf Huh, i believe it be in future. Currently it is recovery.conf parameter, and yes - it can be set (or changed) only at database start. regards, Sergei

Re: Can I just reload the slave to change primary_conninfo?

Hello > Is there any reason this cannot be changed via a signal?  Is it a general > lack of infrastructure or is it there significant problem we want to ensure > never happens? Just make possible reload recovery.conf is not what the pgsql-hackers community want. My patch with exactly this

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

Hello > Currently pg_stat_statements_reset() and other stats reset functions like > pg_stat_reset() can be executed only by superusers. > But why did you allow pg_read_all_stats role to execute that function, > by default? That change looks strange to me. This is not behavior change, only fix

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

06.07.2018, 22:35, "Robert Haas" : > On Fri, Jul 6, 2018 at 1:26 PM, Fujii Masao wrote: >>  Hmm... so pg_stat_statements_reset() is allowed to be executed by >>  pg_read_all_stats role while other stats reset functions like >>  pg_stat_reset() can be executed only by superusers. Which looks >>  

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed I reviewed and tested patch one more times, including check

Re: using index or check in ALTER TABLE SET NOT NULL

Hello Stephen I changed the suggested things and added some regression tests. Also i wrote few words to the documentation. New patch is attached. Thank you for feedback! regards, Sergeidiff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 286c7a8..cf2c56f

Re: using index or check in ALTER TABLE SET NOT NULL

Hello > You should be able to use an event trigger that raises a message when > table_rewrite is hit, to notify the test driver that a rewrite happens. Here is no table rewrite, only verify. So here EventTriggerTableRewrite is not called. Or i missed something? > ISTM that depending on DEBUG

Re: using index or check in ALTER TABLE SET NOT NULL

Hello > Do you actually need test output proving that this code path was taken > rather than the default one? Seems like looking at the code coverage > report might be enough. I not known. In v4 i use DEBUG1 message and do not check code path in tests at all: by full table scan or by

Re: using index or check in ALTER TABLE SET NOT NULL

Hello My patch does not apply after commit 5748f3a0aa7cf78ac6979010273bd9d50869bb8e. Here is update to current master. Not null constraint is immutable too, so here is no changes in PartConstraintImpliedByRelConstraint excepts rename and comments fix. In this patch version i also revert tests

Re: using index or check in ALTER TABLE SET NOT NULL

Hello all! Summarizing, how i must update tests? We want test real skip of verify phase or only result correctness? I can verify what i do not break alter table by tests in v4 of my patch. But here is no check code path. So my feature may be broken in future without test fail. alter table will

Re: using index or check in ALTER TABLE SET NOT NULL

Hello, Ildar Thanks. I looked ATTACH PARTITION tests and found such checks. If no one is against i will just use in my patch INFO level instead DEBUG1, similarly ATTACH PARTITION code. Updated patch attached. Or i can rewrite tests to use DEBUG1 level. regards, Sergeidiff --git

Re: Custom PGC_POSTMASTER GUC variables ... feasible?

Hello See for example contrib pg_stat_statements extension. Setting pg_stat_statements.max defined as PGC_POSTMASTER regards, Sergei

Re: using index or check in ALTER TABLE SET NOT NULL

Hi I noticed new merge conflict, updated version attached. regards, Sergeidiff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index bd22627..db98a98 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -215,8 +215,15 @@ WITH (

Re: using index or check in ALTER TABLE SET NOT NULL

Hello I notice my patch does not apply again. Here is update to current HEAD regards, Sergeidiff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index bd22627..db98a98 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -215,8

Re: using index or check in ALTER TABLE SET NOT NULL

peter.eisentr...@2ndquadrant.com>: > On 11/29/17 10:52, Sergei Kornilov wrote: >>  My target problem of adding NOT NULL to big relation without long downtime >> can be done with ADD CONSTRAINT NOT VALID, VALIDATE it in second >> transaction, then SET NOT NULL by my patch and drop un

Re: using index or check in ALTER TABLE SET NOT NULL

Hello Attached updated patch follows recent Reorganize partitioning code commit. regards, Sergeidiff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index bd22627..db98a98 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -215,8

Re: Reopen logfile on SIGHUP

Hello Something was wrong? Attached file is empty. regards, Sergei

Re: Commit fest 2017-11

Hello I can not find "Reports" in bottom any page of CF app... Such stats covers only reviews marked in CF app? Through "Comment"->"Review" buttons? I'm afraid this statistics will be inaccurate for new users (like me). Wiki page https://wiki.postgresql.org/wiki/Reviewing_a_Patch say > Send

Re: using index or check in ALTER TABLE SET NOT NULL

Hello thank you for review! > Adding check constraint will also force the full table scan. So I think > it would be better to rephrased it as follows: Agree. I updated docs in new attached patch slightly different > Regarding regression tests it may be useful to set client_min_messages > to

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

Hello Thank you for update It may be better to use NULL as the default value at sql level. > ereport(LOG, (errmsg("userid %u, dbid %u, queryid %ld does not exist", > userid, dbid, queryid))); I think LOG level is not useful here. In common case this is server log only. How about WARNING? Or

Re: Continue work on changes to recovery.conf API

Hello I complete new version of this patch. I've attached it. In this version i remove proposed recovery_target_type/recovery_target_value and implement set of current settings: recovery_target (immediate), recovery_target_name, recovery_target_time, recovery_target_xid, recovery_target_lsn

Re: pg11rc1 DROP INDEX: NOTICE: drop_trigger

Hi Maybe you have some event trigger? Please check pg_event_trigger system view: https://www.postgresql.org/docs/11/static/catalog-pg-event-trigger.html regards, Sergei

Re: JSON validation behavior

Hi > I could get behind fixing > it to always throw the error, but that's not what Sergei was hoping for. On the contrary i think it is reasonable way. It is much better to have error on input value instead of finding wrong value during table processing. We always reject this value for jsonb

JSON validation behavior

Hi We have some json regression tests in src/test/regress/expected/json_encoding_1.out with \u symbol select json '{ "a": "null \u escape" }' as not_unescaped; not_unescaped { "a": "null \u escape" } (1 row) select json '{ "a":

Re: JSON validation behavior

Hi 24.10.2018, 17:40, "David G. Johnston" : > On Wed, Oct 24, 2018 at 7:25 AM Sergei Kornilov wrote: > >> DETAIL:  \u cannot be converted to text. >> >> Well, requested text type can not have \u byte. But seems strange: we >> test json t

Re: [HACKERS] generated columns

Hi > OK, so the problem is COPY. > > Which means we have an issue with restore. We need to be able to pg_dump a > table with generated columns, then restore it afterwards. More generally, we > need to be able to handle data that has already been generated - the > "generate" idea should apply

Re: [HACKERS] generated columns

Hi I applied this patch on top 2fe42baf7c1ad96b5f9eb898161e258315298351 commit and found a bug while adding STORED column: postgres=# create table test(i int); CREATE TABLE postgres=# insert into test values (1),(2); INSERT 0 2 postgres=# alter table test add column gen_stored integer GENERATED

Re: [HACKERS] generated columns

Hi > patch --dry-run --ignore-whitespace -p 0 -F 5 < > v5-0001-Generated-columns.patch > > and varied both -p and -F paramaters to no avail. Am I doing it wrong? I am able apply patch by command patch -p1 < v5-0001-Generated-columns.patch or by "git apply v5-0001-Generated-columns.patch", but

Re: Continue work on changes to recovery.conf API

Hi I attached new version of this patch due merge conflict with pg_promote function. regards, Sergeidiff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c index ee1fbd7..946239c 100644 --- a/contrib/pg_standby/pg_standby.c +++ b/contrib/pg_standby/pg_standby.c @@ -611,7

Re: syntax error: VACUUM ANALYZE VERBOSE (PostgreSQL 11 regression)

Hi At least this is documented behavior: > When the option list is surrounded by parentheses, the options can be written > in any order. Without parentheses, options must be specified in exactly the > order shown above. https://www.postgresql.org/docs/current/static/sql-vacuum.html Previously

pg_ls_dir_files issue with concurrent file unlink

Hello We have some functions for directory listing, e.g. pg_ls_dir, pg_ls_logdir, pg_ls_waldir. All of them call pg_ls_dir_files function (src/backend/utils/adt/genfile.c) and i have question about per file error in this function. > /* Get the file info */ >

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

doesn't seem to >>  have any clear purpose. So, I don't see much value in breaking >>  compatibility. >> >>  Does anyone else have an opinion on this matter? > > This was proposed by Sergei Kornilov in > https://postgr.es/m/3368121530260...@web21g.yandex.ru saying th

Re: Index Skip Scan

Hello Last published patch index-skip-fallback-v2 applied and builds cleanly. I found reproducible crash due assert failure: FailedAssertion("!(numCols > 0)", File: "pathnode.c", Line: 2795) > create table tablename (i int primary key); > select distinct i from tablename where i = 1; Query is

Re: Continue work on changes to recovery.conf API

Hello Thank you! Here is patch update addressing your comments. > - useless whitespace change in xlog.c Oops, did not notice. Fixed. > - I think we can drop logRecoveryParameters(). Users can now inspect > those parameters using the normal GUC mechanisms. (They were all DEBUG2 > anyway, so it's

Re: using index or check in ALTER TABLE SET NOT NULL

Hello > This patch went through the last tree commit fests without any noticeable > activity Well, last two CF. During march commitfest patch has activity and was marked as ready for committer. But at end of july CF was no further activity and patch was reverted back to "need review" with

Re: Continue work on changes to recovery.conf API

Hello thank you for review! >>  - if present both standby.signal and recovery.signal - we use standby mode >>  (this design from previous thread) > > Didn't find the discussion on that and don't understand the reasons, but > seems OK and easy to change if we don't like it. I meant this was

Re: pgsql: Improve autovacuum logging for aggressive and anti-wraparound ru

Hello, Robert > My first question was whether TWO of them were dead code ... isn't an > aggressive vacuum to prevent wraparound, and a vacuum to prevent > wraparound aggressive? Maybe i am wrong, aggressive autovacuum was your commit. Message split was in b55509332f50f998b6e8b3830a51c5b9d8f666aa

Re: pgsql: Improve autovacuum logging for aggressive and anti-wraparound ru

Hi > An autovacuum can't be just aggressive; it's either anti-wraparound or normal. But autovacuum _can_ be aggressive and not anti-wraparound. I build current master and can see 3 different line types: 2018-09-24 23:47:31.500 MSK 27939 @ from [vxid:4/272032 txid:0] [] LOG: automatic

Re: [HACKERS] REINDEX CONCURRENTLY 2.0

Hello > I don't want a situation like this: > CREATE INDEX CONCURRENTLY ... > DROP INDEX CONCURRENTLY ... > REINDEX INDEX (CONCURRENTLY) ... > > All three should be the same, and my suggestion is to add the > parenthesized version to CREATE and DROP and not add the unparenthesized >

Re: Making WAL receiver startup rely on GUC context for primary_conninfo and primary_slot_name

Hello > This allows the state of walrcv and startup to diverge, they could e.g. have > different configuration, due to differently time config reloads. So we have exactly same problem with, for example, hot_standby_feedback, right? We change hot_standby_feedback value, reload it and we can have

Re: allow online change primary_conninfo

Hello > Do we no longer need static version of conninfo/slotname in > walreceiver.c? We can detect a change of the variables without > them (as you did in the previous version.). Depends on this discussion: https://www.postgresql.org/message-id/20181212053042.gk17...@paquier.xyz If walreceiver

Re: could recovery_target_timeline=latest be the default in standby mode?

Hello I am +1 for recovery_target_timeline=latest by default. This is common case in my opinion. And first release without recovery.conf is reasonable time for change default value. But i doubt if we can ignore recovery_target_timeline in standby and always use latest in standby. I have no

Re: allow online change primary_conninfo

oops, forgot attach patch 11.12.2018, 13:14, "Sergei Kornilov" : > Hello > > After some thinking i can rewrite this patch in another way. > > This is better or worse? > > regards, Sergeidiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 4a7121

Re: allow online change primary_conninfo

Hello After some thinking i can rewrite this patch in another way. This is better or worse? regards, Sergei

Re: allow online change primary_conninfo

Hello thank you for review! > but I think that this patch should document clearly that if > primary_conninfo or primary_slot_name are changed then the WAL receiver > is stopped immediately. Good idea, will change. > /* > - * replication slot name; is also used for walreceiver to connect

Re: Making WAL receiver startup rely on GUC context for primary_conninfo and primary_slot_name

Hi Not sure i can be reviewer since this was initially my proposal. I vote +1 for this patch. Code looks good and i think we have no reason to leave RequestXLogStreaming as-is. regards, Sergei

Re: allow online change primary_conninfo

Hi > I would recommend waiting for the conclusion of other thread before > moving on with this one. Agreed. I mark this patch as Waiting on Author. Not quite correct for waiting another discussion, but most suitable. > We could also consider using > the show hook function of a parameter to

Re: ATTACH/DETACH PARTITION CONCURRENTLY

Hello > OK. Rebased again, and committed (although I forgot to include a link > to this discussion - sorry about that). Seems we erroneously moved this thread to next CF: https://commitfest.postgresql.org/21/1842/ Can you close this entry? regards, Sergei

Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

Hello > My preference is for NULL to mean *all* so this is my favorite option, > except that the first query should reset everything. I am +1 for this option. NULL treat as "any" and pg_stat_statements_reset(NULL,NULL,NULL) to reset everything We can use named notation?

Re: Frontends including fd.h

Hello I am +1 to have separate header with path names. Another examples are: - pg_ctl with many hardcoded files such as "promote" (PROMOTE_SIGNAL_FILE), "logrotate" (LOGROTATE_SIGNAL_FILE), postgresql.conf, PG_VERSION, backup_label, postmaster.pid - pg_basebackup with currently hardcoded

Re: [HACKERS] REINDEX CONCURRENTLY 2.0

Hello Thank you for working on this patch! I perform some tests and think behavior with partition tables is slightly inconsistent. postgres=# reindex table measurement; WARNING: REINDEX of partitioned tables is not yet implemented, skipping "measurement" NOTICE: table "measurement" has no

Re: [HACKERS] REINDEX CONCURRENTLY 2.0

Hello I review code and documentation and i have few notes. Did you register this patch in CF app? I found one error in phase 4. Simple reproducer: > create table test (i int); > create index this_is_very_large_exactly_maxnamelen_index_name_wink_wink_wink > on test (i); > create index

Re: New GUC to sample log queries

Hi > Ah, I feared that some compiler would not be smart enough to be silent > about that. I hope it does not emit a warning with this patch? Yes, with this change build is clean. Thank you PS: currently i use old gcc (Debian 4.9.2-10+deb8u1) 4.9.2 regards, Sergei

Re: New GUC to sample log queries

Hello I can not build current HEAD cleanly. I got warning: > postgres.c: In function ‘check_log_duration’: > postgres.c:2254:17: warning: ‘in_sample’ may be used uninitialized in this > function [-Wmaybe-uninitialized] > if ((exceeded && in_sample) || log_duration) Should not we have such

Re: Continue work on changes to recovery.conf API

HelloThank you! Regards, Sergei

allow online change primary_conninfo

Hello all Now we integrate recovery.conf into GUC system. So i would like to start discussion to make primary_conninfo and primary_slot_name PGC_SIGHUP. My work-in-progress patch attached. I think we have no futher reason to have a copy of conninfo and slot name in WalRcvData, so i propose

Re: allow online change primary_conninfo

Hi >>  I think we have no futher reason to have a copy of conninfo and slot name >> in WalRcvData, so i propose remove these fields from >> pg_stat_get_wal_receiver() (and pg_stat_wal_receiver view). This data can be >> accesible now via regular GUC commands. > > Is that wise? It seems

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hi > The buildfarm is unhappy after this one: > https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=culicidae=HEAD > > If I use -DEXEC_BACKEND when compiling the tests complain about a > timeout in 003_recovery_targets. Here is what the buildfarm reports, I > can see the failure by myself

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hello >>  - recovery_target = immediate was replaced with recovery_target_immediate >> bool GUC > > Why? Due this comment: https://www.postgresql.org/message-id/20181126172118.GY3415%40tamriel.snowman.net > I've not been following this very closely, but seems like > recovery_target_string is a

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hello > I would think we'd have the different GUCs and then the check functions > would only validate that they're valid inputs and then when we get to > the point where we're starting to do recovery we check and make sure > we've been given a sane overall configuration- which means that only >

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hi > The attached seems to be the simplest way to fix this. (Needs > documentation updates, test updates, error message refinement.) Thank you! I add documentation change, tests and rephrased error message. regards, Sergeidiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hello Updated patch attached: - added testcase to verify database does not start with multiple recovery targets - recovery_target = immediate was replaced with recovery_target_immediate bool GUC thank you! regards, Sergeidiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hi > What is the reason for allowing multiple recovery_target_* settings and > taking the last one? Could we change this aspect to make this behave > better? Correct response to user configuration, similar to old recovery.conf logic or other GUC - we allow redefine with rule "latest win". I

Re: allow online change primary_conninfo

Hi >>  Hmm... I considered SIGHUP processing was in fast loop and therefore >> shutdown should be fast. But i recheck code and found a possible long loop >> without processing SIGHUP (in case we receive new data faster than writes to >> disk). Ok, i will revert back. >>  How about write to

Re: pgsql: Integrate recovery.conf into postgresql.conf

Hello > I think I would have done 'if (targetSettingsCount != 1)' here. Streaming replication does not have any recovery_target. Well, i can check StandbyModeRequested too and allow 0 recovery_target only for standby mode. >>  -# Multiple targets >>  -# Last entry has priority (note that an

Re: Continue work on changes to recovery.conf API

Hi > Why don't we put recovery entries into postgresql.recovery.conf or such? > And overwrite rather than append? Appending to postgressql.auto.conf instead of writing new hardcoded file was proposed here:

  1   2   3   >