Re: [HACKERS] WIP: Separate log file for extension
Hi Antonin,
1. check-world run error as following:
for extra in contrib/adminpack; do make -C '../..'/$extra
DESTDIR='/db/pgmaster/postgres'/tmp_install install
>>'/db/pgmaster/postgres'/tmp_install/log/install.log || exit; done
In file included from ../../src/include/c.h:1135:0,
from ../../src/include/postgres.h:47,
from adminpack.c:15:
adminpack.c: In function ‘convert_and_check_filename’:
adminpack.c:84:38: error: ‘Log_directory’ undeclared (first use in this
function)
(!logAllowed || !is_absolute_path(Log_directory) ||
2. In function get_log_stream
The following code
if (strcmp(id, stream->id))
ereport(ERROR,
(errmsg("log stream with id \"%s\" already exists", id)));
should be " if (strcmp(id,stream->id)==0)" ?
3. In function pipeThread
csvlog_file can't be found but referred by the following statements:
if (ftell(syslog_file) >= Log_RotationSize * 1024L ||
(csvlog_file != NULL &&
4. In logfile_open function
LogStream *stream = &log_streams[stream_id];
int file_mode = stream->file_mode;
It may be better to be as following:
int file_mode = log_streams[stream_id].file_mode
--
Regards,
Jing Wang
Fujitsu Australia
Re: [HACKERS] Support to COMMENT ON DATABASE CURRENT_DATABASE
>Not surprisingly, this patch no longer applies in the wake of commit >b3f840120. Rather than rebasing the pg_dump portions, I would suggest >you just drop them. It has been removed from the pg_dump codes. >I notice some other patch application failures in dbcommands.c, >objectaddress.c, and user.c, so there's more work to do besides >this Yes. fixed it. Enclosed please find the latest patch fixed above. Regards, Jing Wang Fujitsu Australia support_CURRENT_DATABASE_keyword_v4.7.patch Description: Binary data
Re: Libpq support to connect to standby server as priority
Hi All, Recently I put a proposal to support 'prefer-read' parameter in target_session_attrs in libpq. Now I updated the patch with adding content in the sgml and regression test case. Some people may have noticed there is already another patch ( https://commitfest.postgresql.org/15/1148/ ) which looks similar with this. But I would say this patch is more complex than my proposal. It is better separate these 2 patches to consider. Regards, Jing Wang Fujitsu Australia libpq_support_perfer-read_002.patch Description: Binary data
Re: [HACKERS] Support to COMMENT ON DATABASE CURRENT_DATABASE
Hi Stephen and Thomas,
Thanks your review comments.
Enclosed please find the latest patch.
>/src/backend/parser/gram.y: In function ‘base_yyparse’:
>/src/backend/parser/gram.y:1160:19: warning: assignment from incompatible
pointer type [-Wincompatible-pointer-types]
>| IN_P DATABASE db_spec_name { $$ = $3; }
The warning has been dismissed.
>When it should be:
>ALTER DATABASE { name |
CURRENT_DATABASE } OWNER TO { new_owner |
CURRENT_USER | SESSION_USER }
Yes. It should be.
>Please don't include whitespace-only hunks, like this one:
Ok.
>The TAP regression tests for pg_dump are failing.
The test case has been updated.
>make makeDbSpec() return a DbSpec and then try to minimize the
>forced-casting happening.
Makes sense. It has been changed.
Regards,
Jing Wang
Fujitsu Australia
support_CURRENT_DATABASE_keyword_v4.6.patch
Description: Binary data
Re: Libpq support to connect to standby server as priority
Hi Takayuki, Thanks your reminder. I will have a look your patch and the review comments and update the patch soon. Please leave my current submitted patch now. -- Regards, Jing Wang Fujitsu Australia 2018-01-04 17:40 GMT+11:00 Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com>: > From: Jing Wang [mailto:jingwang...@gmail.com] > > This is a proposal that let libpq support 'prefer-read' option in > > target_session_attrs in pg_conn. The 'prefer-read' means the libpq will > > try to connect to a 'read-only' server firstly from the multiple server > > addresses. If failed to connect to the 'read-only' server then it will > try > > to connect to the 'read-write' server. > > There's a pending patch I started. I'd be happy if you could continue > this. > > https://commitfest.postgresql.org/15/1148/ > > Regards > Takayuki Tsunakawa > > -- Kind regards Jing
Re: Libpq support to connect to standby server as priority
Hi, Enclosed please find the patch that the libpq support 'prefer-read' feature. If the *target_session_attrs* is set to 'prefer-read', the patch will connect to server and send 'SHOW transaction_read_only' query to check the server being 'read-only' or not. If server is 'read-write' then it will try next server address. If all connections for 'read-only' get failed it will try to connect to the master server. -- Regards, Jing Wang Fujitsu Australia libpq_support_perfer-read_001.patch Description: Binary data
Libpq support to connect to standby server as priority
Hi Hackers, This is a proposal that let libpq support 'prefer-read' option in target_session_attrs in pg_conn. The 'prefer-read' means the libpq will try to connect to a 'read-only' server firstly from the multiple server addresses. If failed to connect to the 'read-only' server then it will try to connect to the 'read-write' server. By providing this feature the application can have the opportunity to connect to the standby server firstly if failed then connect to master server without caring the sequence of the server addresses provided to the libpq. The 'read-only' server means Standby Server The 'read-write' server means Master Server support to 'read-write' -- Regards, Jing Wang Fujitsu Australia
Re: [HACKERS] Support to COMMENT ON DATABASE CURRENT_DATABASE
Hi, I have rebased the patch on the latest version. Because the CURRENT_DATABASE can not only being used on COMMENT ON statement but also on other statements as following list so the patch name is renamed to "support_CURRENT_DATABASE_keyword_vxx.patch". 1. COMMENT ON DATABASE CURRENT_DATABASE is ... 2. ALTER DATABASE CURRENT_DATABASE OWNER to ... 3. ALTER DATABASE CURRENT_DATABASE SET parameter ... 4. ALTER DATABASE CURRENT_DATABASE RESET parameter ... 5. ALTER DATABASE CURRENT_DATABASE RESET ALL 6. SELECT CURRENT_DATABASE 7. SECURITY LABEL ON DATABASE CURRENT_DATABASE 8. ALTER ROLE ... IN DATABASE CURRENT_DATABASE SET/RESET configuration_parameter 9. GRANT ... ON DATABASE CURRENT_DATABASE TO role_specification ... 10. REVOKE ... ON DATABASE CURRENT_DATABASE FROM ... Regards, Jing Wang Fujitsu Australia support_CURRENT_DATABASE_keyword_v4.5.patch Description: Binary data
Re: [HACKERS] [PATCH] Vacuum: Update FSM more frequently
A few general comments.
+ FreeSpaceMapVacuum(onerel, 64);
Just want to know why '64' is used here? It's better to give a description.
+else
+ {
+ newslot = fsm_get_avail(page, 0);
+ }
Since there is only one line in the else the bracket will not be needed.
And there in one more space ahead of else which should be removed.
+ if (nindexes == 0 &&
+ (vacuumed_pages_at_fsm_vac - vacuumed_pages) >
vacuum_fsm_every_pages)
+ {
+ /* Vacuum the Free Space Map */
+ FreeSpaceMapVacuum(onerel, 0);
+ vacuumed_pages_at_fsm_vac = vacuumed_pages;
+ }
vacuumed_pages_at_fsm_vac is initialised with value zero and seems no
chance to go into the bracket.
Regards,
Jing Wang
Fujitsu Australia
Re: [HACKERS] Support to COMMENT ON DATABASE CURRENT_DATABASE
Hi All, This is a patch for current_database working on ALTER ROLE/GRANT/REVOKE statements which should be applied after the previous patch "comment_on_current_database_no_pgdump_v4.4.patch". By using the patch the CURRENT_DATABASE can working in the following SQL statements: ALTER ROLE ... IN DATABASE CURRENT_DATABASE SET/RESET configuration_parameter GRANT ... ON DATABASE CURRENT_DATABASE TO role_specification ... REVOKE ... ON DATABASE CURRENT_DATABASE FROM ... Regards, Jing Wang Fujitsu Australia current_database_on_grant_revoke_role_v4.4.patch Description: Binary data
Re: [HACKERS] Support to COMMENT ON DATABASE CURRENT_DATABASE
Hi Nathan, Thanks for review comments. Enclosed please find the patch which has been updated according to your suggestion. The CURRENT_DATABASE can be used as following SQL statements and people can find information from sgml files: 1. COMMENT ON DATABASE CURRENT_DATABASE is ... 2. ALTER DATABASE CURRENT_DATABASE OWNER to ... 3. ALTER DATABASE CURRENT_DATABASE SET parameter ... 4. ALTER DATABASE CURRENT_DATABASE RESET parameter ... 5. ALTER DATABASE CURRENT_DATABASE RESET ALL 6. SELECT CURRENT_DATABASE 7. SECURITY LABEL ON DATABASE CURRENT_DATABASE As your mentioned the database_name are also present in the GRANT/REVOKE/ALTER ROLE, so a patch will be present later for supporting CURRENT_DATABASE on these SQL statements. Regards, Jing Wang Fujitsu Australia comment_on_current_database_no_pgdump_v4.4.patch Description: Binary data comment_on_current_database_for_pgdump_v4.4.patch Description: Binary data
