On Wed, Dec 16, 2015 at 3:07 PM, Fabien COELHO <coe...@cri.ensmp.fr> wrote: > >> On Wed, Dec 16, 2015 at 6:10 AM, Robert Haas <robertmh...@gmail.com> >> wrote: >>> >>> It looks fine to me except that I think we should spell out "param" as >>> "parameter" throughout, instead of abbreviating. >> >> >> Fine for me. I have updated the first patch as attached (still looking >> at the second). > > > This doc update threshold -> parameter & sgml and C code looks ok to me.
So I am having a look at the second patch... + double constants such as <literal>3.14156</>, You meant perhaps 3.14159 :) + <entry><literal><function>max(<replaceable>i</>, <replaceable>...</>)</></></> + <entry>integer</> Such function declarations are better with optional arguments listed within brackets. + <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? Both concepts are linked, and for now as I can see the allocation of a \set variable is actually always an integer. 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? 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... 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. 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. 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. 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. Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
