public inbox for [email protected]  
help / color / mirror / Atom feed
Re: [PATCH] Add inline comments to the pg_hba_file_rules view
7+ messages / 3 participants
[nested] [flat]

* Re: [PATCH] Add inline comments to the pg_hba_file_rules view
@ 2023-09-10 22:33  Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2023-09-10 22:33 UTC (permalink / raw)
  To: Jim Jones <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Mon, Sep 04, 2023 at 12:54:15PM +0200, Jim Jones wrote:
> The patch slightly changes the test 004_file_inclusion.pl to accommodate the
> new column and the hba comments.
> 
> Discussion: https://www.postgresql.org/message-id/flat/3fec6550-93b0-b542-b203-b0054aaee83b%40uni-muenster.de

Well, it looks like what I wrote a couple of days ago was perhaps
confusing:
https://www.postgresql.org/message-id/ZPHAiNp%2ByKMsa/vc%40paquier.xyz
https://www.postgresql.org/message-id/[email protected]

This patch touches hbafuncs.c and the system view pg_hba_file_rules,
but I don't think this stuff should touch any of these code paths.
That's what I meant in my second message: the SQL portion should be
usable for all types of configuration files, even pg_ident.conf and
postgresql.conf, and not only pg_hba.conf.  A new SQL function
returning a SRF made of the comments extracted and the line numbers 
can be joined with all the system views of the configuration files,
like sourcefile and sourceline in pg_settings, etc.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [PATCH] Add inline comments to the pg_hba_file_rules view
@ 2023-09-14 11:33  Jim Jones <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Jim Jones @ 2023-09-14 11:33 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Zhang <[email protected]>

Hi

On 11.09.23 00:33, Michael Paquier wrote:
> Well, it looks like what I wrote a couple of days ago was perhaps
> confusing:
> https://www.postgresql.org/message-id/ZPHAiNp%2ByKMsa/vc%40paquier.xyz
> https://www.postgresql.org/message-id/[email protected]
>
> This patch touches hbafuncs.c and the system view pg_hba_file_rules,
> but I don't think this stuff should touch any of these code paths.
> That's what I meant in my second message: the SQL portion should be
> usable for all types of configuration files, even pg_ident.conf and
> postgresql.conf, and not only pg_hba.conf.  A new SQL function
> returning a SRF made of the comments extracted and the line numbers
> can be joined with all the system views of the configuration files,
> like sourcefile and sourceline in pg_settings, etc.
> --
> Michael

Thanks for the feedback.

I indeed misunderstood what you meant in the other thread, as you 
explicitly only mentioned hba.c.

The change to hbafunc.c was mostly a function call and a new column to 
the view:


comment = GetInlineComment(hba->rawline);
if(comment)
    values[index++] = CStringGetTextDatum(comment);
else
    nulls[index++] = true;


Just to make sure I got what you have in mind: you suggest to read the 
pg_hba.conf a second time via a new (generic) function like 
pg_read_file() that returns line numbers and their contents (+comments), 
and the results of this new function would be joined pg_hba_file_rules 
in SQL. Is that correct?

Thanks







^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [PATCH] Add inline comments to the pg_hba_file_rules view
@ 2023-09-14 23:28  Michael Paquier <[email protected]>
  parent: Jim Jones <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2023-09-14 23:28 UTC (permalink / raw)
  To: Jim Jones <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Zhang <[email protected]>

On Thu, Sep 14, 2023 at 01:33:04PM +0200, Jim Jones wrote:
> Just to make sure I got what you have in mind: you suggest to read the
> pg_hba.conf a second time via a new (generic) function like pg_read_file()
> that returns line numbers and their contents (+comments), and the results of
> this new function would be joined pg_hba_file_rules in SQL. Is that correct?

Yes, my suggestion was to define a new set-returning function that
takes in input a file path and that returns as one row one comment and
its line number from the configuration file.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [PATCH] Add inline comments to the pg_hba_file_rules view
@ 2023-09-15 07:37  Jim Jones <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Jim Jones @ 2023-09-15 07:37 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Zhang <[email protected]>

On 15.09.23 01:28, Michael Paquier wrote:
> Yes, my suggestion was to define a new set-returning function that
> takes in input a file path and that returns as one row one comment and
> its line number from the configuration file.
> --
> Michael

