On Thu, Mar 19, 2015 at 6:49 PM, Bruce Momjian <br...@momjian.us> wrote:
> On Thu, Mar 19, 2015 at 06:05:52PM -0700, David G. Johnston wrote: > > On Thu, Mar 19, 2015 at 5:18 PM, Bruce Momjian <br...@momjian.us> wrote: > > > > There are other places later in the docs where we explain all the quote* > > functions and show examples of query construction using string > > concatenation, but I am not sure how we can remove those. > > > > > > > > Can you be more specific? > > Yes. You can see the output of the attached patch here: > > > http://momjian.us/tmp/pgsql/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN > > Notice: > > EXECUTE 'UPDATE tbl SET ' > || quote_ident(colname) > || ' = ' > || quote_nullable(newvalue) > || ' WHERE key = ' > || quote_nullable(keyvalue); > > and > > EXECUTE 'UPDATE tbl SET ' > || quote_ident(colname) > || ' = $$' > || newvalue > || '$$ WHERE key = ' > || quote_literal(keyvalue); > > It is making a point about nulls and stuff. There are later queries > that use format(). > I thought maybe you meant those but your specific mention of " There are other places later in the docs" confused me since you made changes before and after that specific section. Those examples need to be somewhere and it doesn't seem like a undesireable enough setup that major reconstructive surgery is warranted to try and move them elsewhere. > > On a related note: > > > > "If you are dealing with values that might be null, you should usually > use > > quote_nullable in place of quote_literal." > > > > Its unclear why, aside from semantic uncleanliness, someone would use > > quote_literal given its identical behavior for non-null values and > inferior > > behavior which passed NULL. The function table for the two could maybe > be more > > clear since quote_nullable(NULL) returns a string representation of NULL > > without any quotes while quote_literal(NULL) returns an actual NULL that > > ultimately poisons the string concatenation that these functions are > used with. > > > > <reads some more> > > > > The differences between the actual null and the string NULL are strictly > in > > capitalization - which is not consistent even within the table. > concat_ws > > states "NULL arguments are ignored" and so represents actual null with > all-caps > > which is string NULL in the quote_* descriptions. Having read 40.5.4 and > > example 40-1 the difference is clear and obvious so maybe what is in the > table > > is sufficient for this topic. > > > > I would suggest adding a comment to quote_ident and quote_nullable that > > corresponding format codes are %I and %L. Obviously there is no "quote_" > > function to correspond with %S. There is likewise nor corresponding > format > > code for quote_literal since quote_nullable is superior in every way > (that I > > can tell at least). > > OK, I have added that tip --- good suggestion. Patch attached. > > I was actually referring to chapter 9 http://www.postgresql.org/docs/9.4/interactive/functions-string.html The table definitions of the quote_* function should have a comment about their equivalency to format %I and %L Also, in 9.4.1 (format -> type) would be the most obvious place for the equivalency of the format %I and %L to quote_* IMO too much is trying to be done within example 40-1 (for instance, the quote_literal/nullable explanation should be moved elsewhere); and while these are mainly useful with dynamic SQL it still behooves us to put the definition stuff in the structural area and then use the example for comprehension and clarification regarding best practices (i.e., format for %I but USING for literals - though I know some would say we should necessarily express those kinds of opinions in the docs...). That said, it is not as bad as I may seem to be making it out to be and aside from wanting to put and obvious reference to format directly next to the quote_* functions is more style that content. The desire for the linkage is strong though because we want someone who naturally would use string concatenation and the quote_* functions to be made aware of, and convinced to use (they will thank us for this), the format() function instead. David J.
