Re: [HACKERS] Should we automatically run duplicate_oids?
Sent from my iPad On 02-Aug-2013, at 10:30, Bruce Momjian br...@momjian.us wrote: On Mon, Jul 8, 2013 at 06:25:44PM -0700, Peter Geoghegan wrote: When rebasing a patch that I'm working on, I occasionally forget to update the oid of any pg_proc.h entries I may have created. Of course this isn't a real problem; when I go to initdb, I immediately recognize what has happened. All the same, it seems like there is a case to be made for having this run automatically at build time, and having the build fail on the basis of there being a duplicate - this is something that fails reliably, but only when someone has added another pg_proc.h entry, and only when that other person happened to choose an oid in a range of free-in-git-tip oids that I myself fancied. Sure, I ought to remember to check this anyway, but it seems preferable to make this process more mechanical. I can point to commit 55c1687a as a kind of precedent, where the process of running check_keywords.pl was made to run automatically any time gram.c is rebuilt. Granted, that's a more subtle problem than the one I'm proposing to solve, but I still see this as a modest improvement. FYI, attached is the pgtest script I always run before I do a commit; it also calls src/tools/pgtest. It has saved me from erroneous commits many times. +1,a much needed thing.Duplicate oids is a pain enough to deserve its own solution. Regards, Atri -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On Mon, Jul 8, 2013 at 06:25:44PM -0700, Peter Geoghegan wrote: When rebasing a patch that I'm working on, I occasionally forget to update the oid of any pg_proc.h entries I may have created. Of course this isn't a real problem; when I go to initdb, I immediately recognize what has happened. All the same, it seems like there is a case to be made for having this run automatically at build time, and having the build fail on the basis of there being a duplicate - this is something that fails reliably, but only when someone has added another pg_proc.h entry, and only when that other person happened to choose an oid in a range of free-in-git-tip oids that I myself fancied. Sure, I ought to remember to check this anyway, but it seems preferable to make this process more mechanical. I can point to commit 55c1687a as a kind of precedent, where the process of running check_keywords.pl was made to run automatically any time gram.c is rebuilt. Granted, that's a more subtle problem than the one I'm proposing to solve, but I still see this as a modest improvement. FYI, attached is the pgtest script I always run before I do a commit; it also calls src/tools/pgtest. It has saved me from erroneous commits many times. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + : . traprm [ $1 = -d ] DOCS=Y shift export QUIET=$(($QUIET + 1)) . cd_pgtop chown -R postgres . echo Checking SGML cd doc/src/sgml make check $TMP/0 21 if grep -v 'fully-tagged' $TMP/0 | egrep -qi 'Error|Warning' thenecho SGML error cat $TMP/0 exit 1 fi [ $(pwd) != '/pgsql/8.4/doc/src/sgml' ] make check-tabs # Run only at night to check for HISTORY build problems # in HISTORY.html. if [ ! -t 0 -o $DOCS = Y ] thenmake INSTALL.html $TMP/0 21 if egrep -qi 'Error|Warning' $TMP/0 thenecho SGML error cat $TMP/0 exit 1 fi make HISTORY.html $TMP/0 21 if grep -q 'Error' $TMP/0 thenecho SGML error cat $TMP/0 exit 1 fi fi # fails on /bin/sh cd - echo Checking duplicate oids cd src/include/catalog duplicate_oids $TMP/0 if [ -s $TMP/0 ] thenecho Duplicate system oids cat $TMP/0 exit 1 fi cd - # supress assembler warning (aspg /pg/tools/pgtest $@; echo $? $TMP/ret) | # use only one grep so we don't buffer output egrep -v ': Warning: using `%|^SPI.c:.*: warning: |^ppport.h:[[:digit:]][[:digit:]]*: warning: |^/usr/libdata/perl5/5.00503/i386-bsdos/CORE/patchlevel.h|plperl.c:.*: warning: (implicit|passing)|variable .(fast|ptr|cmsg|fe_copy). might be clobbered|warning: unused variable .yyg.|gzwrite''' discards' rm -fr src/test/regress/tmp_check bell exit $(cat $TMP/ret) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On 07/08/2013 11:03 PM, Peter Geoghegan wrote:
On Mon, Jul 8, 2013 at 7:59 PM, Peter Eisentraut pete...@gmx.net wrote:
I don't think rewriting it in Perl is necessary or even desirable. I
don't see anything particularly unportable in that script as it is.
I was under the impression that the final patch ought to work on
Windows too. However, I suppose that since the number of people that
use windows as an everyday development machine is probably zero, we
could reasonably forgo doing anything on that platform.
Why the heck should we? To my certain knowledge there are people using
Windows as a development platform for PostgreSQL code, albeit not core
code. If we ever want to get them involved in writing core code we need
to treat them as first class citizens.
This is actually a pretty trivial task. Here is a simple perl version:
use strict;
my @files = (qw( toasting.h indexing.h), glob(pg_*.h));
my $handle;
my @lines;
foreach my $file (@files)
{
my $handle;
open($handle,$file) || die $!;
my @flines = $handle;
close($handle);
chomp @flines;
push(@lines, @flines);
}
my %oidcounts;
foreach (@lines)
{
next if /^CATALOG\(.*BKI_BOOTSTRAP/;
next unless
/^DATA\(insert *OID *= *([0-9][0-9]*).*$/ ||
/^CATALOG\([^,]*,
*([0-9][0-9]*).*BKI_ROWTYPE_OID\(([0-9][0-9]*)\).*$/ ||
/^CATALOG\([^,]*, *([0-9][0-9]*).*$/ ||
/^DECLARE_INDEX\([^,]*, *([0-9][0-9]*).*$/ ||
/^DECLARE_UNIQUE_INDEX\([^,]*, *([0-9][0-9]*).*$/ ||
/^DECLARE_TOAST\([^,]*, *([0-9][0-9]*), *([0-9][0-9]*).*$/;
$oidcounts{$1}++;
$oidcounts{$2}++ if $2;
}
my $found = 0;
foreach my $oid (sort {$a = $b} keys %oidcounts)
{
next unless $oidcounts{$oid} 1;
$found = 1;
print $oid\n;
}
exit $found;
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On 07/09/2013 10:40 AM, Andrew Dunstan wrote:
On 07/08/2013 11:03 PM, Peter Geoghegan wrote:
On Mon, Jul 8, 2013 at 7:59 PM, Peter Eisentraut pete...@gmx.net
wrote:
I don't think rewriting it in Perl is necessary or even desirable. I
don't see anything particularly unportable in that script as it is.
I was under the impression that the final patch ought to work on
Windows too. However, I suppose that since the number of people that
use windows as an everyday development machine is probably zero, we
could reasonably forgo doing anything on that platform.
Why the heck should we? To my certain knowledge there are people using
Windows as a development platform for PostgreSQL code, albeit not core
code. If we ever want to get them involved in writing core code we
need to treat them as first class citizens.
This is actually a pretty trivial task. Here is a simple perl version:
Slightly cleaner (and shorter) version:
use strict;
BEGIN
{
my @files = (qw( toasting.h indexing.h), glob(pg_*.h));
@ARGV = @files;
}
my %oidcounts;
while()
{
next if /^CATALOG\(.*BKI_BOOTSTRAP/;
next unless
/^DATA\(insert *OID *= *(\d+)/ ||
/^CATALOG\([^,]*, *(\d+).*BKI_ROWTYPE_OID\((\d+)\)/ ||
/^CATALOG\([^,]*, *(\d+)/ ||
/^DECLARE_INDEX\([^,]*, *(\d+)/ ||
/^DECLARE_UNIQUE_INDEX\([^,]*, *(\d+)/ ||
/^DECLARE_TOAST\([^,]*, *(\d+), *(\d+)/;
$oidcounts{$1}++;
$oidcounts{$2}++ if $2;
}
my $found = 0;
foreach my $oid (sort {$a = $b} keys %oidcounts)
{
next unless $oidcounts{$oid} 1;
$found = 1;
print $oid\n;
}
exit $found;
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On 7/9/13 10:40 AM, Andrew Dunstan wrote: This is actually a pretty trivial task. Here is a simple perl version: But that would make Perl a hard build requirement. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On 07/09/2013 01:55 PM, Peter Eisentraut wrote: On 7/9/13 10:40 AM, Andrew Dunstan wrote: This is actually a pretty trivial task. Here is a simple perl version: But that would make Perl a hard build requirement. Well, it already is unless you're not building from git AND you're not building with MSVC AND you're not running a buildfarm animal (and maybe a few other conditions too). In any case, we could make it conditional on $(PERL) not being empty, couldn't we? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
Peter Eisentraut pete...@gmx.net writes: On 7/9/13 10:40 AM, Andrew Dunstan wrote: This is actually a pretty trivial task. Here is a simple perl version: But that would make Perl a hard build requirement. My opinion is that this script should only run if you've changed some catalog .h files. Requiring Perl in those cases is no worse than what we already require it for (cf the keyword crosscheck script mentioned upthread). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Should we automatically run duplicate_oids?
When rebasing a patch that I'm working on, I occasionally forget to update the oid of any pg_proc.h entries I may have created. Of course this isn't a real problem; when I go to initdb, I immediately recognize what has happened. All the same, it seems like there is a case to be made for having this run automatically at build time, and having the build fail on the basis of there being a duplicate - this is something that fails reliably, but only when someone has added another pg_proc.h entry, and only when that other person happened to choose an oid in a range of free-in-git-tip oids that I myself fancied. Sure, I ought to remember to check this anyway, but it seems preferable to make this process more mechanical. I can point to commit 55c1687a as a kind of precedent, where the process of running check_keywords.pl was made to run automatically any time gram.c is rebuilt. Granted, that's a more subtle problem than the one I'm proposing to solve, but I still see this as a modest improvement. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
Peter Geoghegan p...@heroku.com writes: ... All the same, it seems like there is a case to be made for having this run automatically at build time, and having the build fail on the basis of there being a duplicate This would require a rather higher standard of portability than duplicate_oids has heretofore been held to. Sure, I ought to remember to check this anyway, but it seems preferable to make this process more mechanical. I can point to commit 55c1687a as a kind of precedent, where the process of running check_keywords.pl was made to run automatically any time gram.c is rebuilt. Note that any time gram.c is rebuilt is not every build. In fact, for non-developers (tarball consumers), it's not *any* build. I'm not necessarily opposed to making this happen, but there's more legwork involved than just adding a call of the existing script in some random place in the makefiles. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On Mon, Jul 8, 2013 at 6:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: This would require a rather higher standard of portability than duplicate_oids has heretofore been held to. Ah, yes. I suppose that making this happen would necessitate rewriting the script in highly portable Perl. Unfortunately, I'm not a likely candidate for that task. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On Mon, 2013-07-08 at 19:38 -0700, Peter Geoghegan wrote: On Mon, Jul 8, 2013 at 6:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: This would require a rather higher standard of portability than duplicate_oids has heretofore been held to. Ah, yes. I suppose that making this happen would necessitate rewriting the script in highly portable Perl. Unfortunately, I'm not a likely candidate for that task. I don't think rewriting it in Perl is necessary or even desirable. I don't see anything particularly unportable in that script as it is. (Hmm, perhaps egrep should be replaced by grep.) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On Mon, Jul 8, 2013 at 7:59 PM, Peter Eisentraut pete...@gmx.net wrote: I don't think rewriting it in Perl is necessary or even desirable. I don't see anything particularly unportable in that script as it is. I was under the impression that the final patch ought to work on Windows too. However, I suppose that since the number of people that use windows as an everyday development machine is probably zero, we could reasonably forgo doing anything on that platform. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On 07/09/2013 11:03 AM, Peter Geoghegan wrote: On Mon, Jul 8, 2013 at 7:59 PM, Peter Eisentraut pete...@gmx.net wrote: I don't think rewriting it in Perl is necessary or even desirable. I don't see anything particularly unportable in that script as it is. I was under the impression that the final patch ought to work on Windows too. However, I suppose that since the number of people that use windows as an everyday development machine is probably zero, we could reasonably forgo doing anything on that platform. I'd say that the number who use Windows *and the unix-like build chain* day to day is going to be zero. If you're using the Visual Studio based toolchain with vcbuild.pl, vcregress.pl, etc you're not going to notice or care about this change. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Should we automatically run duplicate_oids?
On Tue, Jul 9, 2013 at 6:55 AM, Peter Geoghegan p...@heroku.com wrote: When rebasing a patch that I'm working on, I occasionally forget to update the oid of any pg_proc.h entries I may have created. Of course this isn't a real problem; when I go to initdb, I immediately recognize what has happened. All the same, it seems like there is a case to be made for having this run automatically at build time, and having the build fail on the basis of there being a duplicate - this is something that fails reliably, but only when someone has added another pg_proc.h entry, and only when that other person happened to choose an oid in a range of free-in-git-tip oids that I myself fancied. Sure, I ought to remember to check this anyway, but it seems preferable to make this process more mechanical. I can point to commit 55c1687a as a kind of precedent, where the process of running check_keywords.pl was made to run automatically any time gram.c is rebuilt. Granted, that's a more subtle problem than the one I'm proposing to solve, but I still see this as a modest improvement. I completely agree with the idea. Doing these checks early in the build chain would be much more helpful than seeing the logs when initdb fails. Regards, Atri -- Regards, Atri l'apprenant -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