Thanks!

If reading the file again is an option, perhaps a simple SQL function 
would suffice?

Something along these lines ..

CREATE OR REPLACE FUNCTION pg_read_conf_comments(text)
RETURNS TABLE (line_number int, comment text) AS $$
   SELECT lnum,
     trim(substring(line,
          nullif(strpos(line,'#'),0)+1,
          length(line)-strpos(line,'#')
     )) AS comment
   FROM unnest(string_to_array(pg_read_file($1),E'\n'))
        WITH ORDINALITY hba(line,lnum)
   WHERE trim(line) !~~ '#%' AND trim(line) <> '';
$$
STRICT LANGUAGE SQL ;


.. then we could join it with pg_hba_file_rules (or any other conf file)


SELECT type, database, user_name, address, c.comment
FROM  pg_hba_file_rules h, pg_read_conf_comments(h.file_name) c
WHERE user_name[1]='jim' AND h.line_number = c.line_number ;

  type | database | user_name |  address  | comment
------+----------+-----------+-----------+---------
  host | {db}     | {jim}     | 127.0.0.1 | foo
  host | {db}     | {jim}     | 127.0.0.1 | bar
  host | {db}     | {jim}     | 127.0.0.1 | #foo#
(3 rows)


Is it more or less what you had in mind?






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [PATCH] Add inline comments to the pg_hba_file_rules view
@ 2023-09-16 04:18  Michael Paquier <[email protected]>
  parent: Jim Jones <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2023-09-16 04:18 UTC (permalink / raw)
  To: Jim Jones <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Zhang <[email protected]>

On Fri, Sep 15, 2023 at 09:37:23AM +0200, Jim Jones wrote:
> SELECT type, database, user_name, address, c.comment
> FROM  pg_hba_file_rules h, pg_read_conf_comments(h.file_name) c
> WHERE user_name[1]='jim' AND h.line_number = c.line_number ;
> 
>  type | database | user_name |  address  | comment
> ------+----------+-----------+-----------+---------
>  host | {db}     | {jim}     | 127.0.0.1 | foo
>  host | {db}     | {jim}     | 127.0.0.1 | bar
>  host | {db}     | {jim}     | 127.0.0.1 | #foo#
> (3 rows)
> 
> 
> Is it more or less what you had in mind?

That was the idea.  I forgot about strpos(), but if you do that, do we
actually need a function in core to achieve that?  There are a few
fancy cases with the SQL function you have sent, actually..  strpos()
would grep the first '#' character, ignoring quoted areas.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [PATCH] Add inline comments to the pg_hba_file_rules view
@ 2023-09-19 22:29  Jim Jones <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Jim Jones @ 2023-09-19 22:29 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Zhang <[email protected]>

Hi Michael

On 16.09.23 06:18, Michael Paquier wrote:
> That was the idea. I forgot about strpos(), but if you do that, do we
> actually need a function in core to achieve that? 
I guess it depends who you ask :) I personally think it would be a good 
addition to the view, as it would provide a more comprehensive look into 
the hba file. Yes, the fact that it could possibly be written in SQL 
sounds silly, but it's IMHO still relevant to have it by default.
> There are a few fancy cases with the SQL function you have sent, 
> actually.. strpos() would grep the first '#' character, ignoring 
> quoted areas.

Yes, you're totally right. I didn't take into account any token 
surrounded by double quotes containing #.

v3 attached addresses this issue.

 From the following hba:

  host db jim 192.168.10.1/32 md5 # foo
  host db jim 192.168.10.2/32 md5 #bar
  host db jim 192.168.10.3/32 md5 #     #foo#
  host "a#db" "a#user" 192.168.10.4/32 md5 # fancy #hba entry

We can get these records from the view:

  SELECT type, database, user_name, address, comment
  FROM pg_hba_file_rules
  WHERE address ~~ '192.168.10.%';

  type | database | user_name | address    |     comment
