public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 5/8] Add zlib compression method
9+ messages / 5 participants
[nested] [flat]

* [PATCH 5/8] Add zlib compression method
@ 2018-06-18 12:51  Ildus Kurbangaliev <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Ildus Kurbangaliev @ 2018-06-18 12:51 UTC (permalink / raw)

Signed-off-by: Ildus Kurbangaliev <[email protected]>
---
 src/backend/Makefile                        |   2 +-
 src/backend/access/compression/Makefile     |   2 +-
 src/backend/access/compression/cm_zlib.c    | 252 ++++++++++++++++++++
 src/include/catalog/pg_am.dat               |   3 +
 src/include/catalog/pg_attr_compression.dat |   1 +
 src/include/catalog/pg_proc.dat             |   4 +
 6 files changed, 262 insertions(+), 2 deletions(-)
 create mode 100644 src/backend/access/compression/cm_zlib.c

diff --git a/src/backend/Makefile b/src/backend/Makefile
index 478a96db9b..bd5009e190 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -45,7 +45,7 @@ OBJS = $(SUBDIROBJS) $(LOCALOBJS) $(top_builddir)/src/port/libpgport_srv.a \
 LIBS := $(filter-out -lpgport -lpgcommon, $(LIBS)) $(LDAP_LIBS_BE) $(ICU_LIBS)
 
 # The backend doesn't need everything that's in LIBS, however
