Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
On 21/03/2008, Tom Lane <[EMAIL PROTECTED]> wrote: > "Brendan Jurd" <[EMAIL PROTECTED]> 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
"Brendan Jurd" <[EMAIL PROTECTED]> 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
"Brendan Jurd" <[EMAIL PROTECTED]> 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
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 <[EMAIL PROTECTED]> wrote: > On Nov 30, 2007 9:09 PM, Gregory Stark <[EMAIL PROTECTED]> 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? > -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your Subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.org&extra=pgsql-patches
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
On Nov 30, 2007 9:09 PM, Gregory Stark <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
"Brendan Jurd" <[EMAIL PROTECTED]> 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! ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [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 *** ./doc/src/sgml/syntax.sgml.orig Fri Nov 30 19:31:29 2007 --- ./doc/src/sgml/syntax.sgml Fri Nov 30 19:32:11 2007 *** *** 1497,1503 array value from values for its member elements. A simple array constructor consists of the key word ARRAY, a left square bracket ! [, one or more expressions (separated by commas) for the array element values, and finally a right square bracket ]. For example: --- 1497,1503 array value from values for its member elements. A simple array constructor consists of the key word ARRAY, a left square bracket ! [, a list of expressions (separated by commas) for the array element values, and finally a right square bracket ]. For example: *** *** 1507,1515 {1,2,7} (1 row) ! The array element type is the common type of the member expressions, ! determined using the same rules as for UNION or ! CASE constructs (see ). --- 1507,1516 {1,2,7} (1 row) ! If the array is not explictly cast to a particular type, the array element ! type is the common type of the member expressions, determined using the ! same rules as for UNION or CASE constructs (see ! ). *** *** 1554,1559 --- 1555,1573 +You can construct an empty array, but since it's impossible to have an array +with no type, you must explictly cast your empty array to the desired type. For example: + + SELECT ARRAY[]::int[]; + int4 + -- + {} + (1 row) + +For more on casting, see . + + + It is also possible to construct an array from the results of a subquery. In this form, the array constructor is written with the key word ARRAY followed by a parenthesized (not *** ./src/backend/nodes/copyfuncs.c.origFri Nov 30 19:29:16 2007 --- ./src/backend/nodes/copyfuncs.c Fri Nov 30 19:32:11 2007 *** *** 1704,1709 --- 1704,1719 return newnode; } + static A_ArrayExpr * + _copyA_ArrayExpr(A_ArrayExpr *from) + { + A_ArrayExpr *newnode = makeNode(A_ArrayExpr); + + COPY_NODE_FIELD(elements); + + return newnode; + } + static ResTarget * _copyResTarget(ResTarget *from) { *** *** 3538,3543 --- 3548,3556 case T_A_ArrayExpr: retval = _copyA_ArrayExpr(from); break; + case T_A_ArrayExpr: + retval = _copyA_ArrayExpr(from); + break; case T_ResTarget: retval = _copyResTarget(from); break; *** ./src/backend/nodes/outfuncs.c.orig Fri Nov 30 19:29:16 2007 --- ./src/backend/nodes/outfuncs.c Fri Nov 30 19:32:11 2007 *** *** 1978,1983 --- 1978,1991 } static void + _outA_ArrayExpr(StringInfo str, A_ArrayExpr *node) + { + WRITE_NODE_TYPE("A_ARRAYEXPR"); + + WRITE_NODE_FIELD(elements); + } + + static void _outRes