Re: [COMMITTERS] pgsql: Improve performance of timezone loading, especially pg_timezone_
On 8 May 2017 at 02:57, Tom Lane wrote: > David Rowley writes: >> OK, so it looks like GenerateTimezoneFiles in Install.pm for the MSVC >> build does not quite do what make install does for src/timezone. >> Nothing seems to pass the -p parameter as the following is doing: > > Ah-hah. Still, the log message should have been there before. > Maybe just nobody noticed? Thanks for pushing. The error messages were not there before. I think just load_ok would have ended up false in tzparse(). There seems to be code to handle that without producing any error. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve performance of timezone loading, especially pg_timezone_
On 7 May 2017 at 21:33, David Rowley wrote: > I've attached a patch for review. My perl skills are at "trial and > error" level, so please review carefully. Actually, my Perl code contained a superfluous line to trim the whitespace from $posixrules. The attached is a version without it, which still appears to work. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services posixrules_fix_v2.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve performance of timezone loading, especially pg_timezone_
On 7 May 2017 at 21:03, David Rowley wrote: > Perhaps we just need to put the NUL char back, to trim off the filename again: > > /* If that didn't work, fall through to do it the hard way */ > fullname[fullnamelen] = '\0'; > > but I've not yet looked into why the file is missing in the first place. OK, so it looks like GenerateTimezoneFiles in Install.pm for the MSVC build does not quite do what make install does for src/timezone. Nothing seems to pass the -p parameter as the following is doing: install: all installdirs ifeq (,$(with_system_tzdata)) $(ZIC) -d '$(DESTDIR)$(datadir)/timezone' -p '$(POSIXRULES)' $(TZDATAFILES) I've attached a patch for review. My perl skills are at "trial and error" level, so please review carefully. The attached also adds the NUL char back to fullname in pg_open_tzfile(). -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services posixrules_fix.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve performance of timezone loading, especially pg_timezone_
, On 7 May 2017 at 17:19, Tom Lane wrote:
> Amit Kapila writes:
>> On Wed, May 3, 2017 at 7:21 AM, Tom Lane wrote:
>>> Improve performance of timezone loading, especially pg_timezone_names view.
>
>> I am consistently getting below error after this commit in my Win7 machine:
>> running bootstrap script ... 2017-05-07 05:09:13.383 GMT [7908] LOG:
>> could not open directory "/installation/share/timezone/posixrules": No
>> such file or directory
>
>> This occurs both during initdb and server start.
>
> You sure it wasn't there before, too? That commit did not add any
> file reads that didn't happen before.
It didn't exist before either, but I don't think the error is what you
think. Please note the error message indicated it's looking for a
"directory" named posixrules, not the file.
The error seems to be due to this code:
if (canonname == NULL)
{
int result;
fullname[fullnamelen] = '/';
/* test above ensured this will fit: */
strcpy(fullname + fullnamelen + 1, name);
result = open(fullname, O_RDONLY | PG_BINARY, 0);
if (result >= 0)
return result;
/* If that didn't work, fall through to do it the hard way */
}
where we append the name, then try to open the file, but since the
file does not exist result is < 0 and we fall through to the for(;;)
loop and try to do scan_directory_ci() on the filename rather than the
directory.
On windows, where I also get the error, the stacktrace, as of
b58c433ef90b is as follows:
postgres.exe!scan_directory_ci(const char * dirname, const char *
fname, int fnamelen, char * canonname, int canonnamelen) Line 162 C
> postgres.exe!pg_open_tzfile(const char * name, char * canonname) Line 127 C
postgres.exe!tzloadbody(const char * name, char * canonname, state *
sp, char doextend, local_storage * lsp) Line 240 C
postgres.exe!tzload(const char * name, char * canonname, state * sp,
char doextend) Line 564 C
postgres.exe!tzparse(const char * name, state * sp, char lastditch) Line 959 C
postgres.exe!tzloadbody(const char * name, char * canonname, state *
sp, char doextend, local_storage * lsp) Line 421 C
postgres.exe!tzload(const char * name, char * canonname, state * sp,
char doextend) Line 564 C
postgres.exe!pg_tzset(const char * name) Line 291 C
postgres.exe!check_log_timezone(char * * newval, void * * extra,
GucSource source) Line 415 C
postgres.exe!call_string_check_hook(config_string * conf, char * *
newval, void * * extra, GucSource source, int elevel) Line 9905 C
postgres.exe!parse_and_validate_value(config_generic * record, const
char * name, const char * value, GucSource source, int elevel,
config_var_val * newval, void * * newextra) Line 5834 C
postgres.exe!set_config_option(const char * name, const char *
value, GucContext context, GucSource source, GucAction action, char
changeVal, int elevel, char is_reload) Line 6437 C
postgres.exe!ProcessConfigFileInternal(GucContext context, char
applySettings, int elevel) Line 441 C
postgres.exe!ProcessConfigFile(GucContext context) Line 158 C
postgres.exe!SelectConfigFiles(const char * userDoption, const char
* progname) Line 4810 C
postgres.exe!PostmasterMain(int argc, char * * argv) Line 854 C
postgres.exe!main(int argc, char * * argv) Line 229 C
postgres.exe!__tmainCRTStartup() Line 536 C
postgres.exe!mainCRTStartup() Line 377 C
kernel32.dll!7ffdab5e13d2() Unknown
ntdll.dll!7ffdadf754e4() Unknown
Perhaps we just need to put the NUL char back, to trim off the filename again:
/* If that didn't work, fall through to do it the hard way */
fullname[fullnamelen] = '\0';
but I've not yet looked into why the file is missing in the first place.
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Identity columns
On 7 April 2017 at 00:44, Peter Eisentraut wrote: > Identity columns > > This is the SQL standard-conforming variant of PostgreSQL's serial > columns. It fixes a few usability issues that serial columns have: > > - CREATE TABLE / LIKE copies default but refers to same sequence > - cannot add/drop serialness with ALTER TABLE > - dropping default does not drop sequence > - need to grant separate privileges to sequence > - other slight weirdnesses because serial is some kind of special macro Attached is a small patch which fixes up a warning for compilers not smart enough to know the elog(ERROR) does not return. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services identity_columns_warning_fix.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Faster expression evaluation and targetlist projection.
On 26 March 2017 at 09:55, Thomas Munro wrote: > On Sun, Mar 26, 2017 at 11:11 AM, Andres Freund wrote: >> Faster expression evaluation and targetlist projection. >> >> This replaces the old, recursive tree-walk based evaluation, with >> non-recursive, opcode dispatch based, expression evaluation. >> Projection is now implemented as part of expression evaluation. >> >> This both leads to significant performance improvements, and makes >> future just-in-time compilation of expressions easier. > > This is a huge achievement. Congratulations! Agreed. Congratulations! -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: ICU support
On 24 March 2017 at 06:33, Peter Eisentraut wrote:
> http://git.postgresql.org/pg/commitdiff/eccfef81e1f73ee41f1d8bfe4fa4e80576945048
[...]
> src/include/utils/pg_locale.h| 32 +-
I see this commit changed the definition of pg_locale_t
+struct pg_locale_t
+{
+ charprovider;
+ union
+ {
#ifdef HAVE_LOCALE_T
-typedef locale_t pg_locale_t;
-#else
-typedef int pg_locale_t;
+ locale_t lt;
+#endif
+#ifdef USE_ICU
+ struct {
+ const char *locale;
+ UCollator *ucol;
+ } icu;
#endif
+ } info;
+};
but forgot to update varstr_cmp() completely.
result = wcscoll_l((LPWSTR) a1p, (LPWSTR) a2p, mylocale);
should be:
result = wcscoll_l((LPWSTR) a1p, (LPWSTR) a2p, mylocale->info.lt);
Patch attached.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
icu_fix.patch
Description: Binary data
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: dblink: Replace some macros by static functions
On 11 March 2017 at 03:46, Peter Eisentraut wrote: > dblink: Replace some macros by static functions > > Also remove some unused code and the no longer useful dblink.h file. > > Reviewed-by: Tsunakawa, Takayuki > > Branch > -- > master > > Details > --- > http://git.postgresql.org/pg/commitdiff/acaf7ccb94a3916ea83712671a3563 > f0eb595558 > > Modified Files > -- > contrib/dblink/dblink.c | 290 +++--- > -- > contrib/dblink/dblink.h | 39 --- > 2 files changed, 137 insertions(+), 192 deletions(-) > Hi Peter, Please accept a small patch which fixes a new compiler warning which started as a result of this commit. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services dblink_compiler_warning_fix.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Suppress compiler warnings in ecpg test on newer Windows toolcha
On 25 February 2017 at 10:45, Tom Lane wrote: > Suppress compiler warnings in ecpg test on newer Windows toolchains. > > nan_test.pgc supposed that it could unconditionally #define isnan() > and isinf() on WIN32. This was evidently copied at some point from > src/include/port/win32.h, but nowadays there's a test on _MSC_VER > there. Make nan_test.pgc look the same. > > Per buildfarm warnings. There's no evidence this produces anything > worse than a warning, and besides it's only a test case, so I don't > feel a need to back-patch. > > Branch > -- > master > > Details > --- > http://git.postgresql.org/pg/commitdiff/c5658a0764d5ac5ea8c2c11d27c62d5472234227 > > Modified Files > -- > .../ecpg/test/expected/pgtypeslib-nan_test.c | 102 +++--- > .../ecpg/test/expected/pgtypeslib-nan_test.stderr | 354 ++--- > src/interfaces/ecpg/test/pgtypeslib/nan_test.pgc | 2 + > 3 files changed, 231 insertions(+), 227 deletions(-) This seems to have caused some new compiler warnings [1] on earlier MSVC toolchains. The reason seems to be that these older versions require float.h to be included for _isnan() [2] So anyway, we seem to be including float.h in all the other places we're using isnan(), so the attached adds float.h to the files where we're getting the warnings too. [1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=thrips&dt=2017-03-07%2022%3A37%3A28&stg=make [2] https://msdn.microsoft.com/en-us/library/tzthab44(v=vs.110).aspx -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services isnan_msvc_warning_fixes.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Provide errno-translation wrappers around bind() and listen() on
On 13 April 2016 at 11:52, Tom Lane wrote: > Provide errno-translation wrappers around bind() and listen() on Windows. > > I've seen one too many "could not bind IPv4 socket: No error" log entries > from the Windows buildfarm members. Per previous discussion, this is > likely caused by the fact that we're doing nothing to translate > WSAGetLastError() to errno. Put in a wrapper layer to do that. > > If this works as expected, it should get back-patched, but let's see what > happens in the buildfarm first. > > Discussion: <4065.1452450...@sss.pgh.pa.us> My windows machine gives me a bunch of new warnings with this: src/backend/libpq/auth.c(1620): warning C4047: 'function' : 'int *' differs in levels of indirection from 'size_t' [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/libpq/auth.c(1620): warning C4024: 'pgwin32_bind' : different types for formal and actual parameter 3 [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/libpq/auth.c(2606): warning C4047: 'function' : 'int *' differs in levels of indirection from 'int' [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/libpq/auth.c(2606): warning C4024: 'pgwin32_bind' : different types for formal and actual parameter 3 [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/libpq/pqcomm.c(493): warning C4047: 'function' : 'int *' differs in levels of indirection from 'size_t' [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/libpq/pqcomm.c(493): warning C4024: 'pgwin32_bind' : different types for formal and actual parameter 3 [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/port/win32/socket.c(272): warning C4047: 'function' : 'int' differs in levels of indirection from 'int *' [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/port/win32/socket.c(272): warning C4024: 'bind' : different types for formal and actual parameter 3 [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/postmaster/pgstat.c(396): warning C4047: 'function' : 'int *' differs in levels of indirection from 'size_t' [L:\Projects\Postgres\a\postgres.vcxproj] src/backend/postmaster/pgstat.c(396): warning C4024: 'pgwin32_bind' : different types for formal and actual parameter 3 [L:\Projects\Postgres\a\postgres.vcxproj] I'm not really sure why you made pgwin32_bind take a pointer to an int, instead of just an int. I assume a mistake? The attached fixes and gets rid of the warnings. pgwin32_bind_fix.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Copyedit comments and documentation.
On 4 April 2016 at 12:11, Noah Misch wrote: > On Mon, Apr 04, 2016 at 10:55:03AM +1200, David Rowley wrote: >> On 2 April 2016 at 14:56, Noah Misch wrote: >> > Copyedit comments and documentation. >> >> ... >> > src/backend/storage/freespace/freespace.c | 4 ++-- >> >> - * Returns the physical block number an FSM page >> + * Returns the physical block number of a FSM page >> >> I'm not sure I want to start a debate over such a trivial matter, so >> if you think I'm wrong then I'll probably back down quickly. >> >> I'd say that "an" was correct and "a" is wrong. >> >> I'd pronounce "FSM" as "ef-es-em". I can't really imagine an >> alternative pronunciation for it. > > As utterances, both "a free space map page" and "an ef es em page" seem > plausible to me. I was there to fix the missing preposition. Then, I > wondered about the article. Finding that the other two examples in > freespace.c use "a FSM," I made this line match the majority. That seems fair. Thanks for explaining. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Copyedit comments and documentation.
On 4 April 2016 at 10:55, David Rowley wrote: > It appears that the tender is still open on "a SQL" vs "an SQL", but > that'll likely be down to people pronouncing SQL as either > "es-que-el", or "squeal" Of course I mean "sequel". -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Copyedit comments and documentation.
On 2 April 2016 at 14:56, Noah Misch wrote: > Copyedit comments and documentation. ... > src/backend/storage/freespace/freespace.c | 4 ++-- - * Returns the physical block number an FSM page + * Returns the physical block number of a FSM page I'm not sure I want to start a debate over such a trivial matter, so if you think I'm wrong then I'll probably back down quickly. I'd say that "an" was correct and "a" is wrong. I'd pronounce "FSM" as "ef-es-em". I can't really imagine an alternative pronunciation for it. It appears that the tender is still open on "a SQL" vs "an SQL", but that'll likely be down to people pronouncing SQL as either "es-que-el", or "squeal" -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Best-guess attempt at fixing MSVC build for 68ab8e8ba4a471d9.
On 22 March 2016 at 02:33, Andres Freund wrote:
> On 2016-03-21 09:25:18 -0400, Tom Lane wrote:
>> Shouldn't we revert 6f1f34c92b11593e? AFAICS this patch isn't depending
>> on that, but maybe I miss something.
>
> Hm. I'd guess that at least part of it is still required. AddSimpleFrontend()
> afaics processes these variables, and I don't think the msvc stuff will
> otherwise generate psqlscan.c in the first place. We probably could
> move the knowledge to the explicit AddSimpleFrontend('pgbench')
> site. But I'm not personally excited about tinkering with it...
Yeah, don't revert that. It half fixed things.
As far as I can see both your changes were needed. On testing it seems
the change to $frontend_extraincludes was required, but it all still
works after removing src/bin/psql/psqlscan.l from
$frontend_extrasource, although that's most likely only because psql
compiled first, and created psqlscan.c. So I think both changes are
required, we just end up running flex on psqlscan.l twice.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Best-guess attempt at fixing MSVC build for 68ab8e8ba4a471d9.
On 21 March 2016 at 10:52, Tom Lane wrote:
> Best-guess attempt at fixing MSVC build for 68ab8e8ba4a471d9.
>
> pgbench now needs to use src/bin/psql/psqlscan.l, but it's not very clear
> how to fit that into the MSVC build system. If this doesn't work I'm going
> to need some help from somebody who actually understands those scripts ...
I see this didn't fix the problem :-(
Perl is not my native tongue, but after a little study and some
testing on a windows machine, the attached seems to fix the problem.
I've never much looked at these script before, but it seems that the
parsing of the Makefile just assumes that psqlscan.c is in the same
path as the rest of the stuff. The patch just uses some already
defined file replace functions to switch the wrong filename out for
the correct one before the Visual Studios project file is created.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 3d929e6..ab65fa3 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -71,7 +71,7 @@ my $frontend_extrasource = {
'src/bin/psql/psqlscan.l' ] };
my @frontend_excludes = (
'pgevent', 'pg_basebackup', 'pg_rewind', 'pg_dump',
- 'pg_xlogdump', 'scripts');
+ 'pg_xlogdump', 'scripts', 'pgbench');
sub mkvcbuild
{
@@ -674,6 +674,11 @@ sub mkvcbuild
}
$pg_xlogdump->AddFile('src/backend/access/transam/xlogreader.c');
+ # fix up pgbench once it's been set up
+ # we're borrowing psqlscan.c from psql, so grab it from the correct
place
+ my $pgbench = AddSimpleFrontend('pgbench');
+ $pgbench->ReplaceFile('src/bin/pgbench/psqlscan.c',
'src/bin/psql/psqlscan.c');
+
$solution->Save();
return $solution->{vcver};
}
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Improve index AMs' opclass validation procedures.
On 22 January 2016 at 13:47, Tom Lane wrote: > Improve index AMs' opclass validation procedures. > > The amvalidate functions added in commit 65c5fcd353a859da were on the > crude side. Improve them in a few ways: > > * Perform signature checking for operators and support functions. > > * Apply more thorough checks for missing operators and functions, > where possible. > > * Instead of reporting problems as ERRORs, report most problems as INFO > messages and make the amvalidate function return FALSE. This allows > more than one problem to be discovered per run. > > * Report object names rather than OIDs, and work a bit harder on making > the messages understandable. > > Also, remove a few more opr_sanity regression test queries that are > now superseded by the amvalidate checks. I'm getting 3 new warnings with this change which mention about 32-bit bit shifting then bitwise anding to a 64-bit variable. The attached fixes the warnings -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services amvalidate_64bit_bitshift.patch Description: Binary data -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