------+----------+-----------+--------------+------------------
  host | {db}     | {jim}     | 192.168.10.1 | foo
  host | {db}     | {jim}     | 192.168.10.2 | bar
  host | {db}     | {jim}     | 192.168.10.3 | #foo#
  host | {a#db}   | {a#user}  | 192.168.10.4 | fancy #hba entry


I am still struggling to find a way to enable this function in separated 
path without having to read the conf file multiple times, or writing too 
much redundant code. How many other conf files do you think would profit 
from this feature?

Jim


Attachments:

  [text/x-patch] v3-0001-Add-inline-comments-to-the-pg_hba_file_rules-view.patch (11.2K, ../../[email protected]/2-v3-0001-Add-inline-comments-to-the-pg_hba_file_rules-view.patch)
  download | inline diff:
From 47f55bab0a8e8af286e6be2f40d218f25a5066c9 Mon Sep 17 00:00:00 2001
From: Jim Jones <[email protected]>
Date: Wed, 20 Sep 2023 00:04:05 +0200
Subject: [PATCH v3] Add inline comments to the pg_hba_file_rules view

This patch proposes the column "comment" to the pg_hba_file_rules view.
It basically parses the inline comment (if any) of a valid pg_hba.conf
entry and displays it in the new column. The patch slightly changes the
test 004_file_inclusion.pl to accomodate the new column and the hba comments.
The view's documentation at system-views.sgml is extended accordingly.

The function GetInlineComment() was added to conffiles.c to avoid adding more
complexity directly to hba.c. It also enables this feature to be used by other
configuration files, if necessary.
---
 doc/src/sgml/system-views.sgml                |  9 +++
 src/backend/utils/adt/hbafuncs.c              | 11 ++-
 src/backend/utils/misc/conffiles.c            | 41 ++++++++++
 src/include/catalog/pg_proc.dat               |  6 +-
 src/include/libpq/hba.h                       |  1 +
 src/include/utils/conffiles.h                 |  1 +
 .../authentication/t/004_file_inclusion.pl    | 74 +++++++++++++++++--
 src/test/regress/expected/rules.out           |  3 +-
 8 files changed, 135 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 2b35c2f91b..68f9857de0 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -1090,6 +1090,15 @@
       </para></entry>
      </row>
 
+    <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>comment</structfield> <type>text</type>
+      </para>
+      <para>
+       Text after the first <literal>#</literal> comment character in the end of a valid <literal>pg_hba.conf</literal> entry, if any
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>error</structfield> <type>text</type>
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 73d3ad1dad..929678e97e 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -22,6 +22,7 @@
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/guc.h"
+#include "utils/conffiles.h"
 
 
 static ArrayType *get_hba_options(HbaLine *hba);
@@ -159,7 +160,7 @@ get_hba_options(HbaLine *hba)
 }
 
 /* Number of columns in pg_hba_file_rules view */
