agora inbox for [email protected]
help / color / mirror / Atom feedusleep feature for pgbench
17+ messages / 7 participants
[nested] [flat]
* usleep feature for pgbench
@ 2007-07-05 19:12 Jan Wieck <[email protected]>
0 siblings, 2 replies; 17+ messages in thread
From: Jan Wieck @ 2007-07-05 19:12 UTC (permalink / raw)
To: pgsql-hackers
To test some changes in Slony I needed a
\usleep [microseconds|:variable]
in pgbench's scripting language to be able to have hundreds of
concurrent running transactions without totally swamping the system. I
was wondering if anyone would object to permanently adding this to the
pgbench code?
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [email protected] #
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-05 19:34 Tom Lane <[email protected]>
parent: Jan Wieck <[email protected]>
1 sibling, 1 reply; 17+ messages in thread
From: Tom Lane @ 2007-07-05 19:34 UTC (permalink / raw)
To: Jan Wieck <[email protected]>; +Cc: pgsql-hackers
Jan Wieck <[email protected]> writes:
> To test some changes in Slony I needed a
> \usleep [microseconds|:variable]
> in pgbench's scripting language to be able to have hundreds of
> concurrent running transactions without totally swamping the system. I
> was wondering if anyone would object to permanently adding this to the
> pgbench code?
How's it implemented? Does the whole pgbench app freeze up for the
duration of the sleep, or does it just delay that one client script?
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-05 20:43 Jan Wieck <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Jan Wieck @ 2007-07-05 20:43 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On 7/5/2007 3:34 PM, Tom Lane wrote:
> Jan Wieck <[email protected]> writes:
>> To test some changes in Slony I needed a
>> \usleep [microseconds|:variable]
>> in pgbench's scripting language to be able to have hundreds of
>> concurrent running transactions without totally swamping the system. I
>> was wondering if anyone would object to permanently adding this to the
>> pgbench code?
>
> How's it implemented? Does the whole pgbench app freeze up for the
> duration of the sleep, or does it just delay that one client script?
Only that one client script.
The \usleep command itself calculates a "struct timeval until" that sits
in the state struct and sets a "sleeping" flag also in the state struct.
So both are per client script/connection and multiple clients can be
sleeping in an overlapping fashion. The loop in main() then picks the
lowest timeval (if there is anyone sleeping) and uses that to calculate
the timeout for select(). doCustom() resets the sleeping flag for those
scheduled for wakeup and advances the client to the next scripted command.
I think I've coded it in a way that if one doesn't use the \usleep
command at all, it will never even call gettimeofday() and use a NULL
timeout in select() as it used to.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [email protected] #
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-05 21:30 Tom Lane <[email protected]>
parent: Jan Wieck <[email protected]>
0 siblings, 2 replies; 17+ messages in thread
From: Tom Lane @ 2007-07-05 21:30 UTC (permalink / raw)
To: Jan Wieck <[email protected]>; +Cc: pgsql-hackers
Jan Wieck <[email protected]> writes:
> I think I've coded it in a way that if one doesn't use the \usleep
> command at all, it will never even call gettimeofday() and use a NULL
> timeout in select() as it used to.
Did you check that the observed performance for non-usleep-using scripts
didn't change? If this extra overhead causes a reduction in reported
TPS rates it would make it hard to compare older and newer tests.
Other than that I've got no objection to it.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-05 21:49 Alvaro Herrera <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 17+ messages in thread
From: Alvaro Herrera @ 2007-07-05 21:49 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jan Wieck <[email protected]>; pgsql-hackers
Tom Lane wrote:
> Jan Wieck <[email protected]> writes:
> > I think I've coded it in a way that if one doesn't use the \usleep
> > command at all, it will never even call gettimeofday() and use a NULL
> > timeout in select() as it used to.
>
> Did you check that the observed performance for non-usleep-using scripts
> didn't change? If this extra overhead causes a reduction in reported
> TPS rates it would make it hard to compare older and newer tests.
I keep wondering, why is that important? If you want to compare results
of different PG versions, surely you can run the tests with the newest
version of pgbench?
--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
"Those who use electric razors are infidels destined to burn in hell while
we drink from rivers of beer, download free vids and mingle with naked
well shaved babes." (http://slashdot.org/comments.pl?sid=44793&cid=4647152)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-05 21:54 Tom Lane <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Tom Lane @ 2007-07-05 21:54 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Jan Wieck <[email protected]>; pgsql-hackers
Alvaro Herrera <[email protected]> writes:
> Tom Lane wrote:
>> Did you check that the observed performance for non-usleep-using scripts
>> didn't change? If this extra overhead causes a reduction in reported
>> TPS rates it would make it hard to compare older and newer tests.
> I keep wondering, why is that important? If you want to compare results
> of different PG versions, surely you can run the tests with the newest
> version of pgbench?
It's more about not having to repeat old test cases every time you want
to have numbers comparable to your latest case.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-06 02:58 Jan Wieck <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 17+ messages in thread
From: Jan Wieck @ 2007-07-06 02:58 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On 7/5/2007 5:30 PM, Tom Lane wrote:
> Jan Wieck <[email protected]> writes:
>> I think I've coded it in a way that if one doesn't use the \usleep
>> command at all, it will never even call gettimeofday() and use a NULL
>> timeout in select() as it used to.
>
> Did you check that the observed performance for non-usleep-using scripts
> didn't change? If this extra overhead causes a reduction in reported
> TPS rates it would make it hard to compare older and newer tests.
Given pgbench's unpredictability of results ... lets see.
I ran
dropdb x
createdb x
pgbench -i -s10 x
psql -c 'checkpoint' x
sleep 1
psql -c 'checkpoint' x
pgbench -s10 -c5 -t10000 x
pgbench -s10 -c5 -t10000 x
pgbench -s10 -c5 -t10000 x
Original pgbench reported 39, 37 and 33 TPS. Having my patch applied it
reported 40, 38 and 33 TPS. Inserting a "\usleep 1" after the update to
accounts of a default equivalent script changed those numbers to 40, 37
and 33. I interpret that as "does not change observed performance".
>
> Other than that I've got no objection to it.
Will be committed after adjusting the README.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [email protected] #
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-06 07:46 Greg Smith <[email protected]>
parent: Jan Wieck <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Greg Smith @ 2007-07-06 07:46 UTC (permalink / raw)
To: pgsql-hackers
On Thu, 5 Jul 2007, Jan Wieck wrote:
> Original pgbench reported 39, 37 and 33 TPS. Having my patch applied it
> reported 40, 38 and 33 TPS. Inserting a "\usleep 1" after the update to
> accounts of a default equivalent script changed those numbers to 40, 37 and
> 33. I interpret that as "does not change observed performance".
Tell you what: put your work into a patch, send it to the list, and I'll
test that it doesn't degrade results for you. If your pgbench results are
in the <40 TPS range even with that low of a scale, you're not in a
position to tell whether it has a negative performance impact. That
select statement you're fiddling with can turn into a bottleneck at high
client loads, and from your description I can't tell if you've made that
worse, but you'll never see it unless you're pushing, say,
>1000 TPS and >50 clients. Also: 3 pgbench results at one client load
is quite a bit short of proving no impact on performance; I'll queue up 50
or so, which is where I start to trust results from that unruly tool.
This is actually a feature I'd be kind of interested to have, because it
would allow you to pass two (or more) script files to pgbench and adjust
the transaction mix. What happens when you do that right now is that
inevitably all the clients get blocked at once on whatever the hardest to
execute transaction is, and the results are kind of deceptive as a result.
--
* Greg Smith [email protected] http://www.gregsmith.com Baltimore, MD
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-06 14:44 Peter Eisentraut <[email protected]>
parent: Jan Wieck <[email protected]>
1 sibling, 1 reply; 17+ messages in thread
From: Peter Eisentraut @ 2007-07-06 14:44 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Jan Wieck <[email protected]>
Am Donnerstag, 5. Juli 2007 21:12 schrieb Jan Wieck:
> To test some changes in Slony I needed a
>
> \usleep [microseconds|:variable]
>
> in pgbench's scripting language to be able to have hundreds of
> concurrent running transactions without totally swamping the system. I
> was wondering if anyone would object to permanently adding this to the
> pgbench code?
Or maybe a \sleep command that takes units, if it's not too much work.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-06 17:24 Jan Wieck <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 17+ messages in thread
From: Jan Wieck @ 2007-07-06 17:24 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On 7/6/2007 10:44 AM, Peter Eisentraut wrote:
> Am Donnerstag, 5. Juli 2007 21:12 schrieb Jan Wieck:
>> To test some changes in Slony I needed a
>>
>> \usleep [microseconds|:variable]
>>
>> in pgbench's scripting language to be able to have hundreds of
>> concurrent running transactions without totally swamping the system. I
>> was wondering if anyone would object to permanently adding this to the
>> pgbench code?
>
> Or maybe a \sleep command that takes units, if it's not too much work.
>
You mean as a second, optional argument? Good idea.
us = microseconds
ms = milliseconds
s = seconds (default)
\sleep {value|:variable} [us|ms|s]
Is that okay with everyone?
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [email protected] #
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-06 17:32 Heikki Linnakangas <[email protected]>
parent: Jan Wieck <[email protected]>
1 sibling, 2 replies; 17+ messages in thread
From: Heikki Linnakangas @ 2007-07-06 17:32 UTC (permalink / raw)
To: Jan Wieck <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Jan Wieck wrote:
> On 7/6/2007 10:44 AM, Peter Eisentraut wrote:
>> Am Donnerstag, 5. Juli 2007 21:12 schrieb Jan Wieck:
>>> To test some changes in Slony I needed a
>>>
>>> \usleep [microseconds|:variable]
>>>
>>> in pgbench's scripting language to be able to have hundreds of
>>> concurrent running transactions without totally swamping the system. I
>>> was wondering if anyone would object to permanently adding this to the
>>> pgbench code?
>>
>> Or maybe a \sleep command that takes units, if it's not too much work.
>>
>
> You mean as a second, optional argument? Good idea.
>
> us = microseconds
> ms = milliseconds
> s = seconds (default)
>
> \sleep {value|:variable} [us|ms|s]
>
> Is that okay with everyone?
I won't object, but is it really worth the trouble?
Can you do microsecond precision sleeps, and on what platforms? How much
overhead is there? IIRC, on Linux the minimum time you can sleep depends
on CONFIG_HZ, and the default was 10 ms until recently.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-06 18:35 Jan Wieck <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 0 replies; 17+ messages in thread
From: Jan Wieck @ 2007-07-06 18:35 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
On 7/6/2007 1:32 PM, Heikki Linnakangas wrote:
> Jan Wieck wrote:
>> On 7/6/2007 10:44 AM, Peter Eisentraut wrote:
>>> Am Donnerstag, 5. Juli 2007 21:12 schrieb Jan Wieck:
>>>> To test some changes in Slony I needed a
>>>>
>>>> \usleep [microseconds|:variable]
>>>>
>>>> in pgbench's scripting language to be able to have hundreds of
>>>> concurrent running transactions without totally swamping the system. I
>>>> was wondering if anyone would object to permanently adding this to the
>>>> pgbench code?
>>>
>>> Or maybe a \sleep command that takes units, if it's not too much work.
>>>
>>
>> You mean as a second, optional argument? Good idea.
>>
>> us = microseconds
>> ms = milliseconds
>> s = seconds (default)
>>
>> \sleep {value|:variable} [us|ms|s]
>>
>> Is that okay with everyone?
>
> I won't object, but is it really worth the trouble?
>
> Can you do microsecond precision sleeps, and on what platforms? How much
> overhead is there? IIRC, on Linux the minimum time you can sleep depends
> on CONFIG_HZ, and the default was 10 ms until recently.
>
It is what you tell select(2) in the struct timeval. Why limit it to
some arbitrary precision?
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [email protected] #
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-07 03:13 Tom Lane <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 0 replies; 17+ messages in thread
From: Tom Lane @ 2007-07-07 03:13 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Jan Wieck <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
Heikki Linnakangas <[email protected]> writes:
> Jan Wieck wrote:
>> You mean as a second, optional argument? Good idea.
>>
>> us = microseconds
>> ms = milliseconds
>> s = seconds (default)
>>
>> \sleep {value|:variable} [us|ms|s]
>>
>> Is that okay with everyone?
> I won't object, but is it really worth the trouble?
If we don't get it right now, we'll be stuck with the API later.
+1 for the definition suggested above.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-07 08:36 Peter Eisentraut <[email protected]>
parent: Jan Wieck <[email protected]>
1 sibling, 1 reply; 17+ messages in thread
From: Peter Eisentraut @ 2007-07-07 08:36 UTC (permalink / raw)
To: Jan Wieck <[email protected]>; +Cc: pgsql-hackers
Jan Wieck wrote:
> You mean as a second, optional argument? Good idea.
I think it should only be one argument, with the units being part of the
variable value, if any.
> us = microseconds
> ms = milliseconds
> s = seconds (default)
>
> \sleep {value|:variable} [us|ms|s]
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-07 15:40 Tom Lane <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2007-07-07 15:40 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Jan Wieck <[email protected]>; pgsql-hackers
Peter Eisentraut <[email protected]> writes:
> Jan Wieck wrote:
>> You mean as a second, optional argument? Good idea.
> I think it should only be one argument, with the units being part of the
> variable value, if any.
No, because that makes it hard to do things like "sleep for a small
random number of microseconds". If pgbench had string concatenation
ability in its metalanguage then what you suggest would be sensible,
but let us not go there.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: usleep feature for pgbench
@ 2007-07-07 22:08 Jan Wieck <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Jan Wieck @ 2007-07-07 22:08 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
On 7/7/2007 11:40 AM, Tom Lane wrote:
> Peter Eisentraut <[email protected]> writes:
>> Jan Wieck wrote:
>>> You mean as a second, optional argument? Good idea.
>
>> I think it should only be one argument, with the units being part of the
>> variable value, if any.
>
> No, because that makes it hard to do things like "sleep for a small
> random number of microseconds". If pgbench had string concatenation
> ability in its metalanguage then what you suggest would be sensible,
> but let us not go there.
That is exactly the reason why I made it an optional argument. The usage
example in the README file shows how to actually do a random sleep
between 1000 and 2500 ms.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [email protected] #
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v2 1/2] Support changing a column into a stored generated column
@ 2026-03-29 19:45 Alberto Piai <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Alberto Piai @ 2026-03-29 19:45 UTC (permalink / raw)
This adds an ALTER TABLE subcommand to turn a regular column
into a stored generated column:
... ALTER COLUMN c ADD GENERATED ALWAYS as (expr) STORED
The syntax is chosen to be similar to
... ALTER COLUMN ... ADD GENERATED ... AS IDENTITY
with the difference that in this case, since we're dealing with a
generated column, only ALWAYS is supported. Additionally, STORED must
always be specified.
This new operation fits together with SET EXPRESSION and DROP
EXPRESSION. Phase 2 happens in the same pass as the former, in order to
run the cleanup code in ATPostAlterTypeCleanup, without which for
example we would not re-check constraints when rewriting the table.
Partitioning/inheritance is supported in the same way as DROP
EXPRESSION: it is allowed to change the whole inheritace tree to/from a
generated column at once; it is forbidden to change the parent table
ONLY and it is forbidden to change a partition directly. See
8bf6ec3ba3a44448817af47a080587f3b71bee08 and the associated discussion.
There is one limitation: currently DROP EXPRESSION does not allow to
change an inheritance tree of depth > 2. This seems like an oversight,
but in order to not feature-creep this commit, this is postponed for
later; it should then be fixed for both DROP EXPRESSION and this new
command.
This is mostly useful as a first step to be able to add a stored
generated column without rewriting the table under an exclusive lock.
For ease of review, the operation as of this commit always rewrites the
contents of the column using the new generated expression.
---
src/backend/commands/tablecmds.c | 187 +++++++++++++++++-
src/backend/parser/gram.y | 31 +++
src/include/nodes/parsenodes.h | 1 +
.../test_ddl_deparse/test_ddl_deparse.c | 3 +
src/test/regress/expected/alter_table.out | 126 ++++++++++++
src/test/regress/sql/alter_table.sql | 76 +++++++
6 files changed, 423 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c69c12dc014..66622bf4837 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -760,6 +760,14 @@ static void ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation
static void ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab,
Relation rel, PartitionCmd *cmd,
AlterTableUtilityContext *context);
+static void ATPrepAddGeneratedAsExprStored(Relation rel,
+ AlterTableCmd *cmd,
+ bool recurse,
+ bool recursing, LOCKMODE lockmode);
+static ObjectAddress ATExecAddGeneratedAsExprStored(AlteredTableInfo *tab,
+ Relation rel,
+ const char *colName,
+ Constraint *def);
/* ----------------------------------------------------------------
* DefineRelation
@@ -4743,6 +4751,7 @@ AlterTableGetLockLevel(List *cmds)
case AT_AddIdentity:
case AT_DropIdentity:
case AT_SetIdentity:
+ case AT_AddGeneratedAsExprStored:
case AT_SetExpression:
case AT_DropExpression:
case AT_SetCompression:
@@ -5067,6 +5076,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
pass = AT_PASS_SET_EXPRESSION;
break;
+ case AT_AddGeneratedAsExprStored:
+ ATSimplePermissions(cmd->subtype, rel,
+ ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
+ ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
+ ATPrepAddGeneratedAsExprStored(rel, cmd, recurse, recursing, lockmode);
+ pass = AT_PASS_SET_EXPRESSION;
+ break;
case AT_DropExpression: /* ALTER COLUMN DROP EXPRESSION */
ATSimplePermissions(cmd->subtype, rel,
ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
@@ -5461,6 +5477,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
case AT_SetExpression:
address = ATExecSetExpression(tab, rel, cmd->name, cmd->def, lockmode);
break;
+ case AT_AddGeneratedAsExprStored:
+ Assert(IsA(cmd->def, Constraint));
+ address = ATExecAddGeneratedAsExprStored(tab, rel,
+ cmd->name,
+ (Constraint *) cmd->def);
+ break;
case AT_DropExpression:
address = ATExecDropExpression(rel, cmd->name, cmd->missing_ok, lockmode);
break;
@@ -6667,6 +6689,8 @@ alter_table_type_to_string(AlterTableType cmdtype)
return "ALTER COLUMN ... SET NOT NULL";
case AT_SetExpression:
return "ALTER COLUMN ... SET EXPRESSION";
+ case AT_AddGeneratedAsExprStored:
+ return "ALTER COLUMN ... ADD GENERATED ALWAYS AS (...) STORED";
case AT_DropExpression:
return "ALTER COLUMN ... DROP EXPRESSION";
case AT_SetStatistics:
@@ -8823,6 +8847,165 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName,
return address;
}
+/*
+ * Preparation for
+ *
+ * ALTER TABLE ALTER COLUMN ADD GENERATED ALWAYS AS expr STORED
+ *
+ * Checks whether recursion is allowed, following the same logic as ATPrepDropExpression.
+ */
+static void
+ATPrepAddGeneratedAsExprStored(Relation rel,
+ AlterTableCmd *cmd,
+ bool recurse,
+ bool recursing, LOCKMODE lockmode
+)
+{
+ /*
+ * Reject ONLY if there are child tables. See ATPrepDropExpression.
+ */
+ if (!recurse &&
+ find_inheritance_children(RelationGetRelid(rel), lockmode))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ALTER TABLE / ADD GENERATED ALWAYS AS (expr) STORED must be applied to child tables too")));
+
+ /*
+ * Cannot change only inherited columns to be stored generated columns.
+ */
+ if (!recursing)
+ {
+ HeapTuple tuple;
+ Form_pg_attribute attTup;
+
+ tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), cmd->name);
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("column \"%s\" of relation \"%s\" does not exist",
+ cmd->name, RelationGetRelationName(rel))));
+
+ attTup = (Form_pg_attribute) GETSTRUCT(tuple);
+
+ if (attTup->attinhcount > 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot change inherited column to be a stored generated column")));
+ }
+}
+
+/*
+ * ALTER TABLE ALTER COLUMN ADD GENERATED ALWAYS AS expr STORED
+ */
+static ObjectAddress
+ATExecAddGeneratedAsExprStored(AlteredTableInfo *tab,
+ Relation rel,
+ const char *colName,
+ Constraint *def)
+{
+ HeapTuple tuple;
+ Form_pg_attribute attTup;
+ AttrNumber attnum;
+ ObjectAddress address;
+ Expr *defval;
+ NewColumnValue *newval;
+ RawColumnDefault *rawEnt;
+ Relation pg_attribute;
+
+ Assert(def->raw_expr != NULL);
+ Assert(def->cooked_expr == NULL);
+ Assert(def->generated_when == ATTRIBUTE_IDENTITY_ALWAYS);
+ Assert(def->generated_kind == ATTRIBUTE_GENERATED_STORED);
+
+ tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("column \"%s\" of relation \"%s\" does not exist",
+ colName, RelationGetRelationName(rel))));
+
+ attTup = (Form_pg_attribute) GETSTRUCT(tuple);
+
+ attnum = attTup->attnum;
+ if (attnum <= 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter system column \"%s\"",
+ colName)));
+
+ if (attTup->attgenerated)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("column \"%s\" of relation \"%s\" is already a generated column",
+ colName, RelationGetRelationName(rel))));
+
+ /* Mark as generated stored in pg_attribute */
+ pg_attribute = table_open(AttributeRelationId, RowExclusiveLock);
+ attTup->attgenerated = ATTRIBUTE_GENERATED_STORED;
+ CatalogTupleUpdate(pg_attribute, &tuple->t_self, tuple);
+ table_close(pg_attribute, RowExclusiveLock);
+
+ /* Make above changes visible */
+ CommandCounterIncrement();
+
+ ReleaseSysCache(tuple);
+
+ /*
+ * Find everything that depends on the column (constraints, indexes, etc),
+ * and record enough information to let us recreate the objects.
+ */
+ RememberAllDependentForRebuilding(tab, AT_AddGeneratedAsExprStored,
+ rel, attnum, colName);
+
+ /*
+ * Remove previous default value, if any, and store the new generator
+ * expression.
+ */
+ RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT,
+ false, false);
+
+ rawEnt = palloc_object(RawColumnDefault);
+ rawEnt->attnum = attnum;
+ rawEnt->raw_default = def->raw_expr;
+ rawEnt->generated = def->generated_kind;
+ AddRelationNewConstraints(rel, list_make1(rawEnt), NIL,
+ false, true, false, NULL);
+
+ /* Make above changes visible */
+ CommandCounterIncrement();
+
+ /*
+ * Clear all the missing values if we're rewriting the table, since this
+ * renders them pointless.
+ */
+ RelationClearMissing(rel);
+
+ /* Make above changes visible */
+ CommandCounterIncrement();
+
+ /* Drop any pg_statistic entry for the column */
+ RemoveStatistics(RelationGetRelid(rel), attnum);
+
+ /* Build a concrete expression for the new default (generated) value */
+ defval = (Expr *) build_column_default(rel, attnum);
+ defval = expression_planner(defval);
+
+ /* Schedule a rewrite */
+ newval = palloc0_object(NewColumnValue);
+ newval->attnum = attnum;
+ newval->expr = defval;
+ newval->is_generated = true;
+ tab->newvals = lappend(tab->newvals, newval);
+ tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
+
+ InvokeObjectPostAlterHook(RelationRelationId,
+ RelationGetRelid(rel), attnum);
+
+ ObjectAddressSubSet(address, RelationRelationId,
+ RelationGetRelid(rel), attnum);
+ return address;
+}
+
/*
* ALTER TABLE ALTER COLUMN DROP EXPRESSION
*/
@@ -15290,7 +15473,9 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
SysScanDesc scan;
HeapTuple depTup;
- Assert(subtype == AT_AlterColumnType || subtype == AT_SetExpression);
+ Assert(subtype == AT_AlterColumnType
+ || subtype == AT_SetExpression
+ || subtype == AT_AddGeneratedAsExprStored);
depRel = table_open(DependRelationId, RowExclusiveLock);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0fea726cdd5..315ee7d94e2 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2717,6 +2717,37 @@ alter_table_cmd:
n->name = $3;
n->def = (Node *) c;
+ $$ = (Node *) n;
+ }
+ /* ALTER TABLE <name> ALTER [COLUMN] <colname> ADD GENERATED ALWAYS AS ( <expression> ) STORED */
+ | ALTER opt_column ColId ADD_P GENERATED generated_when AS '(' a_expr ')' STORED
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ Constraint *c = makeNode(Constraint);
+
+ c->contype = CONSTR_GENERATED;
+ c->generated_when = $6;
+ c->raw_expr = $9;
+ c->cooked_expr = NULL;
+ c->generated_kind = ATTRIBUTE_GENERATED_STORED;
+ c->location = @5;
+
+ /*
+ * Like in the case of ColConstraintElem, we cannot handle
+ * this in the grammar because IDENTITY allows both ALWAYS
+ * and BY DEFAULT, while generated columns only allow
+ * ALWAYS. This would lead to shift/reduce conflicts.
+ */
+ if (c->generated_when != ATTRIBUTE_IDENTITY_ALWAYS)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("for a generated column, GENERATED ALWAYS must be specified"),
+ parser_errposition(@6)));
+
+ n->subtype = AT_AddGeneratedAsExprStored;
+ n->name = $3;
+ n->def = (Node *) c;
+
$$ = (Node *) n;
}
/* ALTER TABLE <name> ALTER [COLUMN] <colname> SET <sequence options>/RESET */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index df431220ac5..74958ef0dfa 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2506,6 +2506,7 @@ typedef enum AlterTableType
AT_CookedColumnDefault, /* add a pre-cooked column default */
AT_DropNotNull, /* alter column drop not null */
AT_SetNotNull, /* alter column set not null */
+ AT_AddGeneratedAsExprStored, /* add generated always as (...) stored */
AT_SetExpression, /* alter column set expression */
AT_DropExpression, /* alter column drop expression */
AT_SetStatistics, /* alter column set statistics */
diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
index 64a1dfa9f79..3132ceac61f 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -129,6 +129,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_SetNotNull:
strtype = "SET NOT NULL";
break;
+ case AT_AddGeneratedAsExprStored:
+ strtype = "ADD GENERATED ALWAYS AS (...) STORED";
+ break;
case AT_SetExpression:
strtype = "SET EXPRESSION";
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ccd79dfecc0..2567d918ec3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4863,3 +4863,129 @@ drop publication pub1;
drop schema alter1 cascade;
drop schema alter2 cascade;
NOTICE: drop cascades to table alter2.t1
+-- Tests for ALTER COLUMN ... ADD GENERATED ALWAYS as ( expr ) STORED
+-- turning a regular column into a stored generated column
+create schema testgen;
+create table testgen.t1 (a int, b int not null);
+insert into testgen.t1 (a, b)
+ select x, x from generate_series(1, 10) x;
+alter table testgen.t1 alter column b
+ add generated always as (a * 2) stored;
+\d+ testgen.t1
+ Table "testgen.t1"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+------------------------------------+---------+--------------+-------------
+ a | integer | | | | plain | |
+ b | integer | | not null | generated always as (a * 2) stored | plain | |
+Not-null constraints:
+ "t1_b_not_null" NOT NULL "b"
+
+select a, b, a * 2 as expected, b = (a * 2) as correct
+ from testgen.t1 order by a;
+ a | b | expected | correct
+----+----+----------+---------
+ 1 | 2 | 2 | t
+ 2 | 4 | 4 | t
+ 3 | 6 | 6 | t
+ 4 | 8 | 8 | t
+ 5 | 10 | 10 | t
+ 6 | 12 | 12 | t
+ 7 | 14 | 14 | t
+ 8 | 16 | 16 | t
+ 9 | 18 | 18 | t
+ 10 | 20 | 20 | t
+(10 rows)
+
+insert into testgen.t1 (a, b) values (10, 20);
+ERROR: cannot insert a non-DEFAULT value into column "b"
+DETAIL: Column "b" is a generated column.
+insert into testgen.t1 (a, b) values (10, 21);
+ERROR: cannot insert a non-DEFAULT value into column "b"
+DETAIL: Column "b" is a generated column.
+drop table testgen.t1;
+-- turning a regular column into a stored generated column
+-- fails when another constraint conflicts with the new expression
+create table testgen.t2 (a int, b int not null);
+insert into testgen.t2 (a, b) select x, x * 2 from generate_series(0, 5) x;
+alter table testgen.t2 add constraint chk_gen_clause check (b = a * 2);
+select pg_relation_filenode('testgen.t2') as t2_filenode_before \gset
+alter table testgen.t2 alter column b add generated always as (a * 3) stored;
+ERROR: check constraint "chk_gen_clause" of relation "t2" is violated by some row
+select pg_relation_filenode('testgen.t2') as t2_filenode_after \gset
+select :t2_filenode_before = :t2_filenode_after as did_not_rewrite;
+ did_not_rewrite
+-----------------
+ t
+(1 row)
+
+\d+ testgen.t2
+ Table "testgen.t2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
+ b | integer | | not null | | plain | |
+Check constraints:
+ "chk_gen_clause" CHECK (b = (a * 2))
+Not-null constraints:
+ "t2_b_not_null" NOT NULL "b"
+
+drop table testgen.t2;
+-- rewrite an indexed column
+create table testgen.t3 (a int, b int);
+create index idx_b on testgen.t3 (b);
+insert into testgen.t3 (a, b) select x, x from generate_series(1, 10) x;
+select pg_relation_filenode('testgen.idx_b') as idx_filenode_before \gset
+alter table testgen.t3 alter column b add generated always as (a * 2) stored;
+select pg_relation_filenode('testgen.idx_b') as idx_filenode_after \gset
+select :idx_filenode_before != :idx_filenode_after as did_rewrite_idx;
+ did_rewrite_idx
+-----------------
+ t
+(1 row)
+
+-- tests for invalid invocations
+alter table doesnotexist alter column foo
+ add generated always as (bar * 2) stored;
+ERROR: relation "doesnotexist" does not exist
+create table testgen.t1 (a int);
+alter table testgen.t1 alter column doesnotexist
+ add generated always as (bar * 2) stored;
+ERROR: column "doesnotexist" of relation "t1" does not exist
+alter table testgen.t1 add column b int;
+alter table testgen.t1 alter column b
+ add generated always as (doesnotexist * 2) stored;
+ERROR: column "doesnotexist" does not exist
+-- invalid: only supports ALWAYS
+alter table testgen.t1 alter column b
+ add generated by default as (a * 2) stored;
+ERROR: for a generated column, GENERATED ALWAYS must be specified
+LINE 2: add generated by default as (a * 2) stored;
+ ^
+-- invalid: only supports STORED
+alter table testgen.t1 alter column b add generated always as (a * 2);
+ERROR: syntax error at or near ";"
+LINE 1: ...e testgen.t1 alter column b add generated always as (a * 2);
+ ^
+alter table testgen.t1 alter column b add generated always as (a * 2) virtual;
+ERROR: syntax error at or near "virtual"
+LINE 1: ...n.t1 alter column b add generated always as (a * 2) virtual;
+ ^
+-- invalid: b is already a generated column
+create table testgen.t2 (a int, b int generated always as (a * 2) stored);
+alter table testgen.t2 alter column b add generated always as (a * 2) stored;
+ERROR: column "b" of relation "t2" is already a generated column
+drop table testgen.t2;
+create table testgen.t3 (a int, b int);
+-- invalid: expr must be immutable
+alter table testgen.t3 alter column b
+ add generated always as (a + random()) stored;
+ERROR: generation expression is not immutable
+-- invalid: expr cannot use subselects
+alter table testgen.t3 alter column b
+ add generated always as (a + (select 1)) stored;
+ERROR: cannot use subquery in column generation expression
+drop table testgen.t3;
+drop schema testgen cascade;
+NOTICE: drop cascades to 4 other objects
+DETAIL: drop cascades to table testgen.t3
+drop cascades to table testgen.t1
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f5f13bbd3e7..76187083289 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -3159,3 +3159,79 @@ alter table alter1.t1 set schema alter2;
drop publication pub1;
drop schema alter1 cascade;
drop schema alter2 cascade;
+
+-- Tests for ALTER COLUMN ... ADD GENERATED ALWAYS as ( expr ) STORED
+-- turning a regular column into a stored generated column
+create schema testgen;
+
+create table testgen.t1 (a int, b int not null);
+insert into testgen.t1 (a, b)
+ select x, x from generate_series(1, 10) x;
+alter table testgen.t1 alter column b
+ add generated always as (a * 2) stored;
+\d+ testgen.t1
+select a, b, a * 2 as expected, b = (a * 2) as correct
+ from testgen.t1 order by a;
+insert into testgen.t1 (a, b) values (10, 20);
+insert into testgen.t1 (a, b) values (10, 21);
+drop table testgen.t1;
+
+-- turning a regular column into a stored generated column
+-- fails when another constraint conflicts with the new expression
+create table testgen.t2 (a int, b int not null);
+insert into testgen.t2 (a, b) select x, x * 2 from generate_series(0, 5) x;
+alter table testgen.t2 add constraint chk_gen_clause check (b = a * 2);
+select pg_relation_filenode('testgen.t2') as t2_filenode_before \gset
+alter table testgen.t2 alter column b add generated always as (a * 3) stored;
+select pg_relation_filenode('testgen.t2') as t2_filenode_after \gset
+select :t2_filenode_before = :t2_filenode_after as did_not_rewrite;
+\d+ testgen.t2
+drop table testgen.t2;
+
+-- rewrite an indexed column
+create table testgen.t3 (a int, b int);
+create index idx_b on testgen.t3 (b);
+insert into testgen.t3 (a, b) select x, x from generate_series(1, 10) x;
+select pg_relation_filenode('testgen.idx_b') as idx_filenode_before \gset
+alter table testgen.t3 alter column b add generated always as (a * 2) stored;
+select pg_relation_filenode('testgen.idx_b') as idx_filenode_after \gset
+select :idx_filenode_before != :idx_filenode_after as did_rewrite_idx;
+drop table testgen.t3;
+
+-- tests for invalid invocations
+alter table doesnotexist alter column foo
+ add generated always as (bar * 2) stored;
+
+create table testgen.t1 (a int);
+
+alter table testgen.t1 alter column doesnotexist
+ add generated always as (bar * 2) stored;
+
+alter table testgen.t1 add column b int;
+
+alter table testgen.t1 alter column b
+ add generated always as (doesnotexist * 2) stored;
+
+-- invalid: only supports ALWAYS
+alter table testgen.t1 alter column b
+ add generated by default as (a * 2) stored;
+
+-- invalid: only supports STORED
+alter table testgen.t1 alter column b add generated always as (a * 2);
+alter table testgen.t1 alter column b add generated always as (a * 2) virtual;
+
+-- invalid: b is already a generated column
+create table testgen.t2 (a int, b int generated always as (a * 2) stored);
+alter table testgen.t2 alter column b add generated always as (a * 2) stored;
+drop table testgen.t2;
+
+create table testgen.t3 (a int, b int);
+-- invalid: expr must be immutable
+alter table testgen.t3 alter column b
+ add generated always as (a + random()) stored;
+-- invalid: expr cannot use subselects
+alter table testgen.t3 alter column b
+ add generated always as (a + (select 1)) stored;
+drop table testgen.t3;
+
+drop schema testgen cascade;
base-commit: 01d58d7e3ff3f7482ff478cb4a49c48aad276138
--
2.47.0
--tyerjrpxgsfvwown
Content-Type: text/x-patch; charset=utf-8
Content-Disposition: attachment;
filename="v2-0002-Try-to-avoid-a-rewrite-when-adding-a-stored-gener.patch"
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2026-03-29 19:45 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2007-07-05 19:12 usleep feature for pgbench Jan Wieck <[email protected]>
2007-07-05 19:34 ` Tom Lane <[email protected]>
2007-07-05 20:43 ` Jan Wieck <[email protected]>
2007-07-05 21:30 ` Tom Lane <[email protected]>
2007-07-05 21:49 ` Alvaro Herrera <[email protected]>
2007-07-05 21:54 ` Tom Lane <[email protected]>
2007-07-06 02:58 ` Jan Wieck <[email protected]>
2007-07-06 07:46 ` Greg Smith <[email protected]>
2007-07-06 14:44 ` Peter Eisentraut <[email protected]>
2007-07-06 17:24 ` Jan Wieck <[email protected]>
2007-07-06 17:32 ` Heikki Linnakangas <[email protected]>
2007-07-06 18:35 ` Jan Wieck <[email protected]>
2007-07-07 03:13 ` Tom Lane <[email protected]>
2007-07-07 08:36 ` Peter Eisentraut <[email protected]>
2007-07-07 15:40 ` Tom Lane <[email protected]>
2007-07-07 22:08 ` Jan Wieck <[email protected]>
2026-03-29 19:45 [PATCH v2 1/2] Support changing a column into a stored generated column Alberto Piai <[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