Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oBn35-0004vo-0Q for pgsql-hackers@arkaria.postgresql.org; Thu, 14 Jul 2022 00:49:47 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1oBn33-0002ER-0j for pgsql-hackers@arkaria.postgresql.org; Thu, 14 Jul 2022 00:49:45 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oBn32-0002EI-OS for pgsql-hackers@lists.postgresql.org; Thu, 14 Jul 2022 00:49:44 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oBn30-0003Dc-9a for pgsql-hackers@postgresql.org; Thu, 14 Jul 2022 00:49:43 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 26E0nd1h2593370; Wed, 13 Jul 2022 20:49:39 -0400 From: Tom Lane To: Peter Eisentraut cc: David Rowley , pgsql-hackers Subject: Re: automatically generating node support functions In-reply-to: <20220711224130.tg6bcnslgyfijuiw@awork3.anarazel.de> References: <1116889.1657553859@sss.pgh.pa.us> <1201553.1657562258@sss.pgh.pa.us> <1229446.1657569262@sss.pgh.pa.us> <20220711201755.x5hmdailt63z2xo2@awork3.anarazel.de> <1239476.1657571885@sss.pgh.pa.us> <20220711213755.gwcysmuypke3wckg@awork3.anarazel.de> <1261926.1657577355@sss.pgh.pa.us> <20220711222758.3bdgprqvvjwndn7k@awork3.anarazel.de> <1282697.1657579184@sss.pgh.pa.us> <20220711224130.tg6bcnslgyfijuiw@awork3.anarazel.de> Comments: In-reply-to Andres Freund message dated "Mon, 11 Jul 2022 15:41:30 -0700" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <2593288.1657759752.0@sss.pgh.pa.us> Date: Wed, 13 Jul 2022 20:49:39 -0400 Message-ID: <2593369.1657759779@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2593288.1657759752.1@sss.pgh.pa.us> Just one more thing here ... I really don't like the fact that gen_node_support.pl's response to unparseable input is to silently ignore it. That's maybe tolerable outside a node struct, but I think we need a higher standard inside. I experimented with promoting the commented-out "warn" to "die", and soon learned that there are two shortcomings: * We can't cope with the embedded union inside A_Const. Simplest fix is to move it outside. * We can't cope with function-pointer fields. The only real problem there is that some of them spread across multiple lines, but really that was a shortcoming we'd have to fix sometime anyway. Proposed patch attached. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="improve-gen_node_support-field-parsing.patch"; charset="us-ascii" Content-ID: <2593288.1657759752.2@sss.pgh.pa.us> Content-Description: improve-gen_node_support-field-parsing.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen= _node_support.pl index 96af17516a..35af4e231f 100644 --- a/src/backend/nodes/gen_node_support.pl +++ b/src/backend/nodes/gen_node_support.pl @@ -213,15 +213,34 @@ foreach my $infile (@ARGV) } $file_content .=3D $raw_file_content; = - my $lineno =3D 0; + my $lineno =3D 0; + my $prevline =3D ''; foreach my $line (split /\n/, $file_content) { + # per-physical-line processing $lineno++; chomp $line; $line =3D~ s/\s*$//; next if $line eq ''; next if $line =3D~ /^#(define|ifdef|endif)/; = + # within a node struct, don't process until we have whole logical line + if ($in_struct && $subline > 1) + { + if ($line =3D~ m/;$/) + { + # found the end, re-attach any previous line(s) + $line =3D $prevline . $line; + $prevline =3D ''; + } + else + { + # set it aside for a moment + $prevline .=3D $line . ' '; + next; + } + } + # we are analyzing a struct definition if ($in_struct) { @@ -394,7 +413,7 @@ foreach my $infile (@ARGV) } # normal struct field elsif ($line =3D~ - /^\s*(.+)\s*\b(\w+)(\[\w+\])?\s*(?:pg_node_attr\(([\w(), ]*)\))?;/ + /^\s*(.+)\s*\b(\w+)(\[[\w\s+]+\])?\s*(?:pg_node_attr\(([\w(), ]*)\))?= ;/ ) { if ($is_node_struct) @@ -441,13 +460,46 @@ foreach my $infile (@ARGV) $my_field_attrs{$name} =3D \@attrs; } } - else + # function pointer field + elsif ($line =3D~ + /^\s*([\w\s*]+)\s*\(\*(\w+)\)\s*\((.*)\)\s*(?:pg_node_attr\(([\w(), ]= *)\))?;/ + ) { if ($is_node_struct) { - #warn "$infile:$lineno: could not parse \"$line\"\n"; + my $type =3D $1; + my $name =3D $2; + my $args =3D $3; + my $attrs =3D $4; + + my @attrs; + if ($attrs) + { + @attrs =3D split /,\s*/, $attrs; + foreach my $attr (@attrs) + { + if ( $attr !~ /^copy_as\(\w+\)$/ + && $attr !~ /^read_as\(\w+\)$/ + && !elem $attr, + qw(equal_ignore read_write_ignore)) + { + die + "$infile:$lineno: unrecognized attribute \"$attr\"\n"; + } + } + } + + push @my_fields, $name; + $my_field_types{$name} =3D 'function pointer'; + $my_field_attrs{$name} =3D \@attrs; } } + else + { + # We're not too picky about what's outside structs, + # but we'd better understand everything inside. + die "$infile:$lineno: could not parse \"$line\"\n"; + } } # not in a struct else @@ -709,6 +761,12 @@ _equal${n}(const $n *a, const $n *b) unless $equal_ignore; } } + elsif ($t eq 'function pointer') + { + # we can copy and compare as a scalar + print $cff "\tCOPY_SCALAR_FIELD($f);\n" unless $copy_ignore; + print $eff "\tCOMPARE_SCALAR_FIELD($f);\n" unless $equal_ignore; + } # node type elsif ($t =3D~ /(\w+)\*/ and elem $1, @node_types) { @@ -980,6 +1038,12 @@ _read${n}(void) unless $no_read; } } + elsif ($t eq 'function pointer') + { + # We don't print these, and we can't read them either + die "cannot read function pointer in struct \"$n\" field \"$f\"\n" + unless $no_read; + } # Special treatments of several Path node fields elsif ($t eq 'RelOptInfo*' && elem 'write_only_relids', @a) { diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes= .h index b0c9c5f2ef..98fe1abaa2 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -303,26 +303,26 @@ typedef struct A_Expr = /* * A_Const - a literal constant + * + * Value nodes are inline for performance. You can treat 'val' as a node= , + * as in IsA(&val, Integer). 'val' is not valid if isnull is true. */ +union ValUnion +{ + Node node; + Integer ival; + Float fval; + Boolean boolval; + String sval; + BitString bsval; +}; + typedef struct A_Const { pg_node_attr(custom_copy_equal, custom_read_write, no_read) = NodeTag type; - - /* - * Value nodes are inline for performance. You can treat 'val' as a nod= e, - * as in IsA(&val, Integer). 'val' is not valid if isnull is true. - */ - union ValUnion - { - Node node; - Integer ival; - Float fval; - Boolean boolval; - String sval; - BitString bsval; - } val; + union ValUnion val; bool isnull; /* SQL NULL constant */ int location; /* token location, or -1 if unknown */ } A_Const; ------- =_aaaaaaaaaa0--