-#define NUM_PG_HBA_FILE_RULES_ATTS	 11
+#define NUM_PG_HBA_FILE_RULES_ATTS	 12
 
 /*
  * fill_hba_line
@@ -191,6 +192,7 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
 	const char *addrstr;
 	const char *maskstr;
 	ArrayType  *options;
+	char       *comment;
 
 	Assert(tupdesc->natts == NUM_PG_HBA_FILE_RULES_ATTS);
 
@@ -346,6 +348,13 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
 			values[index++] = PointerGetDatum(options);
 		else
 			nulls[index++] = true;
+
+		/* comments */
+		comment = GetInlineComment(hba->rawline);
+		if(comment)
+			values[index++] = CStringGetTextDatum(comment);
+		else
+			nulls[index++] = true;
 	}
 	else
 	{
diff --git a/src/backend/utils/misc/conffiles.c b/src/backend/utils/misc/conffiles.c
index 376a5c885b..feda49bf31 100644
--- a/src/backend/utils/misc/conffiles.c
+++ b/src/backend/utils/misc/conffiles.c
@@ -25,6 +25,47 @@
 #include "storage/fd.h"
 #include "utils/conffiles.h"
 
+/*
+ * GetInlineComment
+ *
+ * This function returns comments of a given config file line,
+ * if there is any. A comment is any text after the # character,
+ * as long as it is not located after a opeining '"'.
+ */
+char *
+GetInlineComment(char *line)
+{
+
+	size_t nq = 0;
+	char *comment;
+
+	for (int i = 0; i < strlen(line); i++) {
+
+		if (line[i] == '"')
+			nq++;
+		else if (line[i] == '#' && nq % 2 == 0)
+		{
+			size_t len = strlen(line);
+			comment = (char *) palloc(len * sizeof(char *));
+			memcpy(comment,&line[i+1],len-i);
+
+			/* trim leading and trailing whitespaces */
+			while (*comment && isspace((unsigned char) *comment))
+				comment++;
+			len = strlen(comment);
+			while (len > 0 && isspace((unsigned char) comment[len - 1]))
+				len--;
+			comment[len] = '\0';
+
+			return comment;
+		}
+
+	}
+
+	return NULL;
+
+ }
+
 /*
  * AbsoluteConfigLocation
  *
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9805bc6118..360f71e8ef 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6244,9 +6244,9 @@
 { oid => '3401', descr => 'show pg_hba.conf rules',
   proname => 'pg_hba_file_rules', prorows => '1000', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => '',
-  proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text}',
-  proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
-  proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,error}',
+  proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text,text}',
+  proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,comment,error}',
   prosrc => 'pg_hba_file_rules' },
 { oid => '6250', descr => 'show pg_ident.conf mappings',
   proname => 'pg_ident_file_mappings', prorows => '1000', proretset => 't',
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 189f6d0df2..5cb7224712 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -135,6 +135,7 @@ typedef struct HbaLine
 	char	   *radiusidentifiers_s;
 	List	   *radiusports;
 	char	   *radiusports_s;
+	char	   *comments;
 } HbaLine;
 
 typedef struct IdentLine
diff --git a/src/include/utils/conffiles.h b/src/include/utils/conffiles.h
index e3868fbc23..b60adae5fa 100644
--- a/src/include/utils/conffiles.h
+++ b/src/include/utils/conffiles.h
@@ -23,5 +23,6 @@ extern char **GetConfFilesInDir(const char *includedir,
 								const char *calling_file,
 								int elevel, int *num_filenames,
 								char **err_msg);
+extern char *GetInlineComment(char *line);
 
 #endif							/* CONFFILES_H */
diff --git a/src/test/authentication/t/004_file_inclusion.pl b/src/test/authentication/t/004_file_inclusion.pl
index 55d28ad586..199c824e33 100644
--- a/src/test/authentication/t/004_file_inclusion.pl
+++ b/src/test/authentication/t/004_file_inclusion.pl
@@ -63,8 +63,56 @@ sub add_hba_line
 	# Increment pg_hba_file_rules.rule_number and save it.
 	$globline = ++$line_counters{'hba_rule'};
 
+	# It is possible to have the character '#' in several elements of
+	# a hba entry if they're wrapped with double quotes. So we first check
+	# if a '#' comes after an opening '"', and if so we ignore it. If a '#'
+	# is located after a closing '"' or if there is no previous '"' in the
+	# string, we consider it as the beginning of an inline comment.
+
+	my $comment_position = -1;
+	my $nq = 0;
+
+	for my $i (0..length($entry)-1){
+
+		my $char = substr($entry, $i, 1);
+
+		if($char eq '"')
+		{
+			$nq = $nq + 1;
+		} elsif ($char eq '#' && $nq % 2 == 0)
+		{
+			$comment_position = $i;
+			last;
+		}
+
+	}
+
+	# In case the pg_hba entry has a comment, we store it in $comment
+	# and remove it from $entry. The content of $comment will be pushed
+	# into @tokens separataely. This avoids having whitespaces in $comment
+	# being interpreted as element separator by split(). The leading
+	# and trailing whitespaces in $comment are removed to reproduce the
+	# behaviour of GetInlineComment(char *line).
+
+	my $comment;
+
+	if($comment_position != -1)
+	{
+		$comment = substr($entry, $comment_position+1);
+		$comment =~ s/^\s+|\s+$//g;
+		$entry = substr($entry, 0, $comment_position);
+	}
+
 	# Generate the expected pg_hba_file_rules line
 	@tokens = split(/ /, $entry);
+
+	# Remove surrounding double quotes.
+	foreach (@tokens)
+	{
+		$_ =~ s/^"|"$//g;
+	}
+
+	# Wrapping database and user name with {}
 	$tokens[1] = '{' . $tokens[1] . '}';    # database
 	$tokens[2] = '{' . $tokens[2] . '}';    # user_name
 
@@ -72,6 +120,16 @@ sub add_hba_line
 	push @tokens, '';
 	push @tokens, '';
 
+	# Append comment, if any. Otherwise append empty string
+	if(not defined $comment)
+	{
+		push @tokens, '';
+	}
+	else
+	{
+		push @tokens, $comment;
+	}
+
 	# Final line expected, output of the SQL query.
 	$line = "";
 	$line .= "\n" if ($globline > 1);
@@ -167,15 +225,18 @@ mkdir("$data_dir/hba_inc_if");
 mkdir("$data_dir/hba_pos");
 
 # First, make sure that we will always be able to connect.
-$hba_expected .= add_hba_line($node, "$hba_file", 'local all all trust');
+$hba_expected .= add_hba_line($node, "$hba_file", 'local all all trust    #   # First, make sure that we will always be able to connect.   ');
+
+# Add database and user names containing # by wrapping them with double quotes.
+$hba_expected .= add_hba_line($node, "$hba_file", 'local "fancy#dbname" "fancy#username" trust# Add database and user names containing # by wrapping them with double quotes.');
 
 # "include".  Note that as $hba_file is located in $data_dir/subdir1,
 # pg_hba_pre.conf is located at the root of the data directory.
 $hba_expected .=
-  add_hba_line($node, "$hba_file", "include ../pg_hba_pre.conf");
+  add_hba_line($node, "$hba_file", "include ../pg_hba_pre.conf#foo");
 $hba_expected .=
   add_hba_line($node, 'pg_hba_pre.conf', "local pre all reject");
-$hba_expected .= add_hba_line($node, "$hba_file", "local all all reject");
+$hba_expected .= add_hba_line($node, "$hba_file", 'local all "all" reject #bar');
 add_hba_line($node, "$hba_file", "include ../hba_pos/pg_hba_pos.conf");
 $hba_expected .=
   add_hba_line($node, 'hba_pos/pg_hba_pos.conf', "local pos all reject");
@@ -184,7 +245,7 @@ $hba_expected .=
 $hba_expected .=
   add_hba_line($node, 'hba_pos/pg_hba_pos.conf', "include pg_hba_pos2.conf");
 $hba_expected .=
-  add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos2 all reject");
+  add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos2 all reject #bar!#");
 $hba_expected .=
   add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos3 all reject");
 
@@ -216,7 +277,7 @@ $hba_expected .= "\n"
   . $line_counters{'hba_rule'} . "|"
   . basename($hba_file) . "|"
   . $line_counters{$hba_file}
-  . '|local|{db1,db3}|{all}|reject||';
+  . '|local|{db1,db3}|{all}|reject|||';
 
 note "Generating ident structure with include directives";
 
@@ -279,7 +340,8 @@ my $contents = $node->safe_psql(
   user_name,
   auth_method,
   options,
-  error
+  error,
+  comment
  FROM pg_hba_file_rules ORDER BY rule_number;));
 is($contents, $hba_expected, 'check contents of pg_hba_file_rules');
 
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 5058be5411..6a6c6f9edd 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1348,8 +1348,9 @@ pg_hba_file_rules| SELECT rule_number,
     netmask,
     auth_method,
     options,