-LIBS := $(filter-out -lz -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
+LIBS := $(filter-out -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
 
 ifeq ($(with_systemd),yes)
 LIBS += -lsystemd
diff --git a/src/backend/access/compression/Makefile b/src/backend/access/compression/Makefile
index 14286920d3..7ea5ee2e43 100644
--- a/src/backend/access/compression/Makefile
+++ b/src/backend/access/compression/Makefile
@@ -12,6 +12,6 @@ subdir = src/backend/access/compression
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = cm_pglz.o cmapi.o
+OBJS = cm_pglz.o cm_zlib.o cmapi.o
 
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/compression/cm_zlib.c b/src/backend/access/compression/cm_zlib.c
new file mode 100644
index 0000000000..0dcb56ddf3
--- /dev/null
+++ b/src/backend/access/compression/cm_zlib.c
@@ -0,0 +1,252 @@
+/*-------------------------------------------------------------------------
+ *
+ * cm_zlib.c
+ *	  zlib compression method
+ *
+ * Copyright (c) 2015-2018, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/access/compression/cm_zlib.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+#include "access/cmapi.h"
+#include "commands/defrem.h"
+#include "nodes/parsenodes.h"
+#include "utils/builtins.h"
+
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+
+#define ZLIB_MAX_DICTIONARY_LENGTH		32768
+#define ZLIB_DICTIONARY_DELIM			(" ,")
+
+typedef struct
+{
+	int				level;
+	Bytef			dict[ZLIB_MAX_DICTIONARY_LENGTH];
+	unsigned int	dictlen;
+} zlib_state;
+
+/*
+ * Check options if specified. All validation is located here so
+ * we don't need do it again in cminitstate function.
+ */
+static void
+zlib_cmcheck(Form_pg_attribute att, List *options)
+{
+	ListCell	*lc;
+	foreach(lc, options)
+	{
+		DefElem    *def = (DefElem *) lfirst(lc);
+
+		if (strcmp(def->defname, "level") == 0)
+		{
+			if (strcmp(defGetString(def), "best_speed") != 0 &&
+				strcmp(defGetString(def), "best_compression") != 0 &&
+				strcmp(defGetString(def), "default") != 0)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("unexpected value for zlib compression level: \"%s\"", defGetString(def))));
+		}
+		else if (strcmp(def->defname, "dict") == 0)
+		{
+			int		ntokens = 0;
+			char   *val,
+				   *tok;
+
+			val = pstrdup(defGetString(def));
+			if (strlen(val) > (ZLIB_MAX_DICTIONARY_LENGTH - 1))
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						(errmsg("zlib dictionary length should be less than %d", ZLIB_MAX_DICTIONARY_LENGTH))));
+
+			while ((tok = strtok(val, ZLIB_DICTIONARY_DELIM)) != NULL)
+			{
+				ntokens++;
+				val = NULL;
+			}
+
+			if (ntokens < 2)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						(errmsg("zlib dictionary is too small"))));
+		}
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_PARAMETER),
+					 errmsg("unexpected parameter for zlib: \"%s\"", def->defname)));
+	}
+}
+
+static void *
+zlib_cminitstate(Oid acoid, List *options)
+{
+	zlib_state		*state = NULL;
+
+	state = palloc0(sizeof(zlib_state));
+	state->level = Z_DEFAULT_COMPRESSION;
+
+	if (list_length(options) > 0)
+	{
+		ListCell	*lc;
+
+		foreach(lc, options)
+		{
+			DefElem    *def = (DefElem *) lfirst(lc);
+
+			if (strcmp(def->defname, "level") == 0)
+			{
+				if (strcmp(defGetString(def), "best_speed") == 0)
+					state->level = Z_BEST_SPEED;
+				else if (strcmp(defGetString(def), "best_compression") == 0)
+					state->level = Z_BEST_COMPRESSION;
+			}
+			else if (strcmp(def->defname, "dict") == 0)
+			{
+				char   *val,
+					   *tok;
+
+				val = pstrdup(defGetString(def));
+
+				/* Fill the zlib dictionary */
+				while ((tok = strtok(val, ZLIB_DICTIONARY_DELIM)) != NULL)
+				{
+					int len = strlen(tok);
+					memcpy((void *) (state->dict + state->dictlen), tok, len);
+					state->dictlen += len + 1;
+					Assert(state->dictlen <= ZLIB_MAX_DICTIONARY_LENGTH);
+
+					/* add space as dictionary delimiter */
+					state->dict[state->dictlen - 1] = ' ';
+					val = NULL;
+				}
+			}
+		}
+	}
+
+	return state;
+}
+
+static struct varlena *
+zlib_cmcompress(CompressionAmOptions *cmoptions, const struct varlena *value)
+{
+	int32			valsize,
+					len;
+	struct varlena *tmp = NULL;
+	z_streamp		zp;
+	int				res;
+	zlib_state	   *state = (zlib_state *) cmoptions->acstate;
+
+	zp = (z_streamp) palloc(sizeof(z_stream));
+	zp->zalloc = Z_NULL;
+	zp->zfree = Z_NULL;
+	zp->opaque = Z_NULL;
+
+	if (deflateInit(zp, state->level) != Z_OK)
+		elog(ERROR, "could not initialize compression library: %s", zp->msg);
+
+	if (state->dictlen > 0)
+	{
+		res = deflateSetDictionary(zp, state->dict, state->dictlen);
+		if (res != Z_OK)
+			elog(ERROR, "could not set dictionary for zlib: %s", zp->msg);
+	}
+
+	valsize = VARSIZE_ANY_EXHDR(DatumGetPointer(value));
+	tmp = (struct varlena *) palloc(valsize + VARHDRSZ_CUSTOM_COMPRESSED);
+	zp->next_in = (void *) VARDATA_ANY(value);
+	zp->avail_in = valsize;
+	zp->avail_out = valsize;
+	zp->next_out = (void *)((char *) tmp + VARHDRSZ_CUSTOM_COMPRESSED);
+
+	do {
+		res = deflate(zp, Z_FINISH);
+		if (res == Z_STREAM_ERROR)
+			elog(ERROR, "could not compress data: %s", zp->msg);
+	} while (zp->avail_in != 0);
+
+	Assert(res == Z_STREAM_END);
+
+	len = valsize - zp->avail_out;
+	if (deflateEnd(zp) != Z_OK)
+		elog(ERROR, "could not close compression stream: %s", zp->msg);
+	pfree(zp);
+
+	if (len > 0)
+	{
+		SET_VARSIZE_COMPRESSED(tmp, len + VARHDRSZ_CUSTOM_COMPRESSED);
+		return tmp;
+	}
+
+	pfree(tmp);
+#endif
+	return NULL;
+}
+
+static struct varlena *
+zlib_cmdecompress(CompressionAmOptions *cmoptions, const struct varlena *value)
+{
+	struct varlena *result;
+	z_streamp		zp;
+	int				res = Z_OK;
+	zlib_state	   *state = (zlib_state *) cmoptions->acstate;
+
+	zp = (z_streamp) palloc(sizeof(z_stream));
+	zp->zalloc = Z_NULL;
+	zp->zfree = Z_NULL;
+	zp->opaque = Z_NULL;
+
+	if (inflateInit(zp) != Z_OK)
+		elog(ERROR, "could not initialize compression library: %s", zp->msg);
+
+	Assert(VARATT_IS_CUSTOM_COMPRESSED(value));
+	zp->next_in = (void *) ((char *) value + VARHDRSZ_CUSTOM_COMPRESSED);
+	zp->avail_in = VARSIZE(value) - VARHDRSZ_CUSTOM_COMPRESSED;
+	zp->avail_out = VARRAWSIZE_4B_C(value);
+
+	result = (struct varlena *) palloc(zp->avail_out + VARHDRSZ);
+	SET_VARSIZE(result, zp->avail_out + VARHDRSZ);
+	zp->next_out = (void *) VARDATA(result);
+
+	while (zp->avail_in > 0)
+	{
+		res = inflate(zp, 0);
+		if (res == Z_NEED_DICT && state->dictlen > 0)
+		{
+			res = inflateSetDictionary(zp, state->dict, state->dictlen);
+			if (res != Z_OK)
+				elog(ERROR, "could not set dictionary for zlib");
+			continue;
+		}
+		if (!(res == Z_OK || res == Z_STREAM_END))
+			elog(ERROR, "could not uncompress data: %s", zp->msg);
+	}
+
+	if (inflateEnd(zp) != Z_OK)
+		elog(ERROR, "could not close compression library: %s", zp->msg);
+
+	pfree(zp);
+	return result;
+}
+
+Datum
+zlibhandler(PG_FUNCTION_ARGS)
+{
+#ifndef HAVE_LIBZ
+	ereport(ERROR,
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+			 errmsg("not built with zlib support")));
+#else
+	CompressionAmRoutine *routine = makeNode(CompressionAmRoutine);
+
+	routine->cmcheck = zlib_cmcheck;
+	routine->cminitstate = zlib_cminitstate;
+	routine->cmcompress = zlib_cmcompress;
+	routine->cmdecompress = zlib_cmdecompress;
+
+	PG_RETURN_POINTER(routine);
+#endif
+}
diff --git a/src/include/catalog/pg_am.dat b/src/include/catalog/pg_am.dat
index adfc10c443..8c101ed157 100644
--- a/src/include/catalog/pg_am.dat
+++ b/src/include/catalog/pg_am.dat
@@ -33,5 +33,8 @@
 { oid => '4002', oid_symbol => 'PGLZ_COMPRESSION_AM_OID',
   descr => 'pglz compression access method',
   amname => 'pglz', amhandler => 'pglzhandler', amtype => 'c' },
