From: | Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> |
---|---|
To: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Removing savepointLevel from TransactionState |
Date: | 2011-09-29 05:44:19 |
Message-ID: | CABOikdMpyNTDXGs8fQcssYkn-6o8iCzCvR9KQz-=4xS=yH8iww@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 29, 2011 at 10:52 AM, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> wrote:
> On Thu, Sep 29, 2011 at 1:11 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> writes:
>> > I noticed that the savepointLevel member of TransactionStateData struct
>> > is
>> > initialized to 0 from TopTransactionStateData, and never incremented or
>> > decremented afterwards.
>>
>> > Since this is a file-local struct I think we can simply get rid of all
>> > usages of this without any risk.
>>
>> ISTM you have detected a bug, not just dead code that should be removed.
>> Surely those tests that throw error on savepointLevel change were
>> meant to do something important?
>
> That's surprising for dead-code removal!
>
> Not sure which failures you're pointing to. `make check` before and after
> the patch on master says 'All 126 tests passed.'
>
I think Tom is talking about the test in the code and not a regression
test itself. Looking at the archives and git log, it seems though that
we never completed this feature and the code was always dead since its
inception.
http://www.mail-archive.com/pgsql-hackers(at)postgresql(dot)org/msg48321.html
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2011-09-29 08:20:33 | Re: [REVIEW] pg_last_xact_insert_timestamp |
Previous Message | Gurjeet Singh | 2011-09-29 05:22:46 | Re: Removing savepointLevel from TransactionState |