From: | "David E(dot) Wheeler" <david(at)justatheory(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | John Naylor <johncnaylorls(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se> |
Subject: | Re: Generate GUC tables from .dat file |
Date: | 2025-08-28 18:03:32 |
Message-ID: | A02B079A-DBD1-4814-A4F3-C5CF8913D82C@justatheory.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Aug 28, 2025, at 09:29, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> A tiny nitpick is that all the other generator scripts (that I looked at) in
> the tree use GetOptions() with named parameter rather than dereference ARGV
> directly:
>
> +my $input_fname = $ARGV[0];
> +my $output_fname = $ARGV[1];
GetOptions() is overkill when there are no options. I’d opt for:
die "Usage: $0 INPUT_FILE OUTPUT_FILE\n" unless @ARGV == 2;
my ($input_fname, $output_fname) = @ARGV;
And since I griped about Perl style previously, I made a pass over modernizing it a bit. One might argue it’s less clear, of course; there is less alignment of the printing than in the original. Otherwise, I’d note:
* Use the /r regex return sequence to simplify dquote() (requires Perl 5.14, IIRC)
* Iterate over the data types in a single line
* Pass lists of values to `print`
* Use {$fh} syntax to make file handle arguments clearer
But do with it what you will.
One other thing, as an aside, and probably not worth changing this patch: I’d prefer to see the use of explicit I/O layers. IOW, rather than
open my $ofh, '>', $output_fname
Use the UTF-8 layer, which encodes strings as UTF-8 bytes:
open my $ofh, '>:utf8', $output_fname
Or perhaps use pure binary:
open my $ofh, '>:raw', $output_fname
Though then things like `ucfirst` won’t work properly for non-ASCII strings.
The default layer, when not specified, is Latin-1 (because 1994). It’s not a problem if we’re certain we’ll never use anything other than ASCII, but more explicit I/O layers would be clearer, IMO. I didn’t change it in the attached because Catalog.pm doesn’t use an I/O layer, either, so it’s best if they’re consistent.
So, I guess, would there be interest in a patch to update I/O layer handling in the core Perl code?
Best,
David
Attachment | Content-Type | Size |
---|---|---|
gen_guc_tables.pl | text/x-perl-script | 2.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2025-08-28 19:52:12 | Re: index prefetching |
Previous Message | Tom Lane | 2025-08-28 17:26:46 | Re: [BUG] Remove self joins causes 'variable not found in subplan target lists' error |