+{ oid => '4011', oid_symbol => 'ZLIB_COMPRESSION_AM_OID',
+  descr => 'zlib compression access method',
+  amname => 'zlib', amhandler => 'zlibhandler', amtype => 'c' },
 
 ]
diff --git a/src/include/catalog/pg_attr_compression.dat b/src/include/catalog/pg_attr_compression.dat
index 4e72bde16c..a7216fe963 100644
--- a/src/include/catalog/pg_attr_compression.dat
+++ b/src/include/catalog/pg_attr_compression.dat
@@ -19,5 +19,6 @@
 [
 
 { acoid => '4002', acname => 'pglz' },
+{ acoid => '4011', acname => 'zlib' },
 
 ]
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 5b57d46afe..e3dbdb623b 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -907,6 +907,10 @@
   proname => 'pglzhandler', provolatile => 'v',
   prorettype => 'compression_am_handler', proargtypes => 'internal',
   prosrc => 'pglzhandler' },
+{ oid => '4010', descr => 'zlib compression access method handler',
+  proname => 'zlibhandler', provolatile => 'v',
+  prorettype => 'compression_am_handler', proargtypes => 'internal',
+  prosrc => 'zlibhandler' },
 
 { oid => '338', descr => 'validate an operator class',
   proname => 'amvalidate', provolatile => 'v', prorettype => 'bool',
-- 
2.20.1


--MP_/IoARF=uydinlHTRfc3S.uiA
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename=0006-Add-psql-pg_dump-and-pg_upgrade-support-v21.patch



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

* Re: drop postmaster symlink
@ 2023-01-07 22:59  Karl O. Pinc <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Karl O. Pinc @ 2023-01-07 22:59 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Devrim Gündüz <[email protected]>; Joe Conway <[email protected]>; pgsql-hackers

Hello,

This is a review of Peter's 2 patches.  I see only 1 small problem.

                           +++

Looking at the documentation, a "postmaster" in the glossary is
defined as the controlling process.  This works; it needs to be called
something.  There is still a postmaster.pid (etc.) in the data
directory.

The word "postmaster" (case insensitive) shows up 84 times in the
documentation.  I looked at all of these.  

I see a possible problem at line 1,412 of runtime.sgml, the "Linux
Memory Overcommit" section.  It talks about the postmaster's startup
script invoking the postmaster.  It might, possibly, be better to say
that "postgres" is invoked, that being the name of the invoked program.
There's a similar usage at line 1,416. On the other hand, the existing
text makes it quite clear what is going on and there's a lot to be said
for consistently using the word "postmaster".  I mention this only in
case somebody deems it significant.  Perhaps there's a way to use
markup, identifying "postgres" as a program with a name in the file
system, to make things more clear.  Most likely, nobody will care.

In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY
defined which references the deleted postmaster.sgml file.  Since I
did a maintainer-clean, and the patch deletes the postmaster.sgml
file, and I don't see any references to the entity in the docs, I
believe that this line should be removed.  (In other words, I don't
think this file is automatically maintained.)

After applying the patches the documentation seems to build just fine.

I have not run contrib/start-scripts/{freebsd,linux}, but the patch
looks fine and the result of applying the patch looks fine, and the
patch is a one-line simple replacement of "postmaster" with "postgres"
so I expect no problems.

The modification to src/port/path.c is in a comment, so it will surely
not affect anything at runtime.  And the changed comment looks right
to me.

There's thousands of lines of comments in the code where "postmaster"
(case insensitive) shows up after applying the patches.  I've not
reviewed any of these.  Or looked for odd variable names, or
references in the code to the postmaster binary - by name, etc.  I'm
not sure where it would make sense to look for such problems.

Aside from the "allfiles.sgml" problem, I see no reason why these 2
patches should not be applied.  As mentioned by others, I don't have
strong feelings about getting rid of the postmaster symlink.  But I
did pick this patch to review because I remember, once upon a time,
being slightly confused by a "postmaster" in the process list.

Regards,

Karl <[email protected]>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein






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

* Re: drop postmaster symlink
@ 2023-01-07 23:38  Tom Lane <[email protected]>
  parent: Karl O. Pinc <[email protected]>
  0 siblings, 2 replies; 9+ messages in thread

From: Tom Lane @ 2023-01-07 23:38 UTC (permalink / raw)
  To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Devrim Gündüz <[email protected]>; Joe Conway <[email protected]>; pgsql-hackers

"Karl O. Pinc" <[email protected]> writes:
> This is a review of Peter's 2 patches.  I see only 1 small problem.

> Looking at the documentation, a "postmaster" in the glossary is
> defined as the controlling process.  This works; it needs to be called
> something.  There is still a postmaster.pid (etc.) in the data
> directory.

> The word "postmaster" (case insensitive) shows up 84 times in the
> documentation.  I looked at all of these.  

Hmm ... I thought this patch was about getting rid of the
admittedly-obsolete installed symlink.  If it's trying to get rid of
the "postmaster" terminology for our parent process, I'm very strongly
against that, either as regards to code or documentation.

			regards, tom lane






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

* Re: drop postmaster symlink
@ 2023-01-08 00:33  Joe Conway <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 1 reply; 9+ messages in thread

From: Joe Conway @ 2023-01-08 00:33 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Devrim Gündüz <[email protected]>; pgsql-hackers

On 1/7/23 18:38, Tom Lane wrote:
> "Karl O. Pinc" <[email protected]> writes:
>> This is a review of Peter's 2 patches.  I see only 1 small problem.
> 
>> Looking at the documentation, a "postmaster" in the glossary is
>> defined as the controlling process.  This works; it needs to be called
>> something.  There is still a postmaster.pid (etc.) in the data
>> directory.
> 
>> The word "postmaster" (case insensitive) shows up 84 times in the
>> documentation.  I looked at all of these.  
> 
> Hmm ... I thought this patch was about getting rid of the
> admittedly-obsolete installed symlink.

That was my understanding too.

> If it's trying to get rid of the "postmaster" terminology for our
> parent process, I'm very strongly against that, either as regards to
> code or documentation.

+1

-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com







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

* Re: drop postmaster symlink
@ 2023-01-08 01:56  Karl O. Pinc <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 1 reply; 9+ messages in thread

From: Karl O. Pinc @ 2023-01-08 01:56 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Devrim Gündüz <[email protected]>; Joe Conway <[email protected]>; pgsql-hackers

On Sat, 07 Jan 2023 18:38:25 -0500
Tom Lane <[email protected]> wrote:

> "Karl O. Pinc" <[email protected]> writes:
> > This is a review of Peter's 2 patches.  I see only 1 small problem.
> >  
> 
> > Looking at the documentation, a "postmaster" in the glossary is
> > defined as the controlling process.  This works; it needs to be
> > called something.  There is still a postmaster.pid (etc.) in the
> > data directory.  
> 
> > The word "postmaster" (case insensitive) shows up 84 times in the
> > documentation.  I looked at all of these.    
> 
> Hmm ... I thought this patch was about getting rid of the
> admittedly-obsolete installed symlink.  If it's trying to get rid of
> the "postmaster" terminology for our parent process, I'm very strongly
> against that, either as regards to code or documentation.

No.  It's about getting rid of the symlink.  I was grepping around
to look for use of the symlink, started with the glossary just
to be sure, and saw that "postmaster" is consistently (I think)
used to refer to the controlling, parent, process.  And wrote
down what I was doing and what I found as I went along.  And then
sent out my notes.

Sorry for the confusion.

Regards,

Karl <[email protected]>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein






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

* Re: drop postmaster symlink
@ 2023-01-08 01:57  Karl O. Pinc <[email protected]>
  parent: Joe Conway <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Karl O. Pinc @ 2023-01-08 01:57 UTC (permalink / raw)
  To: Joe Conway <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Devrim Gündüz <[email protected]>; pgsql-hackers

On Sat, 7 Jan 2023 19:33:38 -0500
Joe Conway <[email protected]> wrote:

> On 1/7/23 18:38, Tom Lane wrote:
> > "Karl O. Pinc" <[email protected]> writes:  
> >> This is a review of Peter's 2 patches.  I see only 1 small
> >> problem.  

The small problem is a reference to a deleted file.

Regards,

Karl <[email protected]>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein






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

* Re: drop postmaster symlink
@ 2023-01-08 04:29  Karl O. Pinc <[email protected]>
  parent: Karl O. Pinc <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Karl O. Pinc @ 2023-01-08 04:29 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Devrim Gündüz <[email protected]>; Joe Conway <[email protected]>; pgsql-hackers

On Sat, 7 Jan 2023 19:56:08 -0600
"Karl O. Pinc" <[email protected]> wrote:

> On Sat, 07 Jan 2023 18:38:25 -0500
> Tom Lane <[email protected]> wrote:
> 
> > "Karl O. Pinc" <[email protected]> writes:  
> > > This is a review of Peter's 2 patches.  I see only 1 small
> > > problem. ...

> > Hmm ... I thought this patch was about getting rid of the
> > admittedly-obsolete installed symlink. ...

> No.  It's about getting rid of the symlink. 

The only way I could think of to review a patch
that removes something is to report all the places
I looked where a reference to the symlink might be.
Then report what I found each place I looked and
report if there's a problem, or might be.

That way the committer knows where I didn't look if there's
more that needs removing.

Apologies that this was not clear.

Regards,

Karl <[email protected]>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein






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

* Re: drop postmaster symlink
@ 2023-01-08 20:17  Karl O. Pinc <[email protected]>
  parent: Karl O. Pinc <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Karl O. Pinc @ 2023-01-08 20:17 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Tom Lane <[email protected]>; Daniel Gustafsson <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Devrim Gündüz <[email protected]>; Joe Conway <[email protected]>; pgsql-hackers

On Sat, 7 Jan 2023 22:29:35 -0600
"Karl O. Pinc" <[email protected]> wrote:

> The only way I could think of to review a patch
> that removes something is to report all the places
> I looked where a reference to the symlink might be.

I forgot to report that I also tried a `make install`
and 'make uninstall`, with no problems.

FWIW, I suspect the include/backend/postmaster/ directory
would today be named include/backend/postgres/.   Now
would be the time to change the name, but I don't see
it being worth the work.  And while such a change
wouldn't break pg, that kind of change would break something
for somebody.

Regards,

Karl <[email protected]>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein






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

* [PATCH v8 2/7] Row pattern recognition patch (parse/analysis).
@ 2023-09-25 05:01  Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Tatsuo Ishii @ 2023-09-25 05:01 UTC (permalink / raw)

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

diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 85cd47b7ae..aa7a1cee80 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -564,6 +564,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
@@ -953,6 +957,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 334b9b42bd..293d4b1680 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -100,7 +100,10 @@ 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 -
@@ -2950,6 +2953,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;
 
@@ -3815,3 +3822,287 @@ 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 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;
+	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)));
+	}
+
+	return transformTargetList(pstate, windef->rpCommonSyntax->rpDefs,
+							   EXPR_KIND_RPR_DEFINE);
+}
+
+/*
+ * transformPatternClause
+ *		Process PATTERN clause and return PATTERN clause in the raw parse tree
+ */
+static void
+transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+	ListCell	*lc, *l;
+
+	/*
+	 * Row Pattern Common Syntax clause exists?
+	 */
+	if (windef->rpCommonSyntax == NULL)
+		return;
+
+	/*
+	 * Primary row pattern variable names in PATTERN clause must appear in
+	 * DEFINE clause as row pattern definition variable names.
+	 */
+	wc->patternVariable = NIL;
+	wc->patternRegexp = NIL;
+	foreach(lc, windef->rpCommonSyntax->rpPatterns)
+	{
+		A_Expr	*a;
+		char	*name;
+		char	*regexp;
+		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 = (ResTarget *)lfirst(l);
+
+			if (!strcmp(restarget->name, name))
+			{
+				found = true;
+				break;
+			}
+		}
+
+		if (!found)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("primary row pattern variable name \"%s\" does not appear in DEFINE clause",
+							name),
+					 parser_errposition(pstate, exprLocation((Node *)a))));
+		}
+		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 64c582c344..18b58ac263 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -557,6 +557,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;
 
@@ -1770,6 +1771,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:
@@ -3149,6 +3151,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 b3f0b6a137..2ff3699538 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2656,6 +2656,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(Mon_Sep_25_14_26_30_2023_752)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v8-0003-Row-pattern-recognition-patch-planner.patch"



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


end of thread, other threads:[~2023-09-25 05:01 UTC | newest]

Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-06-18 12:51 [PATCH 5/8] Add zlib compression method Ildus Kurbangaliev <[email protected]>
2023-01-07 22:59 Re: drop postmaster symlink Karl O. Pinc <[email protected]>
2023-01-07 23:38 ` Re: drop postmaster symlink Tom Lane <[email protected]>
2023-01-08 00:33   ` Re: drop postmaster symlink Joe Conway <[email protected]>
2023-01-08 01:57     ` Re: drop postmaster symlink Karl O. Pinc <[email protected]>
2023-01-08 01:56   ` Re: drop postmaster symlink Karl O. Pinc <[email protected]>
2023-01-08 04:29     ` Re: drop postmaster symlink Karl O. Pinc <[email protected]>
2023-01-08 20:17       ` Re: drop postmaster symlink Karl O. Pinc <[email protected]>
2023-09-25 05:01 [PATCH v8 2/7] 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