From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: extend pgbench expressions with functions |
Date: | 2016-03-09 08:09:27 |
Message-ID: | alpine.DEB.2.10.1603090822170.25393@sto |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello Robert,
Here is a v35 b & c.
> This is not acceptable:
>
> + /* guess double type (n for "inf",
> "-inf" and "nan") */
> + if (strchr(var, '.') != NULL ||
> strchr(var, 'n') != NULL)
> + {
> + double dv;
> + sscanf(var, "%lf", &dv);
> + setDoubleValue(retval, dv);
> + }
> + else
> + setIntValue(retval, strtoint64(var));
>
> That totally breaks the error handling which someone carefully established here.
I'm not sure what is "not acceptable" as it "totally breaks the error
handling" in the above code.
I assumed that you want to check that sscanf can read what sprintf
generated when handling "\set". I'd guess that libc would be broken if it
was the case. I've made pgbench check that it is not.
If it was something else, you have to spell it out for me.
> Extra space. [...] Another extra space.
Indeed.
> - int nargs = 0;
> - int64 iargs[MAX_FARGS];
> - PgBenchExprLink *l = args;
> + int nargs = 0;
> + PgBenchValue vargs[MAX_FARGS];
> + PgBenchExprLink *l = args;
>
> Completely unnecessary reindentation of the first and third lines.
It just looked better to me.
> + setIntValue(retval, i < 0? -i: i);
>
> Not project style.
Indeed.
> Please fix the whitespace damage git diff --check complains about,
"git diff -check" does not seem to complain on the full v35-b.
> and check for other places where you haven't followed project style.
I've found some more instances of "& ref".
--
Fabien.
Attachment | Content-Type | Size |
---|---|---|
pgbench-funcs-35-b.patch | text/x-diff | 35.2 KB |
pgbench-funcs-35-c.patch | text/x-diff | 8.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Etsuro Fujita | 2016-03-09 08:20:26 | Re: Minor documentation tweak to GetForeignPlan documentation |
Previous Message | pokurev | 2016-03-09 07:54:27 | Re: [PROPOSAL] VACUUM Progress Checker. |