I wrote:
> The squirrely-ness around identity is that while this now works:
> regression=# CREATE TABLE itest8 (f1 int);
> CREATE TABLE
> regression=# ALTER TABLE itest8
> regression-# ADD COLUMN f2 int NOT NULL,
> regression-# ALTER COLUMN f2 ADD GENERATED ALWAYS AS IDENTITY;
> ALTER TABLE
Alvaro Herrera writes:
> I didn't review in detail, but it seems good to me. I especially liked
> getting rid of the ProcessedConstraint code, and the additional test
> cases.
Thanks for looking!
Yeah, all those test cases expose situations where we misbehave
today :-(. I wish this were small
On 2020-Jan-14, Tom Lane wrote:
> I wrote:
> > [ fix-alter-table-order-of-operations-3.patch ]
>
> Rebased again, fixing a minor conflict with f595117e2.
>
> > I'd kind of like to get this cleared out of my queue soon.
> > Does anyone intend to review it further?
>
> If I don't hear objections
Sergei Kornilov writes:
> I am clearly not a good reviewer for such changes... But for a note: I read
> the v4 patch and have no useful comments. Good new tests, reasonable code
> changes to fix multiple bug reports.
Thanks for looking!
> The patch is proposed only for the master branch,
Hello
Thank you!
I am clearly not a good reviewer for such changes... But for a note: I read the
v4 patch and have no useful comments. Good new tests, reasonable code changes
to fix multiple bug reports.
The patch is proposed only for the master branch, right?
regards, Sergei
I wrote:
> [ fix-alter-table-order-of-operations-3.patch ]
Rebased again, fixing a minor conflict with f595117e2.
> I'd kind of like to get this cleared out of my queue soon.
> Does anyone intend to review it further?
If I don't hear objections pretty darn quick, I'm going to
go ahead and push
I wrote:
>> [ fix-alter-table-order-of-operations-1.patch ]
> The cfbot noticed that this failed to apply over a recent commit,
> so here's v2. No substantive changes.
Another rebase required :-(. Still no code changes from v1, but this
time I remembered to add a couple more test cases that I'd
I wrote:
> [ fix-alter-table-order-of-operations-1.patch ]
The cfbot noticed that this failed to apply over a recent commit,
so here's v2. No substantive changes.
regards, tom lane
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index
I wrote:
> Anyway, with the benefit of more time to let this thing percolate
> in my hindbrain, I am thinking that the fundamental error we've made
> is to do transformAlterTableStmt in advance of execution *at all*.
> The idea I now have is to scrap that, and instead apply the
> parse_utilcmd.c
Alvaro Herrera writes:
> On 2019-Oct-29, Tom Lane wrote:
>> The thing I think you are actually worried about is the interaction
>> with event triggers, which is already a pretty horrid mess in this
>> code today. I don't really follow the comment here about
>> "ordering of queued commands". It
On 2019-Oct-29, Tom Lane wrote:
> The thing I think you are actually worried about is the interaction
> with event triggers, which is already a pretty horrid mess in this
> code today. I don't really follow the comment here about
> "ordering of queued commands". It looks like that comment dates
[ starting to think about this issue again ]
I wrote:
> Robert Haas writes:
>> I mean, in an ideal world, I think we'd never call back out to
>> ProcessUtility() from within AlterTable(). That seems like a pretty
>> clear layering violation. I assume the reason we've never tried to do
>>
Alvaro Herrera writes:
> Tom: CFbot says this patch doesn't apply anymore. Could you please
> rebase?
Robert doesn't like the whole approach [1], so I'm not seeing much
point in rebasing the current patch. The idea I'd been thinking
about instead was to invent a new AlterTableType enum value
On 2019-Aug-01, Thomas Munro wrote:
> With my hacker hat: Hmm. I haven't looked at the patch, but not
> passing down the QueryEnvironment when recursing is probably my fault,
> and folding all such things into a new mechanism that would avoid such
> bugs in the future sounds like a reasonable
> This review seems not very on-point, because I made no claim to have fixed
> any of those bugs. The issue at the moment is how to structure the code
I am sorry for that and I have another question now. I researched the related
code and find something as below:
Code:
On Tue, Jul 2, 2019 at 2:00 PM Tom Lane wrote:
> movead li writes:
> > I applied the 'alter-table-with-recursive-process-utility-calls-wip.patch'
> > on the master(e788e849addd56007a0e75f3b5514f294a0f3bca). And
> > when I test the cases, I find it works well on 'alter table t1 add column
> > f2
movead li writes:
> I applied the 'alter-table-with-recursive-process-utility-calls-wip.patch'
> on the master(e788e849addd56007a0e75f3b5514f294a0f3bca). And
> when I test the cases, I find it works well on 'alter table t1 add column
> f2 int not null, alter column f2 add generated always as
I applied the 'alter-table-with-recursive-process-utility-calls-wip.patch'
on the master(e788e849addd56007a0e75f3b5514f294a0f3bca). And
when I test the cases, I find it works well on 'alter table t1 add column
f2 int not null, alter column f2 add generated always as identity' case,
but it
Robert Haas writes:
> From my point of view, the DDL code doesn't do a great job separating
> parsing/parse analysis from optimization/execution. The ALTER TABLE
> stuff is actually pretty good in this regard.
Meh. I think a pretty fair characterization of the bug(s) I'm trying to
fix is "we
On Wed, May 29, 2019 at 5:52 PM Tom Lane wrote:
> Hm ... I'm not exactly clear on why that would be a superior solution.
> It would imply that standalone CREATE INDEX etc would call into the
> ALTER TABLE framework --- how is that not equally a layering violation?
Well, the framework could be
Robert Haas writes:
> On Sun, May 26, 2019 at 6:24 PM Tom Lane wrote:
>> Anybody have thoughts about a different way to approach it?
> I mean, in an ideal world, I think we'd never call back out to
> ProcessUtility() from within AlterTable(). That seems like a pretty
> clear layering
On Sun, May 26, 2019 at 6:24 PM Tom Lane wrote:
> Anybody have thoughts about a different way to approach it?
I mean, in an ideal world, I think we'd never call back out to
ProcessUtility() from within AlterTable(). That seems like a pretty
clear layering violation. I assume the reason we've
We've had numerous bug reports complaining about the fact that ALTER TABLE
generates subsidiary commands that get executed unconditionally, even
if they should be discarded due to an IF NOT EXISTS or other condition;
see e.g. #14827, #15180, #15670, #15710. In [1] I speculated about
fixing this
23 matches
Mail list logo
