Re: Boolean partitions syntax

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