From: Euler Taveira de Oliveira <euler@timbira.com>
To: Greg Smith <greg@2ndquadrant.com>
Cc: pgsql-performance@postgresql.org <pgsql-performance@postgresql.org>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: pgbench to the MAXINT
Date: Tue, 11 Jan 2011 18:34:17 -0300
Message-ID: <4D2CCCD9.802@timbira.com> (raw)
In-Reply-To: <4D2AC273.3010709@2ndquadrant.com>
References: <4D27C4E5.5000609@2ndquadrant.com>
<4D2A965A.3000603@timbira.com>
<4D2AC273.3010709@2ndquadrant.com>
Em 10-01-2011 05:25, Greg Smith escreveu:
> Euler Taveira de Oliveira wrote:
>> Em 07-01-2011 22:59, Greg Smith escreveu:
>>> setrandom: invalid maximum number -2147467296
>>>
>> It is failing at atoi() circa pgbench.c:1036. But it just the first
>> one. There are some variables and constants that need to be converted
>> to int64 and some functions that must speak 64-bit such as getrand().
>> Are you working on a patch?
>
> http://archives.postgresql.org/pgsql-hackers/2010-01/msg02868.php
> http://archives.postgresql.org/message-id/4C326F46.4050801@2ndquadrant.com
>
Greg, I just improved your patch. I tried to work around the problems pointed
out in the above threads. Also, I want to raise some points:
(i) If we want to support and scale factor greater than 21474 we have to
convert some columns to bigint; it will change the test. From the portability
point it is a pity but as we have never supported it I'm not too worried about
it. Why? Because it will use bigint columns only if the scale factor is
greater than 21474. Is it a problem? I don't think so because generally people
compare tests with the same scale factor.
(ii) From the performance perspective, we need to test if the modifications
don't impact performance. I don't create another code path for 64-bit
modifications (it is too ugly) and I'm afraid some modifications affect the
32-bit performance. I'm in a position to test it though because I don't have a
big machine ATM. Greg, could you lead these tests?
(iii) I decided to copy scanint8() (called strtoint64 there) from backend
(Robert suggestion [1]) because Tom pointed out that strtoll() has portability
issues. I replaced atoi() with strtoint64() but didn't do any performance tests.
Comments?
[1] http://archives.postgresql.org/pgsql-hackers/2010-07/msg00173.php
--
Euler Taveira de Oliveira
http://www.timbira.com/Attachments:pgbench-110111.diff (text/x-patch, 11.6K) downloaddiff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.cindex 7c2ca6e..e9eb720 100644
*** a/contrib/pgbench/pgbench.c
--- b/contrib/pgbench/pgbench.c
***************
*** 60,65 ****
--- 60,67 ----
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endif
+ #define MAX_RANDOM_VALUE64 INT64_MAX+
/*
* Multi-platform pthread implementations
*/
*************** usage(const char *progname)
*** 364,378 ****
progname, progname);
}
/* random number generator: uniform distribution from min to max inclusive */
! static int
! getrand(int min, int max)
{
/*
* Odd coding is so that min and max have approximately the same chance of
* being selected as do numbers between them.
*/
! return min + (int) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0));
}
/* call PQexec() and exit() on failure */
--- 366,451 ----
progname, progname);
}
+ /*+ * strtoint64 -- convert a string to 64-bit integer+ *+ * this function is a modified version of scanint8() from+ * src/backend/utils/adt/int8.c.+ *+ * XXX should it have a return value?+ *+ */+ static int64+ strtoint64(const char *str)+ {+ const char *ptr = str;+ int64 result = 0;+ int sign = 1;+ + /*+ * Do our own scan, rather than relying on sscanf which might be broken+ * for long long.+ */+ + /* skip leading spaces */+ while (*ptr && isspace((unsigned char) *ptr))+ ptr++;+ + /* handle sign */+ if (*ptr == '-')+ {+ ptr++;+ + /*+ * Do an explicit check for INT64_MIN. Ugly though this is, it's+ * cleaner than trying to get the loop below to handle it portably.+ */+ if (strncmp(ptr, "9223372036854775808", 19) == 0)+ {+ result = -INT64CONST(0x7fffffffffffffff) - 1;+ ptr += 19;+ goto gotdigits;+ }+ sign = -1;+ }+ else if (*ptr == '+')+ ptr++;+ + /* require at least one digit */+ if (!isdigit((unsigned char) *ptr))+ fprintf(stderr, "invalid input syntax for integer: \"%s\"\n", str);+ + /* process digits */+ while (*ptr && isdigit((unsigned char) *ptr))+ {+ int64 tmp = result * 10 + (*ptr++ - '0');+ + if ((tmp / 10) != result) /* overflow? */+ fprintf(stderr, "value \"%s\" is out of range for type bigint\n", str);+ result = tmp;+ }+ + gotdigits:+ + /* allow trailing whitespace, but not other trailing chars */+ while (*ptr != '\0' && isspace((unsigned char) *ptr))+ ptr++;+ + if (*ptr != '\0')+ fprintf(stderr, "invalid input syntax for integer: \"%s\"\n", str);+ + return ((sign < 0) ? -result : result);+ }+
/* random number generator: uniform distribution from min to max inclusive */
! static int64
! getrand(int64 min, int64 max)
{
/*
* Odd coding is so that min and max have approximately the same chance of
* being selected as do numbers between them.
*/
! return min + (int64) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE64 + 1.0));
}
/* call PQexec() and exit() on failure */
*************** top:
*** 887,893 ****
if (commands[st->state] == NULL)
{
st->state = 0;
! st->use_file = getrand(0, num_files - 1);
commands = sql_files[st->use_file];
}
}
--- 960,966 ----
if (commands[st->state] == NULL)
{
st->state = 0;
! st->use_file = (int) getrand(0, num_files - 1);
commands = sql_files[st->use_file];
}
}
*************** top:
*** 1007,1013 ****
if (pg_strcasecmp(argv[0], "setrandom") == 0)
{
char *var;
! int min,
max;
char res[64];
--- 1080,1086 ----
if (pg_strcasecmp(argv[0], "setrandom") == 0)
{
char *var;
! int64 min,
max;
char res[64];
*************** top:
*** 1019,1033 ****
st->ecnt++;
return true;
}
! min = atoi(var);
}
else
! min = atoi(argv[2]);
#ifdef NOT_USED
if (min < 0)
{
! fprintf(stderr, "%s: invalid minimum number %d\n", argv[0], min);
st->ecnt++;
return;
}
--- 1092,1106 ----
st->ecnt++;
return true;
}
! min = strtoint64(var);
}
else
! min = strtoint64(argv[2]);
#ifdef NOT_USED
if (min < 0)
{
! fprintf(stderr, "%s: invalid minimum number " INT64_FORMAT "\n", argv[0], min);
st->ecnt++;
return;
}
*************** top:
*** 1041,1062 ****
st->ecnt++;
return true;
}
! max = atoi(var);
}
else
! max = atoi(argv[3]);
! if (max < min || max > MAX_RANDOM_VALUE)
{
! fprintf(stderr, "%s: invalid maximum number %d\n", argv[0], max);
st->ecnt++;
return true;
}
#ifdef DEBUG
! printf("min: %d max: %d random: %d\n", min, max, getrand(min, max));
#endif
! snprintf(res, sizeof(res), "%d", getrand(min, max));
if (!putVariable(st, argv[0], argv[1], res))
{
--- 1114,1135 ----
st->ecnt++;
return true;
}
! max = strtoint64(var);
}
else
! max = strtoint64(argv[3]);
! if (max < min || max > MAX_RANDOM_VALUE64)
{
! fprintf(stderr, "%s: invalid maximum number " INT64_FORMAT "\n", argv[0], max);
st->ecnt++;
return true;
}
#ifdef DEBUG
! printf("min: " INT64_FORMAT " max: " INT64_FORMAT " random: " INT64_FORMAT "\n", min, max, getrand(min, max));
#endif
! snprintf(res, sizeof(res), INT64_FORMAT, getrand(min, max));
if (!putVariable(st, argv[0], argv[1], res))
{
*************** top:
*** 1069,1075 ****
else if (pg_strcasecmp(argv[0], "set") == 0)
{
char *var;
! int ope1,
ope2;
char res[64];
--- 1142,1148 ----
else if (pg_strcasecmp(argv[0], "set") == 0)
{
char *var;
! int64 ope1,
ope2;
char res[64];
*************** top:
*** 1081,1093 ****
st->ecnt++;
return true;
}
! ope1 = atoi(var);
}
else
! ope1 = atoi(argv[2]);
if (argc < 5)
! snprintf(res, sizeof(res), "%d", ope1);
else
{
if (*argv[4] == ':')
--- 1154,1166 ----
st->ecnt++;
return true;
}
! ope1 = strtoint64(var);
}
else
! ope1 = strtoint64(argv[2]);
if (argc < 5)
! snprintf(res, sizeof(res), INT64_FORMAT, ope1);
else
{
if (*argv[4] == ':')
*************** top:
*** 1098,1114 ****
st->ecnt++;
return true;
}
! ope2 = atoi(var);
}
else
! ope2 = atoi(argv[4]);
if (strcmp(argv[3], "+") == 0)
! snprintf(res, sizeof(res), "%d", ope1 + ope2);
else if (strcmp(argv[3], "-") == 0)
! snprintf(res, sizeof(res), "%d", ope1 - ope2);
else if (strcmp(argv[3], "*") == 0)
! snprintf(res, sizeof(res), "%d", ope1 * ope2);
else if (strcmp(argv[3], "/") == 0)
{
if (ope2 == 0)
--- 1171,1187 ----
st->ecnt++;
return true;
}
! ope2 = strtoint64(var);
}
else
! ope2 = strtoint64(argv[4]);
if (strcmp(argv[3], "+") == 0)
! snprintf(res, sizeof(res), INT64_FORMAT, ope1 + ope2);
else if (strcmp(argv[3], "-") == 0)
! snprintf(res, sizeof(res), INT64_FORMAT, ope1 - ope2);
else if (strcmp(argv[3], "*") == 0)
! snprintf(res, sizeof(res), INT64_FORMAT, ope1 * ope2);
else if (strcmp(argv[3], "/") == 0)
{
if (ope2 == 0)
*************** top:
*** 1117,1123 ****
st->ecnt++;
return true;
}
! snprintf(res, sizeof(res), "%d", ope1 / ope2);
}
else
{
--- 1190,1196 ----
st->ecnt++;
return true;
}
! snprintf(res, sizeof(res), INT64_FORMAT, ope1 / ope2);
}
else
{
*************** init(void)
*** 1239,1247 ****
"drop table if exists pgbench_tellers",
"create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=%d)",
"drop table if exists pgbench_accounts",
! "create table pgbench_accounts(aid int not null,bid int,abalance int,filler char(84)) with (fillfactor=%d)",
"drop table if exists pgbench_history",
! "create table pgbench_history(tid int,bid int,aid int,delta int,mtime timestamp,filler char(22))"
};
static char *DDLAFTERs[] = {
"alter table pgbench_branches add primary key (bid)",
--- 1312,1320 ----
"drop table if exists pgbench_tellers",
"create table pgbench_tellers(tid int not null,bid int,tbalance int,filler char(84)) with (fillfactor=%d)",
"drop table if exists pgbench_accounts",
! "create table pgbench_accounts(aid %s not null,bid int,abalance int,filler char(84)) with (fillfactor=%d)",
"drop table if exists pgbench_history",
! "create table pgbench_history(tid int,bid int,aid %s,delta int,mtime timestamp,filler char(22))"
};
static char *DDLAFTERs[] = {
"alter table pgbench_branches add primary key (bid)",
*************** init(void)
*** 1253,1258 ****
--- 1326,1332 ----
PGresult *res;
char sql[256];
int i;
+ int64 k;
if ((con = doConnect()) == NULL)
exit(1);
*************** init(void)
*** 1263,1270 ****
* set fillfactor for branches, tellers and accounts tables
*/
if ((strstr(DDLs[i], "create table pgbench_branches") == DDLs[i]) ||
! (strstr(DDLs[i], "create table pgbench_tellers") == DDLs[i]) ||
! (strstr(DDLs[i], "create table pgbench_accounts") == DDLs[i]))
{
char ddl_stmt[128];
--- 1337,1343 ----
* set fillfactor for branches, tellers and accounts tables
*/
if ((strstr(DDLs[i], "create table pgbench_branches") == DDLs[i]) ||
! (strstr(DDLs[i], "create table pgbench_tellers") == DDLs[i]))
{
char ddl_stmt[128];
*************** init(void)
*** 1272,1277 ****
--- 1345,1382 ----
executeStatement(con, ddl_stmt);
continue;
}
+ else if (strstr(DDLs[i], "create table pgbench_accounts") == DDLs[i])+ {+ char ddl_stmt[128];+ + /*+ * consider using bigint columns in cases where scale factor is+ * bigger than INT_MAX / naccounts. For small scale factors, we+ * still use int columns for compatibility reasons.+ */+ if ((naccounts * scale) > INT64CONST(0x7fffffff))+ snprintf(ddl_stmt, 128, DDLs[i], "bigint", fillfactor);+ else+ snprintf(ddl_stmt, 128, DDLs[i], "int", fillfactor);+ executeStatement(con, ddl_stmt);+ continue;+ }+ else if (strstr(DDLs[i], "create table pgbench_history") == DDLs[i])+ {+ char ddl_stmt[128];+ + /*+ * consider using bigint columns in cases where scale factor is+ * bigger than INT_MAX / naccounts. For small scale factors, we+ * still use int columns for compatibility reasons.+ */+ if ((naccounts * scale) > INT64CONST(0x7fffffff))+ snprintf(ddl_stmt, 128, DDLs[i], "bigint");+ else+ snprintf(ddl_stmt, 128, DDLs[i], "int");+ executeStatement(con, ddl_stmt);+ continue;+ }
else
executeStatement(con, DDLs[i]);
}
*************** init(void)
*** 1309,1319 ****
}
PQclear(res);
! for (i = 0; i < naccounts * scale; i++)
{
! int j = i + 1;
! snprintf(sql, 256, "%d\t%d\t%d\t\n", j, i / naccounts + 1, 0);
if (PQputline(con, sql))
{
fprintf(stderr, "PQputline failed\n");
--- 1414,1424 ----
}
PQclear(res);
! for (k = 0; k < naccounts * scale; k++)
{
! int64 j = k + 1;
! snprintf(sql, 256, INT64_FORMAT "\t" INT64_FORMAT "\t%d\t\n", j, k / naccounts + 1, 0);
if (PQputline(con, sql))
{
fprintf(stderr, "PQputline failed\n");
*************** init(void)
*** 1321,1327 ****
}
if (j % 10000 == 0)
! fprintf(stderr, "%d tuples done.\n", j);
}
if (PQputline(con, "\\.\n"))
{
--- 1426,1432 ----
}
if (j % 10000 == 0)
! fprintf(stderr, INT64_FORMAT " tuples done.\n", j);
}
if (PQputline(con, "\\.\n"))
{
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-performance@postgresql.org
Cc: euler@timbira.com, greg@2ndquadrant.com, pgsql-hackers@postgresql.org
Subject: Re: pgbench to the MAXINT
In-Reply-To: <4D2CCCD9.802@timbira.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox