Hello Michael,
Thanks for your remarks.
+ double constants such as <literal>3.14156</>,
You meant perhaps 3.14159 :)
Indeed!
+ <entry><literal><function>max(<replaceable>i</>,
<replaceable>...</>)</></></>
+ <entry>integer</>
Such function declarations are better with optional arguments listed
within brackets.
Why not. I did it that way because this is the standard C syntax for
varargs.
+ <row>
+ <entry><literal><function>double(<replaceable>i</>)</></></>
+ <entry>double</>
+ <entry>cast to double</>
+ <entry><literal>double(5432)</></>
+ <entry><literal>5432.0</></>
+ </row>
+ <row>
+ <entry><literal><function>int(<replaceable>x</>)</></></>
+ <entry>integer</>
+ <entry>cast to int</>
+ <entry><literal>int(5.4 + 3.8)</></>
+ <entry><literal>9</></>
+ </row>
If there are cast functions, why doesn't this patch introduces the
concept of the data type for a variable defined in a script?
Because this would be a pain and this is really useless as far as pgbench
scripts are concerned, it really just needs integers.
Both concepts are linked, and for now as I can see the allocation of a
\set variable is actually always an integer.
Yes.
In consequence, sqrt behavior is a bit surprising, for example this script:
\set aid sqrt(2.0)
SELECT :aid;
returns that:
SELECT 1;
Shouldn't a user expect 1.414 instead? Fabien, am I missing a trick?
There is no trick. There are only integer variables in pgbench, so 1.4 is
rounded to 1 by the assignment.
It seems to me that this patch would gain in clarity by focusing a bit
more on the infrastructure first and remove a couple of things that
are not that mandatory for now...
This is more or less what I did in the beginning, and then someone said
"please show a complete infra with various examples to show that the infra
is really extensible". Now you say the reverse. This is *tiring* and is
not a good use of the little time I can give.
So the following things are not necessary as of now:
- cast functions from/to int/double, because a result variable of a
\set does not include the idea that a result variable can be something
else than an integer. At least no options is given to the user to be
able to make a direct use of a double value.
- functions that return a double number: sqrt, pi
- min and max have value because they allow a user to specify the
expression once as a variable instead of writing it perhaps multiple
times in SQL, still is it enough to justify having them as a set of
functions available by default? I am not really sure either.
AFAICR this is because I was *ASKED* to show an infra which would deal
with various cases: varargs, double/int functions/operators, overloading,
so I added some example in each category, hence the current list of
functions in the patch.
Really, I like this patch, and I think that it is great to be able to
use a set of functions and methods within a pgbench script because
this clearly can bring more clarity for a developer, still it seems
that this patch is trying to do too many things at the same time:
1) Add infrastructure to support function calls and refactor the
existing functions to use it. This is the FUNCTION stuff in the
expression scanner.
2) Add the concept of double return type, this could be an extension
of \set with a data type, or a new command like \setdouble. This
should include as well the casting routines I guess. This is the
DOUBLE stuff in the expression scanner.
3) Introduce new functions, as needed.
Yep. I started with (1), and was *ASKED* to do the others.
Adding double variables in pretty useless in my opinion, and potentially
lead to issues in various places, so I'm not in a hurry to do that.
1) and 2) seem worthwhile to me. 3) may depend on the use cases we'd
like to have.. sqrt has for example value if a variable can be set
double as a double type.
Sure. This is just an example of a double function so that if someone
wants to add another one they can just update where it make sense. Maybe
log/exp would make more sense for pgbench.
In conclusion, for this CF the patch doing the documentation fixes is
"Ready for committer", the second patch introducing the function
infrastructure should focus more on its core structure and should be
marked as "Returned with feedback". Opinions are welcome.
My opinion is that to do and unddo work because of random thoughts on the
list is tiring.
What I can do is:
(1) fix the doc and bugs if any, obviously.
(2a) remove double stuff, just keep integer functions.
I would rather keep min/max, though.
(2b) keep the patch with both int & double as is.
What I will *NOT* do is to add double variables without a convincing use
case.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
