public inbox for [email protected]
help / color / mirror / Atom feedRe: Security lessons from liblzma - libsystemd
9+ messages / 7 participants
[nested] [flat]
* Re: Security lessons from liblzma - libsystemd
@ 2024-04-03 21:19 Magnus Hagander <[email protected]>
2024-04-03 21:58 ` Re: Security lessons from liblzma - libsystemd Tom Lane <[email protected]>
2024-04-03 23:10 ` Re: Security lessons from liblzma - libsystemd Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 9+ messages in thread
From: Magnus Hagander @ 2024-04-03 21:19 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Bruce Momjian <[email protected]>; pgsql-hackers
On Wed, Apr 3, 2024 at 7:57 PM Andres Freund <[email protected]> wrote:
> Hi,
>
> As most will know by now, the way xz debacle was able to make sshd
> vulnerable
> was through a dependency from sshd to libsystemd and then from libsystemd
> to
> liblzma. One lesson from this is that unnecessary dependencies can still
> increase risk.
>
Yeah, I think that's something to consider for every dependency added. I
think we're fairly often protected against "adding too many libraries"
because many libraries simply don't exist for all the platforms we want to
build on. But it's nevertheless something to think about each time.
It's worth noting that we have an optional dependency on libsystemd as well.
>
> Openssh has now integrated [1] a patch to remove the dependency on
> libsystemd
> for triggering service manager readyness notifications, by inlining the
> necessary function. That's not hard, the protocol is pretty simple.
>
> I suspect we should do the same. We're not even close to being a target as
> attractive as openssh, but still, it seems unnecessary.
>
+1.
When the code is this simple, we should definitely consider carrying it
ourselves. At least if we don't expect to need *other* functionality from
the same library in the future, which I doubt we will from libsystemd.
An argument could be made to instead just remove support, but I think it's
> quite valuable to have intra service dependencies that can rely on the
> server
> actually having started up.
>
>
If we remove support we're basically just asking most of our linux
packagers to add it back in, and they will add it back in the same way we
did it. I think we do everybody a disservice if we do that. It's useful
functionality.
//Magnus
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
@ 2024-04-03 21:58 ` Tom Lane <[email protected]>
2024-04-03 22:16 ` Re: Security lessons from liblzma - libsystemd Andres Freund <[email protected]>
1 sibling, 1 reply; 9+ messages in thread
From: Tom Lane @ 2024-04-03 21:58 UTC (permalink / raw)
To: Magnus Hagander <[email protected]>; +Cc: Andres Freund <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Magnus Hagander <[email protected]> writes:
> On Wed, Apr 3, 2024 at 7:57 PM Andres Freund <[email protected]> wrote:
>> Openssh has now integrated [1] a patch to remove the dependency on
>> libsystemd
>> for triggering service manager readyness notifications, by inlining the
>> necessary function. That's not hard, the protocol is pretty simple.
>> I suspect we should do the same. We're not even close to being a target as
>> attractive as openssh, but still, it seems unnecessary.
> +1.
I didn't read the patch, but if it's short and stable enough then this
seems like a good idea. (If openssh and we are using such a patch,
that will probably be a big enough stake in the ground to prevent
somebody deciding to change the protocol ...)
>> An argument could be made to instead just remove support, but I think it's
>> quite valuable to have intra service dependencies that can rely on the
>> server actually having started up.
> If we remove support we're basically just asking most of our linux
> packagers to add it back in, and they will add it back in the same way we
> did it. I think we do everybody a disservice if we do that. It's useful
> functionality.
Yeah, that idea seems particularly silly in view of the desire
expressed earlier in this thread to reduce the number of patches
carried by packagers. People packaging for systemd-using distros
will not consider that this functionality is optional.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-03 21:58 ` Re: Security lessons from liblzma - libsystemd Tom Lane <[email protected]>
@ 2024-04-03 22:16 ` Andres Freund <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Andres Freund @ 2024-04-03 22:16 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Hi,
On 2024-04-03 17:58:55 -0400, Tom Lane wrote:
> Magnus Hagander <[email protected]> writes:
> > On Wed, Apr 3, 2024 at 7:57 PM Andres Freund <[email protected]> wrote:
> >> Openssh has now integrated [1] a patch to remove the dependency on
> >> libsystemd
> >> for triggering service manager readyness notifications, by inlining the
> >> necessary function. That's not hard, the protocol is pretty simple.
> >> I suspect we should do the same. We're not even close to being a target as
> >> attractive as openssh, but still, it seems unnecessary.
>
> > +1.
>
> I didn't read the patch, but if it's short and stable enough then this
> seems like a good idea.
It's basically just checking for an env var, opening the unix socket indicated
by that, writing a string to it and closing the socket again.
> (If openssh and we are using such a patch, that will probably be a big
> enough stake in the ground to prevent somebody deciding to change the
> protocol ...)
One version of the openssh patch to remove liblzma was submitted by one of the
core systemd devs, so I think they agree that it's a stable API. The current
protocol supports adding more information by adding attributes, so it should
be extensible enough anyway.
> >> An argument could be made to instead just remove support, but I think it's
> >> quite valuable to have intra service dependencies that can rely on the
> >> server actually having started up.
>
> > If we remove support we're basically just asking most of our linux
> > packagers to add it back in, and they will add it back in the same way we
> > did it. I think we do everybody a disservice if we do that. It's useful
> > functionality.
>
> Yeah, that idea seems particularly silly in view of the desire
> expressed earlier in this thread to reduce the number of patches
> carried by packagers. People packaging for systemd-using distros
> will not consider that this functionality is optional.
Yep.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
@ 2024-04-03 23:10 ` Peter Eisentraut <[email protected]>
2024-04-08 10:05 ` Re: Security lessons from liblzma - libsystemd Étienne BERSAC <[email protected]>
2024-04-12 14:46 ` Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-12 16:00 ` Re: Security lessons from liblzma - libsystemd Andres Freund <[email protected]>
1 sibling, 3 replies; 9+ messages in thread
From: Peter Eisentraut @ 2024-04-03 23:10 UTC (permalink / raw)
To: Magnus Hagander <[email protected]>; Andres Freund <[email protected]>; +Cc: Bruce Momjian <[email protected]>; pgsql-hackers
On 03.04.24 23:19, Magnus Hagander wrote:
> When the code is this simple, we should definitely consider carrying it
> ourselves. At least if we don't expect to need *other* functionality
> from the same library in the future, which I doubt we will from libsystemd.
Well, I've long had it on my list to do some integration to log directly
to the journal, so you can preserve metadata better. I'm not sure right
now whether this would use libsystemd, but it's not like there is
absolutely no other systemd-related functionality that could be added.
Personally, I think this proposed change is trying to close a barndoor
after a horse has bolted. There are many more interesting and scary
libraries in the dependency tree of "postgres", so just picking off one
right now doesn't really accomplish anything. The next release of
libsystemd will drop all the compression libraries as hard dependencies,
so the issue in that sense is gone anyway. Also, fun fact: liblzma is
also a dependency via libxml2.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-03 23:10 ` Re: Security lessons from liblzma - libsystemd Peter Eisentraut <[email protected]>
@ 2024-04-08 10:05 ` Étienne BERSAC <[email protected]>
2 siblings, 0 replies; 9+ messages in thread
From: Étienne BERSAC @ 2024-04-08 10:05 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; Magnus Hagander <[email protected]>; Andres Freund <[email protected]>; +Cc: Bruce Momjian <[email protected]>; pgsql-hackers
Hi,
> There are many more interesting and scary libraries in the dependency
> tree of "postgres", so just picking off one right now doesn't really
> accomplish anything. The next release of libsystemd will drop all
> the compression libraries as hard dependencies, so the issue in that
> sense is gone anyway. Also, fun fact: liblzma is also a dependency
> via libxml2.
Having an audit of all libraries linked to postgres and their level of
trust should help to point the next weak point. I'm pretty sure we have
several of these tiny libraries maintained by a lone out of time hacker
linked somewhere. What is the next xz ?
Regards,
Étienne
--
DALIBO
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-03 23:10 ` Re: Security lessons from liblzma - libsystemd Peter Eisentraut <[email protected]>
@ 2024-04-12 14:46 ` Magnus Hagander <[email protected]>
2 siblings, 0 replies; 9+ messages in thread
From: Magnus Hagander @ 2024-04-12 14:46 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Andres Freund <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Thu, Apr 4, 2024 at 1:10 AM Peter Eisentraut <[email protected]>
wrote:
> On 03.04.24 23:19, Magnus Hagander wrote:
> > When the code is this simple, we should definitely consider carrying it
> > ourselves. At least if we don't expect to need *other* functionality
> > from the same library in the future, which I doubt we will from
> libsystemd.
>
> Well, I've long had it on my list to do some integration to log directly
> to the journal, so you can preserve metadata better. I'm not sure right
> now whether this would use libsystemd, but it's not like there is
> absolutely no other systemd-related functionality that could be added.
>
Ah interesting. I hadn't thought of that use-case.
> Personally, I think this proposed change is trying to close a barndoor
> after a horse has bolted. There are many more interesting and scary
> libraries in the dependency tree of "postgres", so just picking off one
> right now doesn't really accomplish anything. The next release of
> libsystemd will drop all the compression libraries as hard dependencies,
> so the issue in that sense is gone anyway. Also, fun fact: liblzma is
> also a dependency via libxml2.
>
To be clear, I didn't mean to single out this one, just saying that it's
something we should keep in consideration in general when adding library
dependencies. Every new dependency, no matter how small, increases the
management and risks for it. And we should just be aware of that and weigh
them against each other.
As in we should *consider* it, that doesn't' mean we should necessarily
*do* it.
(And yes, there are many scary dependencies down the tree)
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/;
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-03 23:10 ` Re: Security lessons from liblzma - libsystemd Peter Eisentraut <[email protected]>
@ 2024-04-12 16:00 ` Andres Freund <[email protected]>
2024-04-16 00:35 ` Re: Security lessons from liblzma - libsystemd Michael Paquier <[email protected]>
2 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2024-04-12 16:00 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Hi,
On 2024-04-04 01:10:20 +0200, Peter Eisentraut wrote:
> On 03.04.24 23:19, Magnus Hagander wrote:
> > When the code is this simple, we should definitely consider carrying it
> > ourselves. At least if we don't expect to need *other* functionality
> > from the same library in the future, which I doubt we will from
> > libsystemd.
>
> Well, I've long had it on my list to do some integration to log directly to
> the journal, so you can preserve metadata better. I'm not sure right now
> whether this would use libsystemd, but it's not like there is absolutely no
> other systemd-related functionality that could be added.
Interesting. I think that'd in all likelihood end up using libsystemd.
> Personally, I think this proposed change is trying to close a barndoor after
> a horse has bolted. There are many more interesting and scary libraries in
> the dependency tree of "postgres", so just picking off one right now doesn't
> really accomplish anything. The next release of libsystemd will drop all
> the compression libraries as hard dependencies, so the issue in that sense
> is gone anyway. Also, fun fact: liblzma is also a dependency via libxml2.
I agree that doing this just because of future risk in liblzma is probably not
worth it. Despite soon not being an unconditional dependency of libsystemd
anymore, I'd guess that in a few months it's going to be one of the better
vetted libraries out there. But I don't think that means it's not worth
reducing the dependencies that we have unconditionally loaded.
Having a dependency to a fairly large library (~900k), which could be avoided
with ~30 LOC, is just not worth it. Independent of liblzma. Not from a
performance POV, nor from a risk POV.
I'm actually fairly bothered by us linking to libxml2. It was effectively
unmaintained for most of the last decade, with just very occasional drive-by
commits. And it's not that there weren't significant bugs or such. Maintenance
has picked up some, but it's still not well maintained, I'd say. If I wanted
to attack postgres, it's where I'd start.
My worry level, in decreasing order, about postmaster-level dependencies:
- libxml2 - effectively unmaintained, just about maintained today
- gssapi - heavily undermaintained from what I can see, lots of complicated code
- libldap - undermaintained, lots of creaky old code
- libpam - undermaintained, lots of creaky old code
- the rest
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Security lessons from liblzma - libsystemd
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-03 23:10 ` Re: Security lessons from liblzma - libsystemd Peter Eisentraut <[email protected]>
2024-04-12 16:00 ` Re: Security lessons from liblzma - libsystemd Andres Freund <[email protected]>
@ 2024-04-16 00:35 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Michael Paquier @ 2024-04-16 00:35 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Magnus Hagander <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Fri, Apr 12, 2024 at 09:00:11AM -0700, Andres Freund wrote:
> I'm actually fairly bothered by us linking to libxml2. It was effectively
> unmaintained for most of the last decade, with just very occasional drive-by
> commits. And it's not that there weren't significant bugs or such. Maintenance
> has picked up some, but it's still not well maintained, I'd say. If I wanted
> to attack postgres, it's where I'd start.
Indeed, libxml2 worries me to, as much as out-of-core extensions.
There are a bunch of these out there, some of them not that
maintained, and they could face similar attacks.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2024-04-28 11:00 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-04-03 21:19 Re: Security lessons from liblzma - libsystemd Magnus Hagander <[email protected]>
2024-04-03 21:58 ` Tom Lane <[email protected]>
2024-04-03 22:16 ` Andres Freund <[email protected]>
2024-04-03 23:10 ` Peter Eisentraut <[email protected]>
2024-04-08 10:05 ` Étienne BERSAC <[email protected]>
2024-04-12 14:46 ` Magnus Hagander <[email protected]>
2024-04-12 16:00 ` Andres Freund <[email protected]>
2024-04-16 00:35 ` Michael Paquier <[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