Re: ECPG patch N+1, fix auto-prepare

From: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
To: Michael Meskes <meskes(at)postgresql(dot)org>
Cc: pgsql-hackers(at)postgresql(dot)org, Hans-Juergen Schoenig <hs(at)cybertec(dot)at>
Subject: Re: ECPG patch N+1, fix auto-prepare
Date: 2009-12-16 10:54:41
Message-ID: 4B28BC71.60905@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael Meskes írta:
>> OK, here's another approach. output_statement()'s interface
>> is kept as the original, and not this function decides which
>>
>
> I still think this could be solved more easily.
>

Thanks very much for committing it.

But I don't understand your change. My code was:

=====================================
ecpg.addons:
ECPG: stmtDeleteStmt block
ECPG: stmtInsertStmt block
ECPG: stmtSelectStmt block
ECPG: stmtUpdateStmt block
{ output_statement($1, 1, auto_prepare ? ECPGst_prepnormal :
ECPGst_normal); }
=====================================
output.c:
static char *ecpg_statement_type_name[] = {
"ECPGst_normal",
"ECPGst_execute",
"ECPGst_exec_immediate",
"ECPGst_prepnormal"
};

void
output_statement(char *stmt, int whenever_mode, enum ECPG_statement_type st)
{

fprintf(yyout, "{ ECPGdo(__LINE__, %d, %d, %s, %d, ", compat,
force_indicator, connection ? connection : "NULL", questionmarks);
if (st == ECPGst_normal || st == ECPGst_prepnormal)
{
fprintf(yyout, "%s, \"", ecpg_statement_type_name[st]);
output_escaped_str(stmt, false);
fputs("\", ", yyout);
}
else
fprintf(yyout, "%s, %s, ", ecpg_statement_type_name[st],
stmt);
=====================================

So the ECPGst_normal vs. prepnormal is decided at the caller
and output_statement() is simplified a bit vs the original.

Your code is:

=====================================
ecpg.addons:
ECPG: stmtDeleteStmt block
ECPG: stmtInsertStmt block
ECPG: stmtSelectStmt block
ECPG: stmtUpdateStmt block
{ output_statement($1, 1, ECPGst_prepnormal); }
=====================================
output.c:
static char *ecpg_statement_type_name[] = {
"ECPGst_normal",
"ECPGst_execute",
"ECPGst_exec_immediate",
"ECPGst_prepnormal"
};

void
output_statement(char *stmt, int whenever_mode, enum ECPG_statement_type st)
{
fprintf(yyout, "{ ECPGdo(__LINE__, %d, %d, %s, %d, ", compat,
force_indicator, connection ? connection : "NULL", questionmarks);
if (st == ECPGst_execute || st == ECPGst_exec_immediate)
{
fprintf(yyout, "%s, %s, ", ecpg_statement_type_name[st],
stmt);
}
else
{
if (st == ECPGst_prepnormal && auto_prepare)
fputs("ECPGst_prepnormal, \"", yyout);
else
fputs("ECPGst_normal, \"", yyout);

output_escaped_str(stmt, false);
fputs("\", ", yyout);
}
=====================================

Your code in ecpg.addons calls output_statement()
unconditionally with ECPGst_prepnormal and
output_statement() decides what to do with the
"auto_prepare" global variable. Your code doesn't
seem more readable than mine, but does the same
with the currently existing callers.

>> value it uses. I also introduced
>> static char *ecpg_statement_type_name[]
>> for the textual names of the ECPGst_* symbols to keep the
>> preprocessed code readable, and minimize the impact on the
>> regression tests. So output_statement() always emits
>> ECPGst_* symbols in the preprocessed code instead of
>> ECPGst_normal/prepnormal and numeric value for the
>> other two cases. This way only 7 regression tests' source
>> has changed instead of 45... There are less
>> 1 -> ECPGst_execute and
>> 2 -> ECPGst_exec_immediate
>> changes than
>> ECPGst_normal -> 0
>> changes would have been if I chose emitting the numeric value.
>>
>> Is it acceptable?
>>
>
> Yes sure.
>
> I changed some small parts of your patch (see above) and will commit in a few
> minutes. Just running regression tests.
>

Okay, I have to rebase my SQLDA and DESCRIBE
patches again, since the regression tests' results may
have changed beause of this patch. I will post them soon.

Again, thanks for committing it.

Best regards,
Zoltán Böszörményi

--
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2009-12-16 11:13:56 Re: Hot Standby and prepared transactions
Previous Message Florian Weimer 2009-12-16 10:54:28 Re: Update on true serializable techniques in MVCC