On 2018/01/27 1:31, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Fri, Jan 26, 2018 at 11:07 AM, Stephen Frost <sfrost@snowman.net> wrote:
>>> I've already had two people mention that it'd be neat to have PG support
>>> it, so I don't believe it'd go unused. As for if we should force people
>>> to use quotes, my vote would be no because we don't require that for
>>> other usage of true/false in the parser and I don't see any reason why
>>> this should be different.
>
>> OK. Let's wait a bit and see if anyone else wants to weigh in.
>
> I dunno, this patch seems quite bizarre to me. IIUC, it results in
> TRUE/FALSE behaving differently in a partbound_datum than they do
> anywhere else in the grammar, to wit that they're effectively just
> another spelling for the undecorated literals 't' and 'f', without
> anything indicating that they're boolean. That seems wrong from a
> data typing standpoint. And even if it's really true that we'd
> rather lose type information for partbound literals, a naive observer
> would probably think that these should decay to the strings "true"
> and "false" not "t" and "f" (cf. opt_boolean_or_string).
Partition bound literals as captured gram.y don't have any type
information attached. They're carried over in a A_Const to
transformPartitionBoundValue() and coerced to the target partition key
type there. Note that each of the the partition bound literal datums
received from gram.y is castNode(A_Const)'d in parse_utilcmds.c.
I agree that it's better to simply makeStringConst("true"/"false") for
TRUE_P and FALSE_P, instead of makingBoolAConst(true/false).
> I've not paid any attention to this thread up to now, so maybe there's
> a rational explanation for this choice that I missed. But mucking
> with makeBoolAConst like that doesn't seem to me to pass the smell
> test. I'm on board with the stated goal of allowing TRUE/FALSE here,
> but having to contort the grammar output like this suggests that
> there's something wrong in parse analysis of partbound_datum.
Attached updated patch doesn't change anything about makeBoolAConst and
now is just a 2-line change to gram.y.
> PS: the proposed documentation wording is too verbose by half.
> I'd just cut it down to "<literal constant>".
Yeah, I was getting nervous about the lines in syntax synopsis getting
unwieldily long after this change. I changed all of them to use
literal_constant for anything other than special keywords MINVALUE and
MAXVALUE and a paragraph in the description to clarify.
Attached updated patch. Thanks for the comments.
Regards,
Amit
