Thread: Re: [GENERAL] Empty arrays with ARRAY[]
As discussed on -hackers, this patch allows the construction of an
empty array if an explicit cast to an array type is given (as in,
ARRAY[]::int[]).
postgres=# select array[]::int[];
array
-------
{}
postgres=# select array[];
ERROR: no target type for empty array
HINT: Empty arrays must be explictly cast to the desired array type,
e.g. ARRAY[]::int[]
A few notes on the implementation:
* The syntax now allows an ARRAY constructor with an empty expression
list (array_expr_list may be empty).
* I've added a new parsenode for arrays, A_ArrayExpr (previously the
parser would create ArrayExpr primnodes).
* transformArrayExpr() now takes two extra arguments, a type oid and
a typmod. When transforming a typecast which casts an A_ArrayExpr to
an array type, transformExpr passes these type details down to
transformArrayExpr, and skips the typecast.
* transformArrayExpr() behaves slightly differently when passed type
information. The overall type of the array is set to the given type,
and all elements are explictly coerced to the equivalent element type.
If it was not passed a type, then the behaviour is as previous; the
function looks for a common type among the elements, and coerces them
to that type. The overall type of the array is derived from the
common element type.
The patch is very invasive (at least compared to any of my previous
patches), but so far I haven't managed to find any broken behaviour.
All regression tests pass, and the regression tests for arrays seem to
be quite comprehensive. I did add a couple of new tests for the empty
array behaviours, but the rest I've left alone.
I look forward to your comments -- although given the length of the
8.4 patch review queue, that will probably be an exercise in extreme
patience!
Major thanks go out to Tom for all his guidance on -hackers while I
developed the patch.
Regards,
BJ
Attachment
"Brendan Jurd" <direvus@gmail.com> writes: > The patch is very invasive (at least compared to any of my previous > patches), but so far I haven't managed to find any broken behaviour. I'm sorry to suggest anything at this point, but... would it be less invasive if instead of requiring the immediate cast you created a special case in the array code to allow a placeholder object for "empty array of unknown type". The only operation which would be allowed on it would be to cast it to some specific array type. That way things like UPDATE foo SET col = array[]; INSERT INTO foo (col) VALUES (array[]); could be allowed if they could be contrived to introduce an assignment cast. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services!
On Nov 30, 2007 9:09 PM, Gregory Stark <stark@enterprisedb.com> wrote: > I'm sorry to suggest anything at this point, but... would it be less invasive > if instead of requiring the immediate cast you created a special case in the > array code to allow a placeholder object for "empty array of unknown type". > The only operation which would be allowed on it would be to cast it to some > specific array type. > > That way things like > > UPDATE foo SET col = array[]; > INSERT INTO foo (col) VALUES (array[]); > > could be allowed if they could be contrived to introduce an assignment cast. Hi Gregory. Not sure it would be less invasive, but I do like the outcome of being able to create an empty array pending assignment. In addition to your examples, it might also make it possible to do things like this in plpgsql DECLARE a text[] := array[]; Whereas my patch requires you to write a text[]: =array[]::text[]; ... which seems pretty stupid. So, I like your idea a lot from a usability point of view. But I really, really hate it from a "just spent half a week on this patch" point of view =/ Any suggestions about how you would enforce the "only allow casts to array types" restriction on the empty array? Cheers BJ
A quick recap: I submitted a patch for empty ARRAY[] syntax back in November, and as far as I can see it never made it to the patches list. Gregory suggested a different way of approaching the problem (quoted below), but nobody commented further about how it might be made to work. I'd like to RFC again on Gregory's idea, and if that doesn't bear any fruit I'd like to submit the patch as-is for review. Regards, BJ On 01/12/2007, Brendan Jurd <direvus@gmail.com> wrote: > On Nov 30, 2007 9:09 PM, Gregory Stark <stark@enterprisedb.com> wrote: > > I'm sorry to suggest anything at this point, but... would it be less invasive > > if instead of requiring the immediate cast you created a special case in the > > array code to allow a placeholder object for "empty array of unknown type". > > The only operation which would be allowed on it would be to cast it to some > > specific array type. > > > > That way things like > > > > UPDATE foo SET col = array[]; > > INSERT INTO foo (col) VALUES (array[]); > > > > could be allowed if they could be contrived to introduce an assignment cast. > > Not sure it would be less invasive, but I do like the outcome of being > able to create an empty array pending assignment. In addition to your > examples, it might also make it possible to do things like this in > plpgsql > > DECLARE > a text[] := array[]; > > Whereas my patch requires you to write > > a text[]: =array[]::text[]; > > ... which seems pretty stupid. > ... > Any suggestions about how you would enforce the "only allow casts to > array types" restriction on the empty array? >
"Brendan Jurd" <direvus@gmail.com> writes:
> A quick recap: I submitted a patch for empty ARRAY[] syntax back in
> November, and as far as I can see it never made it to the patches
> list. Gregory suggested a different way of approaching the problem
> (quoted below), but nobody commented further about how it might be
> made to work.
> I'd like to RFC again on Gregory's idea, and if that doesn't bear any
> fruit I'd like to submit the patch as-is for review.
Greg's idea is basically to invent array-of-UNKNOWN as a genuine
datatype, which as I stated way back when seems fairly dangerous
to me. UNKNOWN is already a pretty slippery animal, and I don't
know what cast paths we might open up by doing that. I think
the require-a-cast solution is a lot less likely to result in
unforeseen side-effects.
>> Whereas my patch requires you to write
>> a text[]: =array[]::text[];
>> ... which seems pretty stupid.
In practice you'd write
DECLARE
a text[] := '{}';
which is even shorter, so I don't find this convincing.
regards, tom lane
"Brendan Jurd" <direvus@gmail.com> writes:
> As discussed on -hackers, this patch allows the construction of an
> empty array if an explicit cast to an array type is given (as in,
> ARRAY[]::int[]).
Applied with minor fixes; mostly, ensuring that the cast action would
propagate down to sub-arrays, as in
regression=# select array[[1],[2.2]]::int[];
array
-----------
{{1},{2}}
(1 row)
I was interested to realize that this fix validates the decision to
pass down the type information on-the-fly during transformExpr recursion.
It would have been a lot more painful to do it if we'd taken the A_Const
approach.
I didn't do anything about removing A_Const's typename field, but I'm
thinking that would be a good cleanup patch.
regards, tom lane
On 21/03/2008, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Brendan Jurd" <direvus@gmail.com> writes: > > > As discussed on -hackers, this patch allows the construction of an > > empty array if an explicit cast to an array type is given (as in, > > ARRAY[]::int[]). > > > Applied with minor fixes; mostly, ensuring that the cast action would > propagate down to sub-arrays, as in Great, thanks Tom. > I was interested to realize that this fix validates the decision to > pass down the type information on-the-fly during transformExpr recursion. > It would have been a lot more painful to do it if we'd taken the A_Const > approach. > Indeed. > I didn't do anything about removing A_Const's typename field, but I'm > thinking that would be a good cleanup patch. > I'd be happy to take this on. My day job is pretty busy at the moment but I should be able to submit something in a week or so. Cheers, BJ