+    comment,
     error
-   FROM pg_hba_file_rules() a(rule_number, file_name, line_number, type, database, user_name, address, netmask, auth_method, options, error);
+   FROM pg_hba_file_rules() a(rule_number, file_name, line_number, type, database, user_name, address, netmask, auth_method, options, comment, error);
 pg_ident_file_mappings| SELECT map_number,
     file_name,
     line_number,
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v17 2/8] Row pattern recognition patch (parse/analysis).
@ 2024-04-28 11:00  Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Tatsuo Ishii @ 2024-04-28 11:00 UTC (permalink / raw)

---
 src/backend/parser/parse_agg.c    |   7 +
 src/backend/parser/parse_clause.c | 296 +++++++++++++++++++++++++++++-
 src/backend/parser/parse_expr.c   |   4 +
 src/backend/parser/parse_func.c   |   3 +
 4 files changed, 309 insertions(+), 1 deletion(-)

diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index bee7d8346a..9bc22a836a 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -577,6 +577,10 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
 			errkind = true;
 			break;
 
+		case EXPR_KIND_RPR_DEFINE:
+			errkind = true;
+			break;
+
 			/*
 			 * There is intentionally no default: case here, so that the
 			 * compiler will warn if we add a new ParseExprKind without
@@ -967,6 +971,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 		case EXPR_KIND_CYCLE_MARK:
 			errkind = true;
 			break;
+		case EXPR_KIND_RPR_DEFINE:
+			errkind = true;
+			break;
 
 			/*
 			 * There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 4fc5fc87e0..003a1e14ce 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -98,7 +98,14 @@ static WindowClause *findWindowClause(List *wclist, const char *name);
 static Node *transformFrameOffset(ParseState *pstate, int frameOptions,
 								  Oid rangeopfamily, Oid rangeopcintype, Oid *inRangeFunc,
 								  Node *clause);
-
+static void transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+						 List **targetlist);
+static List *transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+								   List **targetlist);
+static void transformPatternClause(ParseState *pstate, WindowClause *wc,
+								   WindowDef *windef);
+static List *transformMeasureClause(ParseState *pstate, WindowClause *wc,
+									WindowDef *windef);
 
 /*
  * transformFromClause -
@@ -2956,6 +2963,10 @@ transformWindowDefinitions(ParseState *pstate,
 											 rangeopfamily, rangeopcintype,
 											 &wc->endInRangeFunc,
 											 windef->endOffset);
+
+		/* Process Row Pattern Recognition related clauses */
+		transformRPR(pstate, wc, windef, targetlist);
+
 		wc->runCondition = NIL;
 		wc->winref = winref;
 
