Thread: [PATCH] Improve function toast_delete_external (src/backend/access/table/toast_helper.c)

Hi,

I think this change can improve this particular function by avoiding touching value if not needed.
Test if not isnull is cheaper than test TupleDescAttr is -1.

best regards,

Ranier Vilela
Attachment
Hi,

On Wed, Feb 09, 2022 at 07:56:35AM -0300, Ranier Vilela wrote:
> 
> I think this change can improve this particular function by avoiding
> touching value if not needed.
> Test if not isnull is cheaper than test TupleDescAttr is -1.

It looks sensible.



Hi,

I've applied your patch. I think it's reasonable.
but  IMHO It looks more complicated to read because of many conditions
in if statement.
so what about just moving up if-statement?

Thanks.
Dong Wook Lee

2022년 2월 9일 (수) 오후 7:56, Ranier Vilela <ranier.vf@gmail.com>님이 작성:
>
> Hi,
>
> I think this change can improve this particular function by avoiding touching value if not needed.
> Test if not isnull is cheaper than test TupleDescAttr is -1.
>
> best regards,
>
> Ranier Vilela

Attachment
Oops, I realized that I made a mistake in my patch and now I have corrected it.

2022년 2월 9일 (수) 오후 11:33, Dong Wook Lee <sh95119@gmail.com>님이 작성:
>
> Hi,
>
> I've applied your patch. I think it's reasonable.
> but  IMHO It looks more complicated to read because of many conditions
> in if statement.
> so what about just moving up if-statement?
>
> Thanks.
> Dong Wook Lee
>
> 2022년 2월 9일 (수) 오후 7:56, Ranier Vilela <ranier.vf@gmail.com>님이 작성:
> >
> > Hi,
> >
> > I think this change can improve this particular function by avoiding touching value if not needed.
> > Test if not isnull is cheaper than test TupleDescAttr is -1.
> >
> > best regards,
> >
> > Ranier Vilela

Attachment
Our style is that we group all declarations in a block to appear at the
top.  We don't mix declarations and statements.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/



Yes, now I understand it.
Thank you for letting me know about that.

Thanks.

2022년 2월 10일 (목) 00:39, Ranier Vilela <ranier.vf@gmail.com>님이 작성:

Em qua., 9 de fev. de 2022 às 11:34, Dong Wook Lee <sh95119@gmail.com> escreveu:
Hi,

I've applied your patch. I think it's reasonable.
but  IMHO It looks more complicated to read because of many conditions
in if statement.
so what about just moving up if-statement?

No.
Your version is worse than HEAD, please don't.

1. Do not declare variables after statement.
Although Postgres accepts C99, this is not acceptable, for a number of problems, already discussed.

2. Still slow unnecessarily, because still test TupleDescAttr(tupleDesc, i)->attlen,
which is much more onerous than test isnull.

3. We don't write Baby C code.
C has short break expressions, use-it!

My version is the right thing, here.

regards,
Ranier Vilela
Em qua., 9 de fev. de 2022 às 20:10, Dong Wook Lee <sh95119@gmail.com> escreveu:
Yes, now I understand it.
Thank you for letting me know about that.
You are welcome.

Best regards,
Ranier Vilela