@@ -3821,3 +3832,286 @@ transformFrameOffset(ParseState *pstate, int frameOptions,
 
 	return node;
 }
+
+/*
+ * transformRPR
+ *		Process Row Pattern Recognition related clauses
+ */
+static void
+transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+			 List **targetlist)
+{
+	/*
+	 * Window definition exists?
+	 */
+	if (windef == NULL)
+		return;
+
+	/*
+	 * Row Pattern Common Syntax clause exists?
+	 */
+	if (windef->rpCommonSyntax == NULL)
+		return;
+
+	/* Check Frame option. Frame must start at current row */
+	if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("FRAME must start at current row when row patttern recognition is used")));
+
+	/* Transform AFTER MACH SKIP TO clause */
+	wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo;
+
+	/* Transform AFTER MACH SKIP TO variable */
+	wc->rpSkipVariable = windef->rpCommonSyntax->rpSkipVariable;
+
+	/* Transform SEEK or INITIAL clause */
+	wc->initial = windef->rpCommonSyntax->initial;
+
+	/* Transform DEFINE clause into list of TargetEntry's */
+	wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist);
+
+	/* Check PATTERN clause and copy to patternClause */
+	transformPatternClause(pstate, wc, windef);
+
+	/* Transform MEASURE clause */
+	transformMeasureClause(pstate, wc, windef);
+}
+
+/*
+ * transformDefineClause Process DEFINE clause and transform ResTarget into
+ *		list of TargetEntry.
+ *
+ * XXX we only support column reference in row pattern definition search
+ * condition, e.g. "price". <row pattern definition variable name>.<column
+ * reference> is not supported, e.g. "A.price".
+ */
+static List *
+transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+					  List **targetlist)
+{
+	/* DEFINE variable name initials */
+	static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz";
+
+	ListCell   *lc,
+			   *l;
+	ResTarget  *restarget,
+			   *r;
+	List	   *restargets;
+	List	   *defineClause;
+	char	   *name;
+	int			initialLen;
+	int			i;
+
+	/*
+	 * If Row Definition Common Syntax exists, DEFINE clause must exist. (the
+	 * raw parser should have already checked it.)
+	 */
+	Assert(windef->rpCommonSyntax->rpDefs != NULL);
+
+	/*
+	 * Check and add "A AS A IS TRUE" if pattern variable is missing in DEFINE
+	 * per the SQL standard.
+	 */
+	restargets = NIL;
+	foreach(lc, windef->rpCommonSyntax->rpPatterns)
+	{
+		A_Expr	   *a;
+		bool		found = false;
+
+		if (!IsA(lfirst(lc), A_Expr))
+			ereport(ERROR,
+					errmsg("node type is not A_Expr"));
+
+		a = (A_Expr *) lfirst(lc);
+		name = strVal(a->lexpr);
+
+		foreach(l, windef->rpCommonSyntax->rpDefs)
+		{
+			restarget = (ResTarget *) lfirst(l);
+
+			if (!strcmp(restarget->name, name))
+			{
+				found = true;
+				break;
+			}
+		}
+
+		if (!found)
+		{
+			/*
+			 * "name" is missing. So create "name AS name IS TRUE" ResTarget
+			 * node and add it to the temporary list.
+			 */
+			A_Const    *n;
+
+			restarget = makeNode(ResTarget);
+			n = makeNode(A_Const);
+			n->val.boolval.type = T_Boolean;
+			n->val.boolval.boolval = true;
+			n->location = -1;
+			restarget->name = pstrdup(name);
+			restarget->indirection = NIL;
+			restarget->val = (Node *) n;
+			restarget->location = -1;
+			restargets = lappend((List *) restargets, restarget);
+		}
+	}
+
+	if (list_length(restargets) >= 1)
+	{
+		/* add missing DEFINEs */
+		windef->rpCommonSyntax->rpDefs =
+			list_concat(windef->rpCommonSyntax->rpDefs, restargets);
+		list_free(restargets);
+	}
+
+	/*
+	 * Check for duplicate row pattern definition variables.  The standard
+	 * requires that no two row pattern definition variable names shall be
+	 * equivalent.
+	 */
+	restargets = NIL;
+	foreach(lc, windef->rpCommonSyntax->rpDefs)
+	{
+		restarget = (ResTarget *) lfirst(lc);
+		name = restarget->name;
+
+		/*
+		 * Add DEFINE expression (Restarget->val) to the targetlist as a
+		 * TargetEntry if it does not exist yet. Planner will add the column
+		 * ref var node to the outer plan's target list later on. This makes
+		 * DEFINE expression could access the outer tuple while evaluating
+		 * PATTERN.
+		 *
+		 * XXX: adding whole expressions of DEFINE to the plan.targetlist is
+		 * not so good, because it's not necessary to evalute the expression
+		 * in the target list while running the plan. We should extract the
+		 * var nodes only then add them to the plan.targetlist.
+		 */
+		findTargetlistEntrySQL99(pstate, (Node *) restarget->val,
+								 targetlist, EXPR_KIND_RPR_DEFINE);
+
+		/*
+		 * Make sure that the row pattern definition search condition is a
+		 * boolean expression.
+		 */
+		transformWhereClause(pstate, restarget->val,
+							 EXPR_KIND_RPR_DEFINE, "DEFINE");
+
+		foreach(l, restargets)
+		{
+			char	   *n;
+
+			r = (ResTarget *) lfirst(l);
+			n = r->name;
+
+			if (!strcmp(n, name))
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("row pattern definition variable name \"%s\" appears more than once in DEFINE clause",
+								name),
+						 parser_errposition(pstate, exprLocation((Node *) r))));
+		}
+		restargets = lappend(restargets, restarget);
+	}
+	list_free(restargets);
+
+	/*
+	 * Create list of row pattern DEFINE variable name's initial. We assign
+	 * [a-z] to them (up to 26 variable names are allowed).
+	 */
+	restargets = NIL;
+	i = 0;
+	initialLen = strlen(defineVariableInitials);
+
+	foreach(lc, windef->rpCommonSyntax->rpDefs)
+	{
+		char		initial[2];
+
+		restarget = (ResTarget *) lfirst(lc);
+		name = restarget->name;
+
+		if (i >= initialLen)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("number of row pattern definition variable names exceeds %d",
+							initialLen),
+					 parser_errposition(pstate,
+										exprLocation((Node *) restarget))));
+		}
+		initial[0] = defineVariableInitials[i++];
+		initial[1] = '\0';
+		wc->defineInitial = lappend(wc->defineInitial,
+									makeString(pstrdup(initial)));
+	}
+
+	defineClause = transformTargetList(pstate, windef->rpCommonSyntax->rpDefs,
+									   EXPR_KIND_RPR_DEFINE);
+
+	/* mark column origins */
+	markTargetListOrigins(pstate, defineClause);
+
+	/* mark all nodes in the DEFINE clause tree with collation information */
+	assign_expr_collations(pstate, (Node *) defineClause);
+
+	return defineClause;
+}
+
+/*
+ * transformPatternClause
+ *		Process PATTERN clause and return PATTERN clause in the raw parse tree
+ */
+static void
+transformPatternClause(ParseState *pstate, WindowClause *wc,
+					   WindowDef *windef)
+{
+	ListCell   *lc;
+
+	/*
+	 * Row Pattern Common Syntax clause exists?
+	 */
+	if (windef->rpCommonSyntax == NULL)
+		return;
+
+	wc->patternVariable = NIL;
+	wc->patternRegexp = NIL;
+	foreach(lc, windef->rpCommonSyntax->rpPatterns)
+	{
+		A_Expr	   *a;
+		char	   *name;
+		char	   *regexp;
+
+		if (!IsA(lfirst(lc), A_Expr))
+			ereport(ERROR,
+					errmsg("node type is not A_Expr"));
+
+		a = (A_Expr *) lfirst(lc);
+		name = strVal(a->lexpr);
+
+		wc->patternVariable = lappend(wc->patternVariable, makeString(pstrdup(name)));
+		regexp = strVal(lfirst(list_head(a->name)));
+
+		wc->patternRegexp = lappend(wc->patternRegexp, makeString(pstrdup(regexp)));
+	}
+}
+
+/*
+ * transformMeasureClause
+ *		Process MEASURE clause
+ *	XXX MEASURE clause is not supported yet
+ */
+static List *
+transformMeasureClause(ParseState *pstate, WindowClause *wc,
+					   WindowDef *windef)
+{
+	if (windef->rowPatternMeasures == NIL)
+		return NIL;
+
+	ereport(ERROR,
+			(errcode(ERRCODE_SYNTAX_ERROR),
+			 errmsg("%s", "MEASURE clause is not supported yet"),
+			 parser_errposition(pstate, exprLocation((Node *) windef->rowPatternMeasures))));
+	return NIL;
+}
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 1c1c86aa3e..5540a0fb0a 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -578,6 +578,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
 		case EXPR_KIND_COPY_WHERE:
 		case EXPR_KIND_GENERATED_COLUMN:
 		case EXPR_KIND_CYCLE_MARK:
+		case EXPR_KIND_RPR_DEFINE:
 			/* okay */
 			break;
 
@@ -1817,6 +1818,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
 		case EXPR_KIND_VALUES:
 		case EXPR_KIND_VALUES_SINGLE:
 		case EXPR_KIND_CYCLE_MARK:
+		case EXPR_KIND_RPR_DEFINE:
 			/* okay */
 			break;
 		case EXPR_KIND_CHECK_CONSTRAINT:
@@ -3197,6 +3199,8 @@ ParseExprKindName(ParseExprKind exprKind)
 			return "GENERATED AS";
 		case EXPR_KIND_CYCLE_MARK:
 			return "CYCLE";
+		case EXPR_KIND_RPR_DEFINE:
+			return "DEFINE";
 
 			/*
 			 * There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 0cbc950c95..ad982a7c17 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2657,6 +2657,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
 		case EXPR_KIND_CYCLE_MARK:
 			errkind = true;
 			break;
+		case EXPR_KIND_RPR_DEFINE:
+			errkind = true;
+			break;
 
 			/*
 			 * There is intentionally no default: case here, so that the
-- 
2.25.1


----Next_Part(Sun_Apr_28_20_28_26_2024_444)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v17-0003-Row-pattern-recognition-patch-rewriter.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2024-04-28 11:00 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-09-10 22:33 Re: [PATCH] Add inline comments to the pg_hba_file_rules view Michael Paquier <[email protected]>
2023-09-14 11:33 ` Jim Jones <[email protected]>
2023-09-14 23:28   ` Michael Paquier <[email protected]>
2023-09-15 07:37     ` Jim Jones <[email protected]>
2023-09-16 04:18       ` Michael Paquier <[email protected]>
2023-09-19 22:29         ` Jim Jones <[email protected]>
2024-04-28 11:00 [PATCH v17 2/8] Row pattern recognition patch (parse/analysis). Tatsuo Ishii <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox