Re: [HACKERS] Arrays of domains
On 09/29/2017 01:10 PM, Tom Lane wrote: > Andrew Dunstanwrites: >> On 09/28/2017 05:44 PM, Tom Lane wrote: >>> Assuming that that's going to happen for v11, I'm inclined to leave the >>> optimization problem until the dust settles around CaseTestExpr. >>> Does anyone feel that this can't be committed before that's addressed? >> I'm Ok with it as long as we don't forget to revisit this. > I decided to go ahead and build a quick optimization for this case, > as per the attached patch that applies on top of what we previously > discussed. It brings us back to almost par with HEAD: > > HEADPatch + 04.patch > > Q15453.235 ms 5440.876 ms 5407.965 ms > Q29340.670 ms 10191.194 ms9407.093 ms > Q319078.457 ms18967.279 ms19050.392 ms > Q448196.338 ms58547.531 ms48696.809 ms Nice. > > Unless Andres feels this is too ugly to live, I'm inclined to commit > the patch with this addition. If we don't get around to revisiting > CaseTestExpr, I think this is OK, and if we do, this will make sure > that we consider this case in the revisit. > > It's probably also worth pointing out that this test case is intentionally > chosen to be about the worst possible case for the patch. A less-trivial > coercion function would make the overhead proportionally less meaningful. > There's also the point that the old code sometimes applies two layers of > array coercion rather than one. As an example, coercing int4[] to > varchar(10)[] will do that. If I replace "x::int8[]" with > "x::varchar(10)[]" in Q2 and Q4 in this test, I get > > HEADPatch (+04) > > Q246929.728 ms20646.003 ms > Q4386200.286 ms 155917.572 ms > > Yeah, testing the worst case was the idea. This improvement in the non-worst case is pretty good. +1 for going ahead. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
Andrew Dunstanwrites: > On 09/28/2017 05:44 PM, Tom Lane wrote: >> Assuming that that's going to happen for v11, I'm inclined to leave the >> optimization problem until the dust settles around CaseTestExpr. >> Does anyone feel that this can't be committed before that's addressed? > I'm Ok with it as long as we don't forget to revisit this. I decided to go ahead and build a quick optimization for this case, as per the attached patch that applies on top of what we previously discussed. It brings us back to almost par with HEAD: HEADPatch + 04.patch Q1 5453.235 ms 5440.876 ms 5407.965 ms Q2 9340.670 ms 10191.194 ms9407.093 ms Q3 19078.457 ms18967.279 ms19050.392 ms Q4 48196.338 ms58547.531 ms48696.809 ms Unless Andres feels this is too ugly to live, I'm inclined to commit the patch with this addition. If we don't get around to revisiting CaseTestExpr, I think this is OK, and if we do, this will make sure that we consider this case in the revisit. It's probably also worth pointing out that this test case is intentionally chosen to be about the worst possible case for the patch. A less-trivial coercion function would make the overhead proportionally less meaningful. There's also the point that the old code sometimes applies two layers of array coercion rather than one. As an example, coercing int4[] to varchar(10)[] will do that. If I replace "x::int8[]" with "x::varchar(10)[]" in Q2 and Q4 in this test, I get HEADPatch (+04) Q2 46929.728 ms20646.003 ms Q4 386200.286 ms 155917.572 ms regards, tom lane diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index e0a8998..c5e97ef 100644 *** a/src/backend/executor/execExprInterp.c --- b/src/backend/executor/execExprInterp.c *** *** 34,43 * * For very simple instructions the overhead of the full interpreter * "startup", as minimal as it is, is noticeable. Therefore ! * ExecReadyInterpretedExpr will choose to implement simple scalar Var ! * and Const expressions using special fast-path routines (ExecJust*). ! * Benchmarking shows anything more complex than those may as well use the ! * "full interpreter". * * Complex or uncommon instructions are not implemented in-line in * ExecInterpExpr(), rather we call out to a helper function appearing later --- 34,41 * * For very simple instructions the overhead of the full interpreter * "startup", as minimal as it is, is noticeable. Therefore ! * ExecReadyInterpretedExpr will choose to implement certain simple ! * opcode patterns using special fast-path routines (ExecJust*). * * Complex or uncommon instructions are not implemented in-line in * ExecInterpExpr(), rather we call out to a helper function appearing later *** static Datum ExecJustConst(ExprState *st *** 149,154 --- 147,153 static Datum ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull); + static Datum ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull); /* *** ExecReadyInterpretedExpr(ExprState *stat *** 184,193 /* * Select fast-path evalfuncs for very simple expressions. "Starting up" ! * the full interpreter is a measurable overhead for these. Plain Vars ! * and Const seem to be the only ones where the intrinsic cost is small ! * enough that the overhead of ExecInterpExpr matters. For more complex ! * expressions it's cheaper to use ExecInterpExpr always. */ if (state->steps_len == 3) { --- 183,190 /* * Select fast-path evalfuncs for very simple expressions. "Starting up" ! * the full interpreter is a measurable overhead for these, and these ! * patterns occur often enough to be worth optimizing. */ if (state->steps_len == 3) { *** ExecReadyInterpretedExpr(ExprState *stat *** 230,235 --- 227,239 state->evalfunc = ExecJustAssignScanVar; return; } + else if (step0 == EEOP_CASE_TESTVAL && + step1 == EEOP_FUNCEXPR_STRICT && + state->steps[0].d.casetest.value) + { + state->evalfunc = ExecJustApplyFuncToCase; + return; + } } else if (state->steps_len == 2 && state->steps[0].opcode == EEOP_CONST) *** ExecJustAssignScanVar(ExprState *state, *** 1811,1816 --- 1815,1857 return 0; } + /* Evaluate CASE_TESTVAL and apply a strict function to it */ + static Datum + ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull) + { + ExprEvalStep *op = >steps[0]; + FunctionCallInfo fcinfo; + bool *argnull; + int argno; +
Re: [HACKERS] Arrays of domains
On 09/28/2017 05:44 PM, Tom Lane wrote: > > I get these query timings in a non-cassert build: > > HEADPatch > > Q15453.235 ms 5440.876 ms > Q29340.670 ms 10191.194 ms > Q319078.457 ms18967.279 ms > Q448196.338 ms58547.531 ms > > [ analysis elided] > > Assuming that that's going to happen for v11, I'm inclined to leave the > optimization problem until the dust settles around CaseTestExpr. > Does anyone feel that this can't be committed before that's addressed? > > I'm Ok with it as long as we don't forget to revisit this. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
Andrew Dunstanwrites: > On 09/28/2017 01:11 PM, Tom Lane wrote: >>> I wonder if we need to do any benchmarking to assure ourselves that the >>> changes to ArrayCoerceExpr don't have a significant performance impact? >> That would likely be a good idea, though I'm not very sure what or >> how to benchmark. > Some case where we only expect the current code to produce a single > ArrayCoerceExpr, I guess. say doing text[] -> int[] ? I spent some time looking into this. I settled on int4[] -> int8[] as the appropriate case to test, because int48() is about as cheap a cast function as we have. Q1 is the base case without a cast: select count(x) from (select array[i,i,i,i,i,i,i,i,i,i] as x from generate_series(1,1000) i) ss; Q2 is same with a cast added: select count(x::int8[]) from (select array[i,i,i,i,i,i,i,i,i,i] as x from generate_series(1,1000) i) ss; Q3 and Q4 are the same thing, but testing 100-element instead of 10-element arrays: select count(x) from (select array[ i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i ] as x from generate_series(1,1000) i) ss; select count(x::int8[]) from (select array[ i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i, i,i,i,i,i,i,i,i,i,i ] as x from generate_series(1,1000) i) ss; I get these query timings in a non-cassert build: HEADPatch Q1 5453.235 ms 5440.876 ms Q2 9340.670 ms 10191.194 ms Q3 19078.457 ms18967.279 ms Q4 48196.338 ms58547.531 ms (Timings are reproducible to a few percent.) So that's a bit disappointing; the per-element overhead is clearly noticeably more than before. However, poking into it with "perf" gives some grounds for optimism; Q4's hotspots with the patch are Children Self Samples Command Shared Object Symbol + 33.44%33.35% 81314 postmaster postgres [.] ExecInterpExpr + 21.88%21.83% 53223 postmaster postgres [.] array_map + 15.19%15.15% 36944 postmaster postgres [.] CopyArrayEls + 14.63%14.60% 35585 postmaster postgres [.] ArrayCastAndSet +6.07% 6.06% 14765 postmaster postgres [.] construct_md_array +1.80% 1.79% 4370 postmaster postgres [.] palloc0 +0.77% 0.77% 1883 postmaster postgres [.] AllocSetAlloc +0.75% 0.74% 1815 postmaster postgres [.] int48 +0.52% 0.52% 1276 postmaster postgres [.] advance_aggregates Surely we could get the amount of time spent in ExecInterpExpr down. One idea is to make a dedicated evalfunc for the case where the expression is just EEOP_CASE_TESTVAL + EEOP_FUNCEXPR[_STRICT], similar to the existing fast-path routines (ExecJust*). I've not actually tried to do that, but a reasonable guess is that it'd about halve that overhead, putting this case back on a par with the HEAD code. Also, I'd imagine that Andres' planned work on JIT-compiled expressions would put this back on par with HEAD, if not better, for installations using that. Also I believe that Andres has plans to revamp the CaseTestExpr mechanism, which might shave a few cycles as well. Right now there's an extra copy of each array datum + isnull value, because array_map sticks those into one pair of variables and then the EEOP_CASE_TESTVAL opcode just moves them somewhere else. It's reasonable to hope that we could redesign that so that array_map sticks the values straight into where they're needed, and then we need only the FUNCEXPR opcode, which'd be a great candidate for an ExecJust* fast-path. Assuming that that's going to happen for v11, I'm inclined to leave the optimization problem until the dust settles around CaseTestExpr. Does anyone feel that this can't be committed before that's addressed? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
On 09/28/2017 01:11 PM, Tom Lane wrote: > >> I wonder if we need to do any benchmarking to assure ourselves that the >> changes to ArrayCoerceExpr don't have a significant performance impact? > That would likely be a good idea, though I'm not very sure what or > how to benchmark. > > Some case where we only expect the current code to produce a single ArrayCoerceExpr, I guess. say doing text[] -> int[] ? cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
Andrew Dunstanwrites: > On 08/11/2017 01:17 PM, Tom Lane wrote: >> Attached is a patch series that allows us to create arrays of domain >> types. > I've reviewed and tested the updated versions of these patches. The > patches apply but there's an apparent typo in arrayfuncs.c - > DatumGetAnyArray instead of DatumGetAnyArrayP Thanks for reviewing! The DatumGetAnyArrayP thing is another artifact of 4bd199465 --- sorry for missing that. > Some of the line breaking in argument lists for some of the code > affected by these patches is a bit bizarre. It hasn't been made worse by > these patches but it hasn't been made better either. That's especially > true of patch 1. Yeah, perhaps. A lot of these argument lists are long enough that I'm not especially thrilled with the idea of making them one-arg-per-line; that seems like it would consume a lot of vertical space and make it harder to see context in a finite-size window. I think there's been some attempt at grouping the arguments into related groups on single lines, though I concede it's probably not very obvious nor 100% consistent. > I wonder if we need to do any benchmarking to assure ourselves that the > changes to ArrayCoerceExpr don't have a significant performance impact? That would likely be a good idea, though I'm not very sure what or how to benchmark. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
On 08/11/2017 01:17 PM, Tom Lane wrote: > I wrote: >> Probably a better answer is to start supporting arrays over domain >> types. That was left unimplemented in the original domains patch, >> but AFAICS not for any better reason than lack of round tuits. > Attached is a patch series that allows us to create arrays of domain > types. I haven't tested this in great detail, so there might be some > additional corners of the system that need work, but it passes basic > sanity checks. I believe it's independent of the other patch I have > in the commitfest for domains over composites, but I haven't tested > for interactions there either. > > 01-rationalize-coercion-APIs.patch cleans up the APIs of > coerce_to_domain() and some internal functions in parse_coerce.c so that > we consistently pass around a CoercionContext along with CoercionForm. > Previously, we sometimes passed an "isExplicit" boolean flag instead, > which is strictly less information; and coerce_to_domain() didn't even get > that, but instead had to reverse-engineer isExplicit from CoercionForm. > That's contrary to the documentation in primnodes.h that says that > CoercionForm only affects display and not semantics. I don't think this > change fixes any live bugs, but it makes things more consistent. The > main reason for doing it though is that now build_coercion_expression() > receives ccontext, which it needs in order to be able to recursively > invoke coerce_to_target_type(), as required by the next patch. > > 02-reimplement-ArrayCoerceExpr.patch is the core of the patch. It changes > ArrayCoerceExpr so that the node does not directly know any details of > what has to be done to the individual array elements while performing the > array coercion. Instead, the per-element processing is represented by > a sub-expression whose input is a source array element and whose output > is a target array element. This simplifies life in parse_coerce.c, > because it can build that sub-expression by a recursive invocation of > coerce_to_target_type(), and it allows the executor to handle the > per-element processing as a compiled expression instead of hard-wired > code. This is probably about a wash or a small loss performance-wise > for the simplest case where we just need to invoke one coercion function > for each element. However, there are many cases where the existing code > ends up generating two nested ArrayCoerceExprs, one to do the type > conversion and one to apply a typmod (length) coercion function. In the > new code there will be just one ArrayCoerceExpr, saving one deconstruction > and reconstruction of the array. If I hadn't done it like this, adding > domains into the mix could have produced as many as three > ArrayCoerceExprs, where the top one would have only checked domain > constraints; that did not sound nice performance-wise, and it would have > required a lot of code duplication as well. > > Finally, 03-support-arrays-of-domains.patch simply turns on the spigot > by creating an array type in DefineDomain(), and adds some test cases. > Given the new method of handling ArrayCoerceExpr, everything Just Works. > > I'll add this to the next commitfest. > > > I've reviewed and tested the updated versions of these patches. The patches apply but there's an apparent typo in arrayfuncs.c - DatumGetAnyArray instead of DatumGetAnyArrayP Some of the line breaking in argument lists for some of the code affected by these patches is a bit bizarre. It hasn't been made worse by these patches but it hasn't been made better either. That's especially true of patch 1. Patch 1 is fairly straightforward, as is patch 3. Patch 2 is fairly complex, but it still does the one thing stated above - there's just a lot of housekeeping that goes along with that. I couldn't see any obvious problems with the implementation. I wonder if we need to do any benchmarking to assure ourselves that the changes to ArrayCoerceExpr don't have a significant performance impact? Apart from those concerns I think this is ready to be committed. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
I wrote: > Here's a rebased-up-to-HEAD version of this patch set. The only > actual change is removal of a no-longer-needed hunk in pl_exec.c. I see the patch tester is complaining that this broke, due to commit 4bd199465. The fix is trivial (s/GETARG_ANY_ARRAY/GETARG_ANY_ARRAY_P/) but for convenience here's an updated patch set. regards, tom lane diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index 9d75e86..d7db32e 100644 *** a/src/backend/optimizer/prep/preptlist.c --- b/src/backend/optimizer/prep/preptlist.c *** expand_targetlist(List *tlist, int comma *** 306,314 new_expr = coerce_to_domain(new_expr, InvalidOid, -1, atttype, COERCE_IMPLICIT_CAST, -1, - false, false); } else --- 306,314 new_expr = coerce_to_domain(new_expr, InvalidOid, -1, atttype, + COERCION_IMPLICIT, COERCE_IMPLICIT_CAST, -1, false); } else diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index e79ad26..5a241bd 100644 *** a/src/backend/parser/parse_coerce.c --- b/src/backend/parser/parse_coerce.c *** *** 34,48 static Node *coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod, ! CoercionForm cformat, int location, ! bool isExplicit, bool hideInputCoercion); static void hide_coercion_node(Node *node); static Node *build_coercion_expression(Node *node, CoercionPathType pathtype, Oid funcId, Oid targetTypeId, int32 targetTypMod, ! CoercionForm cformat, int location, ! bool isExplicit); static Node *coerce_record_to_complex(ParseState *pstate, Node *node, Oid targetTypeId, CoercionContext ccontext, --- 34,49 static Node *coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod, ! CoercionContext ccontext, CoercionForm cformat, ! int location, ! bool hideInputCoercion); static void hide_coercion_node(Node *node); static Node *build_coercion_expression(Node *node, CoercionPathType pathtype, Oid funcId, Oid targetTypeId, int32 targetTypMod, ! CoercionContext ccontext, CoercionForm cformat, ! int location); static Node *coerce_record_to_complex(ParseState *pstate, Node *node, Oid targetTypeId, CoercionContext ccontext, *** coerce_to_target_type(ParseState *pstate *** 110,117 */ result = coerce_type_typmod(result, targettype, targettypmod, ! cformat, location, ! (cformat != COERCE_IMPLICIT_CAST), (result != expr && !IsA(result, Const))); if (expr != origexpr) --- 111,117 */ result = coerce_type_typmod(result, targettype, targettypmod, ! ccontext, cformat, location, (result != expr && !IsA(result, Const))); if (expr != origexpr) *** coerce_type(ParseState *pstate, Node *no *** 355,361 result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! cformat, location, false, false); ReleaseSysCache(baseType); --- 355,362 result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! ccontext, cformat, location, ! false); ReleaseSysCache(baseType); *** coerce_type(ParseState *pstate, Node *no *** 417,436 result = build_coercion_expression(node, pathtype, funcId, baseTypeId, baseTypeMod, ! cformat, location, ! (cformat != COERCE_IMPLICIT_CAST)); /* * If domain, coerce to the domain type and relabel with domain ! * type ID. We can skip the internal length-coercion step if the ! * selected coercion function was a type-and-length coercion. */ if (targetTypeId != baseTypeId) result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! cformat, location, true, ! exprIsLengthCoercion(result, ! NULL)); } else { --- 418,434 result = build_coercion_expression(node, pathtype, funcId, baseTypeId, baseTypeMod, ! ccontext, cformat, location); /* * If domain, coerce to the domain type and relabel with domain ! * type ID, hiding the previous coercion node. */ if (targetTypeId != baseTypeId) result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! ccontext, cformat, location, ! true); } else { *** coerce_type(ParseState *pstate, Node *no *** 444,450 *
Re: [HACKERS] Arrays of domains
I wrote: > Attached is a patch series that allows us to create arrays of domain > types. Here's a rebased-up-to-HEAD version of this patch set. The only actual change is removal of a no-longer-needed hunk in pl_exec.c. regards, tom lane diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index 9d75e86..d7db32e 100644 *** a/src/backend/optimizer/prep/preptlist.c --- b/src/backend/optimizer/prep/preptlist.c *** expand_targetlist(List *tlist, int comma *** 306,314 new_expr = coerce_to_domain(new_expr, InvalidOid, -1, atttype, COERCE_IMPLICIT_CAST, -1, - false, false); } else --- 306,314 new_expr = coerce_to_domain(new_expr, InvalidOid, -1, atttype, + COERCION_IMPLICIT, COERCE_IMPLICIT_CAST, -1, false); } else diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index e79ad26..5a241bd 100644 *** a/src/backend/parser/parse_coerce.c --- b/src/backend/parser/parse_coerce.c *** *** 34,48 static Node *coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod, ! CoercionForm cformat, int location, ! bool isExplicit, bool hideInputCoercion); static void hide_coercion_node(Node *node); static Node *build_coercion_expression(Node *node, CoercionPathType pathtype, Oid funcId, Oid targetTypeId, int32 targetTypMod, ! CoercionForm cformat, int location, ! bool isExplicit); static Node *coerce_record_to_complex(ParseState *pstate, Node *node, Oid targetTypeId, CoercionContext ccontext, --- 34,49 static Node *coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod, ! CoercionContext ccontext, CoercionForm cformat, ! int location, ! bool hideInputCoercion); static void hide_coercion_node(Node *node); static Node *build_coercion_expression(Node *node, CoercionPathType pathtype, Oid funcId, Oid targetTypeId, int32 targetTypMod, ! CoercionContext ccontext, CoercionForm cformat, ! int location); static Node *coerce_record_to_complex(ParseState *pstate, Node *node, Oid targetTypeId, CoercionContext ccontext, *** coerce_to_target_type(ParseState *pstate *** 110,117 */ result = coerce_type_typmod(result, targettype, targettypmod, ! cformat, location, ! (cformat != COERCE_IMPLICIT_CAST), (result != expr && !IsA(result, Const))); if (expr != origexpr) --- 111,117 */ result = coerce_type_typmod(result, targettype, targettypmod, ! ccontext, cformat, location, (result != expr && !IsA(result, Const))); if (expr != origexpr) *** coerce_type(ParseState *pstate, Node *no *** 355,361 result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! cformat, location, false, false); ReleaseSysCache(baseType); --- 355,362 result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! ccontext, cformat, location, ! false); ReleaseSysCache(baseType); *** coerce_type(ParseState *pstate, Node *no *** 417,436 result = build_coercion_expression(node, pathtype, funcId, baseTypeId, baseTypeMod, ! cformat, location, ! (cformat != COERCE_IMPLICIT_CAST)); /* * If domain, coerce to the domain type and relabel with domain ! * type ID. We can skip the internal length-coercion step if the ! * selected coercion function was a type-and-length coercion. */ if (targetTypeId != baseTypeId) result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! cformat, location, true, ! exprIsLengthCoercion(result, ! NULL)); } else { --- 418,434 result = build_coercion_expression(node, pathtype, funcId, baseTypeId, baseTypeMod, ! ccontext, cformat, location); /* * If domain, coerce to the domain type and relabel with domain ! * type ID, hiding the previous coercion node. */ if (targetTypeId != baseTypeId) result = coerce_to_domain(result, baseTypeId, baseTypeMod, targetTypeId, ! ccontext, cformat, location, ! true); } else { *** coerce_type(ParseState *pstate, Node *no *** 444,450 * then we won't need a RelabelType node. */ result = coerce_to_domain(node, InvalidOid, -1, targetTypeId, !
Re: [HACKERS] Arrays of domains
I wrote: > Probably a better answer is to start supporting arrays over domain > types. That was left unimplemented in the original domains patch, > but AFAICS not for any better reason than lack of round tuits. Attached is a patch series that allows us to create arrays of domain types. I haven't tested this in great detail, so there might be some additional corners of the system that need work, but it passes basic sanity checks. I believe it's independent of the other patch I have in the commitfest for domains over composites, but I haven't tested for interactions there either. 01-rationalize-coercion-APIs.patch cleans up the APIs of coerce_to_domain() and some internal functions in parse_coerce.c so that we consistently pass around a CoercionContext along with CoercionForm. Previously, we sometimes passed an "isExplicit" boolean flag instead, which is strictly less information; and coerce_to_domain() didn't even get that, but instead had to reverse-engineer isExplicit from CoercionForm. That's contrary to the documentation in primnodes.h that says that CoercionForm only affects display and not semantics. I don't think this change fixes any live bugs, but it makes things more consistent. The main reason for doing it though is that now build_coercion_expression() receives ccontext, which it needs in order to be able to recursively invoke coerce_to_target_type(), as required by the next patch. 02-reimplement-ArrayCoerceExpr.patch is the core of the patch. It changes ArrayCoerceExpr so that the node does not directly know any details of what has to be done to the individual array elements while performing the array coercion. Instead, the per-element processing is represented by a sub-expression whose input is a source array element and whose output is a target array element. This simplifies life in parse_coerce.c, because it can build that sub-expression by a recursive invocation of coerce_to_target_type(), and it allows the executor to handle the per-element processing as a compiled expression instead of hard-wired code. This is probably about a wash or a small loss performance-wise for the simplest case where we just need to invoke one coercion function for each element. However, there are many cases where the existing code ends up generating two nested ArrayCoerceExprs, one to do the type conversion and one to apply a typmod (length) coercion function. In the new code there will be just one ArrayCoerceExpr, saving one deconstruction and reconstruction of the array. If I hadn't done it like this, adding domains into the mix could have produced as many as three ArrayCoerceExprs, where the top one would have only checked domain constraints; that did not sound nice performance-wise, and it would have required a lot of code duplication as well. Finally, 03-support-arrays-of-domains.patch simply turns on the spigot by creating an array type in DefineDomain(), and adds some test cases. Given the new method of handling ArrayCoerceExpr, everything Just Works. I'll add this to the next commitfest. regards, tom lane diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index afc733f..277ca8b 100644 *** a/src/backend/optimizer/prep/preptlist.c --- b/src/backend/optimizer/prep/preptlist.c *** expand_targetlist(List *tlist, int comma *** 306,314 new_expr = coerce_to_domain(new_expr, InvalidOid, -1, atttype, COERCE_IMPLICIT_CAST, -1, - false, false); } else --- 306,314 new_expr = coerce_to_domain(new_expr, InvalidOid, -1, atttype, + COERCION_IMPLICIT, COERCE_IMPLICIT_CAST, -1, false); } else diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index 0bc7dba..c406cea 100644 *** a/src/backend/parser/parse_coerce.c --- b/src/backend/parser/parse_coerce.c *** *** 34,48 static Node *coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod, ! CoercionForm cformat, int location, ! bool isExplicit, bool hideInputCoercion); static void hide_coercion_node(Node *node); static Node *build_coercion_expression(Node *node, CoercionPathType pathtype, Oid funcId, Oid targetTypeId, int32 targetTypMod, ! CoercionForm cformat, int location, ! bool isExplicit); static Node *coerce_record_to_complex(ParseState *pstate, Node *node, Oid targetTypeId, CoercionContext ccontext, --- 34,49 static Node *coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod, ! CoercionContext ccontext, CoercionForm cformat, ! int location, ! bool hideInputCoercion); static void hide_coercion_node(Node *node); static Node
Re: [HACKERS] Arrays of domains
On 07/11/2017 12:44 PM, Tom Lane wrote: > > Can anyone think of a reason not to pursue that? > > I'm all for it. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of domains
On Tue, Jul 11, 2017 at 12:44:33PM -0400, Tom Lane wrote: > Over in > https://www.postgresql.org/message-id/877ezgyn60@metapensiero.it > there's a gripe about array_agg() not working for a domain type. > It fails because we don't create an array type over a domain type, > so the parser can't identify a suitable output type for the polymorphic > aggregate. > > We could imagine tweaking the polymorphic-function resolution rules > so that a domain matched to ANYELEMENT is smashed to its base type, > allowing ANYARRAY to be resolved as the base type's array type. > While that would be a pretty localized fix, it seems like a kluge > to me. > > Probably a better answer is to start supporting arrays over domain > types. That was left unimplemented in the original domains patch, > but AFAICS not for any better reason than lack of round tuits. > I did find an argument here: > https://www.postgresql.org/message-id/3c98f7f6.29fe1...@redhat.com > that the SQL spec forbids domains over arrays, but that's the opposite > case (and a restriction we long since ignored, anyway). > > Can anyone think of a reason not to pursue that? +1 for pursuing it. When operations just compose, users get a more fun experience. Best, David. -- David Fetterhttp://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Arrays of domains
Over in https://www.postgresql.org/message-id/877ezgyn60@metapensiero.it there's a gripe about array_agg() not working for a domain type. It fails because we don't create an array type over a domain type, so the parser can't identify a suitable output type for the polymorphic aggregate. We could imagine tweaking the polymorphic-function resolution rules so that a domain matched to ANYELEMENT is smashed to its base type, allowing ANYARRAY to be resolved as the base type's array type. While that would be a pretty localized fix, it seems like a kluge to me. Probably a better answer is to start supporting arrays over domain types. That was left unimplemented in the original domains patch, but AFAICS not for any better reason than lack of round tuits. I did find an argument here: https://www.postgresql.org/message-id/3c98f7f6.29fe1...@redhat.com that the SQL spec forbids domains over arrays, but that's the opposite case (and a restriction we long since ignored, anyway). Can anyone think of a reason not to pursue that? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Arrays of Records in PL/Perl
Hackers, Given this script: BEGIN; CREATE TYPE foo AS ( this int, that int ); CREATE OR REPLACE FUNCTION dump(foo[]) returns text language plperlu AS $$ use Data::Dumper; Dumper shift; $$; CREATE OR REPLACE FUNCTION dump(foo) returns text language plperlu AS $$ use Data::Dumper; Dumper shift; $$; select dump(row(3, 5)::foo); select dump(ARRAY[row(3, 5)::foo]); ROLLBACK; The output is: dump -- $VAR1 = { + 'that' = '5',+ 'this' = '3' + }; + (1 row) Time: 0.936 ms dump -- $VAR1 = '{(3,5)}';+ (1 row) That is, if a record is passed to a PL/Perl function, it's correctly converted into a hash. If, however, an array of records are passed, the record are stringified, rather than turned into hashes. This seems inconsistent. Bug? Thanks, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of Records in PL/Perl
On Tue, Jul 12, 2011 at 12:45, David E. Wheeler da...@kineticode.com wrote: Hackers, That is, if a record is passed to a PL/Perl function, it's correctly converted into a hash. If, however, an array of records are passed, the record are stringified, rather than turned into hashes. This seems inconsistent. Bug? All Arrays in 9.0 and lower are strings, regardless of if they are comprised of composite types. Its not so much a bug as a limitation. Alexey Klyukin fixed this for 9.1 :-) [ In 9.1 we could not make them straight perl arrayrefs as we needed to keep the string representation for backwards compatibility. What we did in 9.1 is overload the arrayref and string operations on blessed object so you can treat it as either. ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Arrays of Records in PL/Perl
On Jul 12, 2011, at 12:19 PM, Alex Hunsaker wrote: All Arrays in 9.0 and lower are strings, regardless of if they are comprised of composite types. Its not so much a bug as a limitation. Alexey Klyukin fixed this for 9.1 :-) Oh? dump -- $VAR1 = { + 'that' = '5',+ 'this' = '3' + }; + (1 row) Time: 2.016 ms dump $VAR1 = bless( { + 'array' = [ + { + 'that' = '5',+ 'this' = '3' + } + ],+ 'typeoid' = 16457 + }, 'PostgreSQL::InServer::ARRAY' );+ (1 row) Woo! Thanks! [ In 9.1 we could not make them straight perl arrayrefs as we needed to keep the string representation for backwards compatibility. What we did in 9.1 is overload the arrayref and string operations on blessed object so you can treat it as either. ] Yep, awesome. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from Alex Hunsaker's message of sáb feb 12 04:53:14 -0300 2011: - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Incidentally, I think this bit was lost, no? -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 17, 2011 at 16:18, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Alex Hunsaker's message of sáb feb 12 04:53:14 -0300 2011: - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Incidentally, I think this bit was lost, no? It was, yes. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from Alvaro Herrera's message of mié feb 16 19:54:07 -0300 2011: I cleaned up the patch a bit -- result is v11, attached. I'll give it another look tomorrow and hopefully commit it. Applied. Thanks. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Sat, Feb 12, 2011 at 02:17:12AM +0200, Alexey Klyukin wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). Given that encode_array_literal() encodes an _array_ as a literal, I'd assume encode_type_literal() encodes a _type_ as a literal. I'd suggest encode_typed_literal() as a better name. Tim [just passing though] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from Tim Bunce's message of mié feb 16 14:08:11 -0300 2011: On Sat, Feb 12, 2011 at 02:17:12AM +0200, Alexey Klyukin wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). Given that encode_array_literal() encodes an _array_ as a literal, I'd assume encode_type_literal() encodes a _type_ as a literal. I'd suggest encode_typed_literal() as a better name. FYI I'm looking at this patch (v10), and I'll incorporate Tim's suggestion. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
I cleaned up the patch a bit -- result is v11, attached. I'll give it another look tomorrow and hopefully commit it. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support pg_to_perl_arrays_v11.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Feb 16, 2011 at 12:21, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Tim Bunce's message of mié feb 16 14:08:11 -0300 2011: I'd suggest encode_typed_literal() as a better name. FYI I'm looking at this patch (v10), and I'll incorporate Tim's suggestion. Looks good to me. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 12, 2011, at 9:53 AM, Alex Hunsaker wrote: On Fri, Feb 11, 2011 at 17:17, Alexey Klyukin al...@commandprompt.com wrote: Anyway in playing with this patch a bit more I found another bug return [[]]; would segfault. So find attached a v9 that: - fixes above segfault - made plperl_sv_to_literal vastly simpler (no longer uses SPI and calls regtypin directly) - removed extraneous /para added in v8 in plperl.sgml (my docbook stuff is broken, so I can't build them, hopefully there are no other problems) - we now on the fly (when its requested) make the backwards compatible string for arrays (it was a few lines now that we have plperl_sv_to_literal) - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Anyway barring any more bugs, I think this patch is ready. pg_to_perl_arrays_v9.patch.gz Thank you for finding and fixing the bugs there. I think removing the para was a mistake. I specifically added it to fix the problem I had when building the docs: openjade:plperl.sgml:353:8:E: end tag for PARA omitted, but OMITTAG NO was specified openjade:plperl.sgml:201:2: start tag was here After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. pg_to_perl_arrays_v10.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 7:45 PM, David E. Wheeler wrote: On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Yes. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 9:49 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Yes. Awesom, thanks Alexey Alex! David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 10, 2011, at 11:26 PM, Alexey Klyukin wrote: On Feb 10, 2011, at 9:44 PM, Andrew Dunstan wrote: On 02/10/2011 08:15 AM, Alexey Klyukin wrote: Let me try implementing that as an XS interface to plperl_array_to_datum. Are you intending this as a completion of the current patch or as 9.2 work? If the former you need to send it in real fast. I'd like to extend the current patch, going to post the update by tomorrow. So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). basically a user-level interface to plperl_sv_to_datum, which accepts the perl variable and the type name as a text string and returns the string representation of the input variable according to the output function of the argument type, e..g: do $$ elog(NOTICE, encode_type_literal([[1],[2],[3]], 'int[]'));$$ language plperl; NOTICE: {{1},{2},{3}} I can easily convert the encode_array_literal to just call this function, but not encode_array_constructor, since the latter needs to produce its own string representation, different from the result of the type output function. One option would be traversing through the SV *, duplicating the efforts of plperl_sv_to_datum, but I actually wonder what do we need the encode_array_constructor (and encode_array_literal) functions for ? I guess they were useful to pass array references to SPI, but per my understanding it's possible to use perl array references in SPI functions directly now, so are there any other use cases for these functions, which justify the time to spend on updating them to support arrays of composites (and shouldn't we also provide encode_composite_literal and encode_composite_constructor as well) ? /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. pg_to_perl_arrays_v8.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Fri, Feb 11, 2011 at 17:17, Alexey Klyukin al...@commandprompt.com wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). ... I can easily convert the encode_array_literal to just call this function, but not encode_array_constructor, but I actually wonder what do we need the encode_array_constructor (and encode_array_literal) functions for ? I guess they were useful to pass array references to SPI, I dunno, Ill have to go dig through the archives. but per my understanding it's possible to use perl array references in SPI functions directly now, so are there any other use cases for these functions, which justify the time to spend on updating them to support arrays of composites Probably not, but I dunno if we can just drop them out from people using them... (and shouldn't we also provide encode_composite_literal and encode_composite_constructor as well) ? Well we should not need encode_composite_literal, encode_type_literal() should work for that. I don't see a reason for the _constructor variant so id vote against it unless there is a use case. Anyway in playing with this patch a bit more I found another bug return [[]]; would segfault. So find attached a v9 that: - fixes above segfault - made plperl_sv_to_literal vastly simpler (no longer uses SPI and calls regtypin directly) - removed extraneous /para added in v8 in plperl.sgml (my docbook stuff is broken, so I can't build them, hopefully there are no other problems) - we now on the fly (when its requested) make the backwards compatible string for arrays (it was a few lines now that we have plperl_sv_to_literal) - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Anyway barring any more bugs, I think this patch is ready. pg_to_perl_arrays_v9.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 9, 2011, at 9:28 PM, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 08:24, Alexey Klyukin al...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Damn, right. Each perl hash corresponds to multiple composite types, different by the order of the type elements. Passing the typid sounds like a fair requirement (and if it's missing we could assume that the order of columns in composites doesn't matter to the caller). Let me try implementing that as an XS interface to plperl_array_to_datum. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 02/10/2011 08:15 AM, Alexey Klyukin wrote: On Feb 9, 2011, at 9:28 PM, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 08:24, Alexey Klyukinal...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Damn, right. Each perl hash corresponds to multiple composite types, different by the order of the type elements. Passing the typid sounds like a fair requirement (and if it's missing we could assume that the order of columns in composites doesn't matter to the caller). Let me try implementing that as an XS interface to plperl_array_to_datum. Are you intending this as a completion of the current patch or as 9.2 work? If the former you need to send it in real fast. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 10, 2011, at 9:44 PM, Andrew Dunstan wrote: On 02/10/2011 08:15 AM, Alexey Klyukin wrote: On Feb 9, 2011, at 9:28 PM, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 08:24, Alexey Klyukinal...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Damn, right. Each perl hash corresponds to multiple composite types, different by the order of the type elements. Passing the typid sounds like a fair requirement (and if it's missing we could assume that the order of columns in composites doesn't matter to the caller). Let me try implementing that as an XS interface to plperl_array_to_datum. Are you intending this as a completion of the current patch or as 9.2 work? If the former you need to send it in real fast. I'd like to extend the current patch, going to post the update by tomorrow. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 9, 2011, at 3:44 AM, Alex Hunsaker wrote: So the merge while not exactly trivial was fairly simple. However it would be great if you could give it another look over. Find attached v7 changes include: - rebased against HEAD - fix potential use of uninitialized dims[cur_depth] - took out accidental (broken) hack to try and support composite types in ::encode_array_literal (added in v4 or something) - make_array_ref() now uses plperl_hash_from_datum() for composite types instead of its own hand rolled version - get_perl_array_ref() now grabs the 'array' directly instead of through the magic interface for simplicity - moved added static declarations to the bottom instead of being half on top and half on bottom pg_to_perl_arrays_v7.patch.gz Thank you very much, the new patch applies cleanly and passes all tests on my system. The new get_perl_array_ref seems to be much more clear to me, than the prev. magic call. What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 08, 2011 at 09:40:38AM -0500, Andrew Dunstan wrote: On 02/03/2011 01:20 PM, Andrew Dunstan wrote: Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. do you till have concerns about this, or are you happy for us to move ahead on it? [I'm not really paying close enough attention for you to put much weight on my opinions, but...] I can't see any major issues so I'm happy for you to move ahead. Thanks! Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Feb 9, 2011 at 08:24, Alexey Klyukin al...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Here is a longer perhaps more concrete example: Imagine you have a composite type with two 'columns': = create type foo as (z int, a int); = create or replace function foo_pl(foo[]) returns foo[] as $$ my $arg = shift; $$ language plperl; = select foo_pl('{(1,2), (3,4)}'); In the above $arg looks something like (ignoring the PostgreSQL::InServer::ARRAY object) [{'a'=2, 'z'=1}, {'a'=4, 'z'=3}]. When we call encode_arary_literal() we need to put it back in to composite literal form which is basically (ignoring the array) (column_z, column_a). However without type information we don't know the order of the columns, as the composite is represented as a hash we get kind of stuck. The hack I did sorted the hash keys alphabetically, which worked for the regression tests as they happened to have their composite columns sorted alphabetically. But would break for this example putting $arg-[0]{a} into z and $arg-[0]{z} into a. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 01:20 PM, Andrew Dunstan wrote: - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Normally, onle plperl_ref_from_pg_array will be invoked every time the function is called with an array argument, which would take little time to convert the PostgreSQL internal array representation (not a string) to the perl references, but that's no different from what is already done with composite type arguments, which are converted to perl hash references on every corresponding function call. I'd missed that it was using the internal array representation (obvious in hindsight) but there's still a significant cost in allocating the SVs that won't be used by existing code. Though I agree it's of the same order as for composite types. Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. Tim, do you till have concerns about this, or are you happy for us to move ahead on it? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote: On Thu, Feb 3, 2011 at 18:29, Alex Hunsaker bada...@gmail.com wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. For example: ... On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash: So here is a v6 that does the above. It does so by providing a generic plperl_sv_to_datum() function and converting the various places to use it. One cool thing is you can now use the composite types or arrays passed in (or returned from another spi!) as arguments for spi_exec_prepared(). Before you would have to convert the hashes to strings. It also provides a real plperl_convert_to_pg_array (now plperl_array_to_datum) that can handle composite and other random datatypes. This also means we don't have to convert arrays to a string representation first. (which removes part of the argument for #5 @ http://archives.postgresql.org/pgsql-hackers/2011-02/msg00197.php) I have attached a stand alone version of the above so its easier to look over. The diffstat is fairly small (ignoring added regression tests) 1 files changed, 259 insertions(+), 165 deletions(-) Comments? Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for the first element of the corresponding dimension, but non-NULL for the second one - it would use uninitialized dims[cur_depth] value in comparison (which, although, would probably lead to the desired comparison result). /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin al...@commandprompt.com wrote: On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote: So here is a v6 Comments? Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Well, you can't AV *av = (AV *)SvRV(sv); And the SV * amagic_call returns is already a reference, so the newRV_noinc() would be redundant no? It occurs to me instead of doing the amagic_call we could just call the to_array method directly using perl_call_pv(). That would look more normal and less magic-- thats probably a good thing? Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for the first element of the corresponding dimension, but non-NULL for the second one - it would use uninitialized dims[cur_depth] value in comparison (which, although, would probably lead to the desired comparison result). Good catch, will fix. All of those checks should be outside of the while loop. While Im at it Ill also rebase against HEAD (im sure there will be some conflicts with that other plperl patch that just went in ;)). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 8, 2011 at 10:33, Alex Hunsaker bada...@gmail.com wrote: On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin al...@commandprompt.com wrote: Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Well, you can't AV *av = (AV *)SvRV(sv); And the SV * amagic_call returns is already a reference, so the newRV_noinc() would be redundant no? It occurs to me instead of doing the amagic_call we could just call the to_array method directly using perl_call_pv(). That would look more normal and less magic-- thats probably a good thing? Err, even simpler would be to just access the 'array' member directly out of the hash object. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 8, 2011 at 10:33, Alex Hunsaker bada...@gmail.com wrote: On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin al...@commandprompt.com wrote: On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote: So here is a v6 Comments? Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Err, even simpler would be to just access the 'array' member directly out of the hash object. Done as the above, an added bonus is we no longer have to SvREF_dec() so its even simpler. Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for the first element of the corresponding dimension, but non-NULL for the second one - it would use uninitialized dims[cur_depth] value in comparison (which, although, would probably lead to the desired comparison result). Good catch, will fix. All of those checks should be outside of the while loop. I clearly needed more caffeine in my system when i wrote that. Partly due to the shadowed av variable. I've fixed it by initiating all of the dims to 0. I also renamed the shadowed av var to nav for nested av. While Im at it Ill also rebase against HEAD (im sure there will be some conflicts with that other plperl patch that just went in ;)). So the merge while not exactly trivial was fairly simple. However it would be great if you could give it another look over. Find attached v7 changes include: - rebased against HEAD - fix potential use of uninitialized dims[cur_depth] - took out accidental (broken) hack to try and support composite types in ::encode_array_literal (added in v4 or something) - make_array_ref() now uses plperl_hash_from_datum() for composite types instead of its own hand rolled version - get_perl_array_ref() now grabs the 'array' directly instead of through the magic interface for simplicity - moved added static declarations to the bottom instead of being half on top and half on bottom pg_to_perl_arrays_v7.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 3, 2011 at 18:29, Alex Hunsaker bada...@gmail.com wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. For example: ... On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash: So here is a v6 that does the above. It does so by providing a generic plperl_sv_to_datum() function and converting the various places to use it. One cool thing is you can now use the composite types or arrays passed in (or returned from another spi!) as arguments for spi_exec_prepared(). Before you would have to convert the hashes to strings. It also provides a real plperl_convert_to_pg_array (now plperl_array_to_datum) that can handle composite and other random datatypes. This also means we don't have to convert arrays to a string representation first. (which removes part of the argument for #5 @ http://archives.postgresql.org/pgsql-hackers/2011-02/msg00197.php) I have attached a stand alone version of the above so its easier to look over. The diffstat is fairly small (ignoring added regression tests) 1 files changed, 259 insertions(+), 165 deletions(-) Comments? plperl_uniform_inout.patch.gz Description: GNU Zip compressed data pg_to_perl_arrays_v6.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 08:29 PM, Alex Hunsaker wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukinal...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. This is not unique to the patch, it how nested composites are handled in general. That is if you passed in ROW(ROW()), the first ROW would be a hash while the 2nd row would be a scalar. On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash I think the reason this is so has to do with the history. Composite support came to plperl about the same time (in the 8.0 cycle, IIRC) that we were getting more support for nested composites in Postgres, and they sorta passed like ships in the night. Anyone object to fixing the above as part of this patch? That is making plperl_(build_tuple_result, modify_tuple, return_next, hash_from_tuple) handle array and hash (composite/row) types consistently? And _that_ would be to recurse and turn them from/into perl objects. Thoughts? I have no objection to making the whole thing work recursively, in principle, but will it break legacy code? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Fri, Feb 4, 2011 at 10:29, Andrew Dunstan and...@dunslane.net wrote: Anyone object to fixing the above as part of this patch? That is making plperl_(build_tuple_result, modify_tuple, return_next, hash_from_tuple) handle array and hash (composite/row) types consistently? And _that_ would be to recurse and turn them from/into perl objects. Thoughts? I have no objection to making the whole thing work recursively, in principle, but will it break legacy code? It will certainly change how nested composites are represented on the 'input side'. I would argue its a bug the way it is now and also violates the POLA. I think we should also remain backwards compatible on the output side so you could still return a string: -- how things are currently, manually assigning a composite-literal (would continue to work) = create or replace function trigger_func() returns trigger as $$ $_TD-{'new'}{'nested_composite'} = {'a'='(1,2)'}'; return 'MODIFY'; -- it would also work with perl nested structures (currently broken) = create or replace function trigger_func() returns trigger as $$ $_TD-{'new'}{'nested_composite'} = {'a'={'b'=1, 'c'=2}}; return 'MODIFY'; $$ Or perhaps you are saying we should do something similar with what we now do with arrays? The pseudo array dance that is. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Hi, On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote: I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers. Better late than never :) I'd kind'a hoped that this functionality could be tied into extending PL/Perl to handle named arguments. That way the perl variables corresponding to the named arguments could be given references without breaking any code. Franky I don't see a direct connection between conversion of arrays into array references and supporting named arguments. Could you, please, show some examples of how you hoped the functionality could be extended? Some observations on the current code (based on a quick skim): - I'd like to see the conversion function exposed as a builtin $ref = decode_array_literal({...}); In principal, I think that's not hard to built with the functionality provided by this patch. I see this as an XS function which takes the array string, calls the array_in to convert it to the array datum, and, finally, calls plperl_ref_from_pg_array (provided by the patch) to produce the resulting array reference. - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Normally, onle plperl_ref_from_pg_array will be invoked every time the function is called with an array argument, which would take little time to convert the PostgreSQL internal array representation (not a sting) to the perl references, but that's no different from what is already done with composite type arguments, which are converted to perl hash references on every corresponding function call. - Some of those functions may not use the array at all and some may simply pass it on as an argument to another function. I don't see how it would be good to optimize for the functions that are declared to get the array but in fact do nothing with it. And considering the case of passing an array through to another function, it's currently not possible to call another pl/perl function from an existing one directly, and when passing muti-dimensional arrays to a perl function one would need to convert it to the array references anyway. - Making the conversion lazy would be a big help. Converting it to string is already lazy, and, per the argumens above, I don't see a real benefit of lazy conversion to the perl reference (as comparing to the hurdles of implementing that). /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 3, 2011, at 4:23 AM, Alexey Klyukin wrote: - Making the conversion lazy would be a big help. Converting it to string is already lazy, and, per the argumens above, I don't see a real benefit of lazy conversion to the perl reference (as comparing to the hurdles of implementing that). I agree that we should prefer an actual array as the implementation and lazily provide a string when needed. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 03, 2011 at 02:23:32PM +0200, Alexey Klyukin wrote: Hi, On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote: I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers. Better late than never :) I'd kind'a hoped that this functionality could be tied into extending PL/Perl to handle named arguments. That way the perl variables corresponding to the named arguments could be given references without breaking any code. Franky I don't see a direct connection between conversion of arrays into array references and supporting named arguments. There is no direct connection. I wasn't clear, tied was too strong a word for it. Could you, please, show some examples of how you hoped the functionality could be extended? I wasn't suggesting extending the functionality, just a way of adding the functionality that wouldn't risk impacting existing code. Imagine that PL/Perl could handle named arguments: CREATE FUNCTION join_list( separator text, list array ) AS $$ return join( $separator, @$list ); $$ LANGUAGE plperl; The $list variable, magically created by PL/Perl, could be the array reference created by your code, without altering the contents of @_. Existing code that does the traditional explicit unpacking of @_ wouldn't be affected. So there would be no need for the string overload hack which, although I suggested it, I'm a little uncomfortable with. (The problems with recursion and the need for is_array_ref with hardwired class names are a little troubling. Not to mention the extra overheads in accessing the array.) On the ther hand, a string version of the array would still need to be created for @_. Some observations on the current code (based on a quick skim): - I'd like to see the conversion function exposed as a builtin $ref = decode_array_literal({...}); In principal, I think that's not hard to built with the functionality provided by this patch. I see this as an XS function which takes the array string, calls the array_in to convert it to the array datum, and, finally, calls plperl_ref_from_pg_array (provided by the patch) to produce the resulting array reference. I think that would be useful. - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Normally, onle plperl_ref_from_pg_array will be invoked every time the function is called with an array argument, which would take little time to convert the PostgreSQL internal array representation (not a string) to the perl references, but that's no different from what is already done with composite type arguments, which are converted to perl hash references on every corresponding function call. I'd missed that it was using the internal array representation (obvious in hindsight) but there's still a significant cost in allocating the SVs that won't be used by existing code. Though I agree it's of the same order as for composite types. - Some of those functions may not use the array at all and some may simply pass it on as an argument to another function. I don't see how it would be good to optimize for the functions that are declared to get the array but in fact do nothing with it. And considering the case of passing an array through to another function, it's currently not possible to call another pl/perl function from an existing one directly, and when passing muti-dimensional arrays to a perl function one would need to convert it to the array references anyway. I was thinking of calls to other pl/perl functions via sql. - Making the conversion lazy would be a big help. Converting it to string is already lazy, and, per the argumens above, I don't see a real benefit of lazy conversion to the perl reference (as comparing to the hurdles of implementing that). I (now) agree. Thanks. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 01:01 PM, Tim Bunce wrote: Imagine that PL/Perl could handle named arguments: CREATE FUNCTION join_list( separator text, list array ) AS $$ return join( $separator, @$list ); $$ LANGUAGE plperl; The $list variable, magically created by PL/Perl, could be the array reference created by your code, without altering the contents of @_. I think that's getting way too subtle, and it would probably violate the POLA. If we implement named arguments I would expect $list to be the same as $_[0] - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Normally, onle plperl_ref_from_pg_array will be invoked every time the function is called with an array argument, which would take little time to convert the PostgreSQL internal array representation (not a string) to the perl references, but that's no different from what is already done with composite type arguments, which are converted to perl hash references on every corresponding function call. I'd missed that it was using the internal array representation (obvious in hindsight) but there's still a significant cost in allocating the SVs that won't be used by existing code. Though I agree it's of the same order as for composite types. Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On tor, 2011-02-03 at 18:01 +, Tim Bunce wrote: Imagine that PL/Perl could handle named arguments: CREATE FUNCTION join_list( separator text, list array ) AS $$ return join( $separator, @$list ); $$ LANGUAGE plperl; The $list variable, magically created by PL/Perl, could be the array reference created by your code, without altering the contents of @_. I would find that pretty surprising, since Perl itself doesn't even provide named arguments. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 3, 2011 at 05:23, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote: [...] Some observations on the current code (based on a quick skim): - I'd like to see the conversion function exposed as a builtin $ref = decode_array_literal({...}); In principal, I think that's not hard to built with the functionality provided by this patch. I see this as an XS function which takes the array string, *cough* array string and Oid for the datatype right? :P calls the array_in to convert it to the array datum, and, finally, calls plperl_ref_from_pg_array (provided by the patch) to produce the resulting array reference. - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Sorry, there does seem to be some confusion here. The first version I posted did lazy conversion to a string using encode_array_literal(). Unfortunately that function falls over with row/composite types. I don't see a way to quote those without knowing the datatype, so in interest of having it be 'correct' instead of fast, I made it always decode to an array ref _and_ string in the next version :-(. I see a couple of ways to fix this: 1) Using PTR2IV store the datum pointer in the array pseudo object. This way when used as a string we have the original Datum and can call the correct OutputFunction on demand. It would also let us call plperl_ref_from_pg_array lazily. I have not actually tried it, but it should work. 2) Add encode_composite_literal($hashref, $typeoid) and store the type/Oid in the pseudo object. We could then call that lazily from the perl. 3) Add the column position to the pseudo object and quote appropriately. Simpler than #1 or #2 but not as robust. 4) Maybe there is a way of setting composite columns by column instead of by position? If so we can encode that way. However everything on http://www.postgresql.org/docs/current/interactive/rowtypes.html that has to do with the 'literal' format seems to be positional. 5) Decided its worth the performance hit to always decode to both. ( Note it may not be as big as people think, in the case that you return the array reference we have to convert it to a string anyway ) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. For example: = create type foo as (a int[]); = create type foofoo as (b foo); = create or replace function fa(foofoo[]) returns void as $$ my $foofoo_array = shift; warn ref $foofoo_array-[0] || 'SCALAR'; warn ref $foofoo_array-[0]-{'b'} || 'SCALAR'; $$ language plperl; = select fa(ARRAY[ROW(ROW(ARRAY[1]))]::foofoo[]); WARNING: HASH at line 3. CONTEXT: PL/Perl function fa WARNING: SCALAR at line 4. CONTEXT: PL/Perl function fa Why is foofoo.b a scalar but foofoo is a hash? This is not unique to the patch, it how nested composites are handled in general. That is if you passed in ROW(ROW()), the first ROW would be a hash while the 2nd row would be a scalar. On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash: = create type foo as (a int[]); = create or replace function foo_in(foo) returns foo as $$ shift; $$ language plperl; = create or replace function foo_out() returns foo as $$ return {'a'=[1,2,3]}; $$ language plperl; = -- this works with the patch because we treat the reference as a string = select foo_in('({1,2,3})'::foo); foo_in - ({1,2,3}) = select foo_out(); ERROR: array value must start with { or dimension information CONTEXT: PL/Perl function foo_out STATEMENT: SELECT foo_out(); -- also works as a straight string = create or replace function foo_out_str() returns foo as $$ return {'a'='{1,2,3}'}; $$ language plperl; = select foo_out_str(); foo_out_str - ({1,2,3}) (1 row) Anyone object to fixing the above as part of this patch? That is making plperl_(build_tuple_result, modify_tuple, return_next, hash_from_tuple) handle array and hash (composite/row) types consistently? And _that_ would be to recurse and turn them from/into perl objects. Thoughts? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers. I'd kind'a hoped that this functionality could be tied into extending PL/Perl to handle named arguments. That way the perl variables corresponding to the named arguments could be given references without breaking any code. Some observations on the current code (based on a quick skim): - I'd like to see the conversion function exposed as a builtin $ref = decode_array_literal({...}); - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. - Some of those functions may not use the array at all and some may simply pass it on as an argument to another function. - Making the conversion lazy would be a big help. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. Looks good. Marked as Ready for committer -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 29, 2011, at 12:27 AM, Alex Hunsaker wrote: On Thu, Jan 27, 2011 at 03:38, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 27, 2011, at 9:31 AM, Alex Hunsaker wrote: Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Nice improvement. It passes all the regression tests on my OS X system. I have only a minor suggestion, I think is_array is worth mentioning in the utility functions chapter of the pl/perl documentation, it would be also more clear to use it in regression tests as opposed to manually checking whether the ref is equal to 'PostgreSQL::InServer::ARRAY'. Wait a second... Just who is reviewing who's patch? :P Oh, sorry, got confused along the way :) Both done in the attached. I also renamed is_array() to is_array_ref() for clarity (hopefully). pg_to_perl_arrays_v4.patch.gz Thank you. I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. pg_to_perl_arrays_v5.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 27, 2011 at 03:38, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 27, 2011, at 9:31 AM, Alex Hunsaker wrote: Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Nice improvement. It passes all the regression tests on my OS X system. I have only a minor suggestion, I think is_array is worth mentioning in the utility functions chapter of the pl/perl documentation, it would be also more clear to use it in regression tests as opposed to manually checking whether the ref is equal to 'PostgreSQL::InServer::ARRAY'. Wait a second... Just who is reviewing who's patch? :P Both done in the attached. I also renamed is_array() to is_array_ref() for clarity (hopefully). pg_to_perl_arrays_v4.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Hi, On Jan 27, 2011, at 9:31 AM, Alex Hunsaker wrote: Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Nice improvement. It passes all the regression tests on my OS X system. I have only a minor suggestion, I think is_array is worth mentioning in the utility functions chapter of the pl/perl documentation, it would be also more clear to use it in regression tests as opposed to manually checking whether the ref is equal to 'PostgreSQL::InServer::ARRAY'. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: WARNING: Deep recursion on subroutine main::encode_array_literal at -e line 74 just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), but it still segfaults after I fixed that. Other than that, the approach looks good to me, I'm for eliminating the GUC setting in favor of it. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 26, 2011 at 12:44, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: WARNING: Deep recursion on subroutine main::encode_array_literal at -e line 74 just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), Yeah, I noticed that after I sent it out :(. but it still segfaults after I fixed that. I seem to recall fixing this post email as well... Can you provide the function that broke so I can double check? (Or was it part of the regression test?) Thanks for taking the time to play with it. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 26, 2011, at 10:08 PM, Alex Hunsaker wrote: On Wed, Jan 26, 2011 at 12:44, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: WARNING: Deep recursion on subroutine main::encode_array_literal at -e line 74 just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), Yeah, I noticed that after I sent it out :(. but it still segfaults after I fixed that. I seem to recall fixing this post email as well... Can you provide the function that broke so I can double check? (Or was it part of the regression test?) Sure, it's really simple (and the plperl_array regressions tests exposes this problem as well): CREATE OR REPLACE FUNCTION test_array(INTEGER[]) RETURNS TEXT AS $$ my $array = shift; print $array.\n; $$ LANGUAGE plperl; /A I will look into this problem tomorrow unless you'll be lucky to fix it before that. Thank you for the review and your patch. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 26, 2011 at 13:35, Alexey Klyukin al...@commandprompt.com wrote: On Jan 26, 2011, at 10:08 PM, Alex Hunsaker wrote: (it's obviously reversed comparing with the original one), but it still segfaults after I fixed that. Ahh yep, the reason reversing the check did not fix it is order of operations. I had this fixed, but I had some unrelated changes in my tree. So I manually git add(ed) the plperl files so I could use git diff --cached to make the diff. Then I fixed this issue, but forgot to git-add the changes :(. That explains why make installcheck worked for me, but the diff I made was broken. If you add some parens around ref it should work: if ref($arg) !~ /ARRAY/; btw the next version I plan on posting will check more explicitly: if ref($arg) !~ /^(?:ARRAY|PostgreSQL::InServer::ARRAY)$/; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). Arrays have a reference of 'PostgreSQL::InServer::ARRAY'-- mainly to be consistent with the other PL/Perl packages we have. It also lets you match ref() =~ m/ARRAY$/ to make it a tad easier to figure out if something is an array. The other thing to note is I only applied this to the 'top' array. That is when you have a multidimensional array, its children are 'real' arrays: create or replace function takes_array(text[]) returns void as $$ my $array = shift; elog(NOTICE, ref $array); elog(NOTICE, ref $_) for (@$array); $$ language plperl; select takes_array('{{1}, {2, 3}}'::text[]); NOTICE: PostgreSQL::InServer::ARRAY CONTEXT: PL/Perl function takes_array NOTICE: ARRAY CONTEXT: PL/Perl function takes_array NOTICE: ARRAY CONTEXT: PL/Perl function takes_array We could change that so _all_ arrays have a ref of PostgreSQL::InServer::ARRAY. However I thought it would be nice to easily use built-ins, this way you can just 'cast' away just the top level without having to recurse to children (my @arr = @$array). This also will create a string representation if and when you try to use it as a string. It currently lacks row support in the stringify case (and I left the regression test Alexey added for that failing) so we would need to fix that up if we want to go down this road. Thoughts? Should I polish this a bit more? Or do we like the GUC better? pg_to_perl_arrays_kind_of.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 13, 2011 at 12:06:33AM -0700, Alex Hunsaker wrote: I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. Yep, In-fact if we wanted we could even die (or throw an exception in other language speak :) ) when the string value is demanded. I played with this a little and it is fairly easy to make a variable such that $a is the string representation and $a[0] the first value of the array. The problem is that you can't pass such a variable into a subroutine. I was thinking however, if the parameters if the function have names you can use, then you can make it work. $_[0] would still go the old way, but the named parameters could be the array. == cut == #!/usr/bin/perl -w use strict; no strict 'vars'; package MyClass; sub TIESCALAR { my $class = shift; my $self = shift; return bless $self, $class; } sub FETCH { my $self = shift; return join(,, @$self); } my @a=(1,2); tie $a, MyClass, \@a; print \$a='$a'\n; print \$a[0]='$a[0]'\n; -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ Patriotism is when love of your own people comes first; nationalism, when hate for people other than your own comes first. - Charles de Gaulle signature.asc Description: Digital signature
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 13, 2011 at 01:06, Martijn van Oosterhout klep...@svana.org wrote: On Thu, Jan 13, 2011 at 12:06:33AM -0700, Alex Hunsaker wrote: I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. Yep, In-fact if we wanted we could even die (or throw an exception in other language speak :) ) when the string value is demanded. I played with this a little and it is fairly easy to make a variable such that $a is the string representation and $a[0] the first value of the array. The problem is that you can't pass such a variable into a subroutine. [ snip ] my @a=(1,2); tie $a, MyClass, \@a; print \$a='$a'\n; print \$a[0]='$a[0]'\n; Erm... the reason you can't seem to pass it to any subroutines is its actually 2 variables: $a, @a. When you print $a\n; you are using the tied variable that uses @a; And when you print $a[0]\n; you are accessing the array directly. I think you just had an unfortunate variable name, otherwise strict would have complained appropriately. :) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 13, 2011 at 2:06 AM, Martijn van Oosterhout klep...@svana.org wrote: I played with this a little and it is fairly easy to make a variable such that $a is the string representation and $a[0] the first value of the array. The problem is that you can't pass such a variable into a subroutine. I played with this too: #!/usr/bin/perl -w use strict; package Pg::ArrayArg; use overload ''= \as_s, '@{}' = \as_a; sub new { my $proto = shift; my $class = ref $proto || $proto; bless { string = shift, array = shift }, $class; } sub as_s { shift-{ 'string' }; } sub as_a { shift-{ 'array' }; } package main; my $aa = Pg::ArrayArg-new( '{1,2}', [ 1, 2 ] ); printf ref = %s\n, ref $aa; print string = $aa\n; printf string = %s\n, $aa; printf array index = (%s, %s)\n, $aa-[ 0 ], $aa-[ 1 ]; printf array_ref = %s\n, scalar @$aa; print regexp test = ; if ($aa =~ /^{(.*)}$/) { print looks like array\n; printf join of split = %s\n, join ';', split /,/, $1; } else { print doesn't look like array\n; } Suppose one of these compatibility objects is passed into legacy code as $_[0]. The problem is that 'ref $_[0]' will return 'Pg::ArrayArg' instead of what it used to, '' (empty string). Other than that, I think it performs as people would expect. You could even change 'as_s' to generate the string on the fly as requested instead of generating both representations at instantiation. Just my $0.02. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 13, 2011, at 4:15 PM, Stephen J. Butler wrote: Suppose one of these compatibility objects is passed into legacy code as $_[0]. The problem is that 'ref $_[0]' will return 'Pg::ArrayArg' instead of what it used to, '' (empty string). Other than that, I think it performs as people would expect. Well, frankly, since up to this patch you *never* got an ARRAY reference argument, who would be calling `ref` on it anyway? You could even change 'as_s' to generate the string on the fly as requested instead of generating both representations at instantiation. Yep. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 1:07 AM, David E. Wheeler wrote: On Jan 11, 2011, at 2:25 PM, Alexey Klyukin wrote: Hello, Here's the patch that improves handling of arrays as pl/perl function input arguments, converting postgres arrays of arbitrary dimensions into perl array references. Awesome! I've wanted this for *years*. It includes regression tests and a documentation changes, and it builds and runs successfully on my mac os x and linux boxes. To maintain compatibility with existing pl/perl code a new variable, plperl.convert_array_arguments (better name?), is introduced. Its default value is false, when set to true it triggers the new behavior, i.e. Have you considered instead passing an array-based object with is string overloading designed to return the pg array string format? That would make for nice, transparent compatibility without the need for a GUC. You mean packing both a string representation and a reference to a single SV * value? I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. If there is a strong dislike of the proposed 'compatibility' GUC option - I think I can change the existing patch to incorporate array string representation into the reference-holding SV quite easily. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 4:06 AM, Robert Haas wrote: On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan and...@dunslane.net wrote: On 01/11/2011 07:17 PM, David E. Wheeler wrote: On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote: I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. I think that'd be pretty rare. Possibly it would. But we usually try pretty hard to avoid that sort of breakage. By the same token, I'm not convinced it's a good idea for this behavior to be off by default. Surely many people will altogether fail to notice that it's an option? If we're going to have a backward-compatibility GUC at all, ISTM that you ought to get the good stuff unless you ask for the old way. I think the number of people failing to notice the changes would be the same whenever we set the new or the old behavior by default. I decided to default to the the old behavior since it won't break the existing code as opposed to just hiding the good stuff, although it would slower the adoption of the new behavior. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. If there is a strong dislike of the proposed 'compatibility' GUC option - I think I can change the existing patch to incorporate array string representation into the reference-holding SV quite easily. Andrew's objections have merit. So perhaps just add this patch to the commit fest as is? Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 06:34, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 4:06 AM, Robert Haas wrote: By the same token, I'm not convinced it's a good idea for this behavior to be off by default. Surely many people will altogether fail to notice that it's an option? If we're going to have a backward-compatibility GUC at all, ISTM that you ought to get the good stuff unless you ask for the old way. I think the number of people failing to notice the changes would be the same whenever we set the new or the old behavior by default. I decided to default to the the old behavior since it won't break the existing code as opposed to just hiding the good stuff, although it would slower the adoption of the new behavior. Personally, I think the point of a compatibility GUC is that at some point in the distant future we can get rid of it. If we default to the old behavior thats going to be harder to do. +1 for defaulting to the new behavior. [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 11:22 AM, Alex Hunsaker wrote: Personally, I think the point of a compatibility GUC is that at some point in the distant future we can get rid of it. If we default to the old behavior thats going to be harder to do. +1 for defaulting to the new behavior. +1 [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] Yeah, but the removal of Safe.pm actually *improved* compatibility. This patch, without the GUC, would break it. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 12:22:55PM -0700, Alex Hunsaker wrote: On Wed, Jan 12, 2011 at 06:34, Alexey Klyukin al...@commandprompt.com wrote: On Jan 12, 2011, at 4:06 AM, Robert Haas wrote: By the same token, I'm not convinced it's a good idea for this behavior to be off by default. Surely many people will altogether fail to notice that it's an option? If we're going to have a backward-compatibility GUC at all, ISTM that you ought to get the good stuff unless you ask for the old way. I think the number of people failing to notice the changes would be the same whenever we set the new or the old behavior by default. I decided to default to the the old behavior since it won't break the existing code as opposed to just hiding the good stuff, although it would slower the adoption of the new behavior. Personally, I think the point of a compatibility GUC is that at some point in the distant future we can get rid of it. If we default to the old behavior thats going to be harder to do. +1 for defaulting to the new behavior. [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] +1 for changing the behavior to something sane with loud, specific warnings in the release notes about what will break and how. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from Alex Hunsaker's message of mié ene 12 16:22:55 -0300 2011: [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] I think the main question here is: how loudly is existing code going to break? If the breakage is silent, it's going to be very problematic. If functions fail to run at all, then we can live without the compatibility option. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 11:36 AM, Alvaro Herrera wrote: [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] I think the main question here is: how loudly is existing code going to break? If the breakage is silent, it's going to be very problematic. If functions fail to run at all, then we can live without the compatibility option. I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like ARRAY(0x118ee2a0) and return an empty array, or a NULL. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from David E. Wheeler's message of mié ene 12 16:39:56 -0300 2011: On Jan 12, 2011, at 11:36 AM, Alvaro Herrera wrote: [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] I think the main question here is: how loudly is existing code going to break? If the breakage is silent, it's going to be very problematic. If functions fail to run at all, then we can live without the compatibility option. I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like ARRAY(0x118ee2a0) and return an empty array, or a NULL. I kinda doubt that a function failing in that way would pass any testing. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 11:51 AM, Alvaro Herrera wrote: I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like ARRAY(0x118ee2a0) and return an empty array, or a NULL. I kinda doubt that a function failing in that way would pass any testing. What is this “testing” thing of which you speak? David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. If there is a strong dislike of the proposed 'compatibility' GUC option - I think I can change the existing patch to incorporate array string representation into the reference-holding SV quite easily. Andrew's objections have merit. So perhaps just add this patch to the commit fest as is? Already done :) /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 9:36 PM, Alvaro Herrera wrote: Excerpts from Alex Hunsaker's message of mié ene 12 16:22:55 -0300 2011: [ Id actually vote for _not_ having a compatibility option at all, we change more major things than this IMHO every major release. (And even then some major things in minor releases, for example the removal of Safe.pm) ] I think the main question here is: how loudly is existing code going to break? If the breakage is silent, it's going to be very problematic. If functions fail to run at all, then we can live without the compatibility option. Not really loud. Perl won't even complain when you try to interpret a reference as a string. Since almost everyone votes for making the new behavior a default option I'm inclined to do that change, although I'm against throwing out the compatibility option. There are many other reasons except for PL/Perl for people to upgrade to 9.1, let's not force them to rewrite their Perl code if they were not planning to do that. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from David E. Wheeler's message of mié ene 12 16:55:17 -0300 2011: On Jan 12, 2011, at 11:51 AM, Alvaro Herrera wrote: I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like ARRAY(0x118ee2a0) and return an empty array, or a NULL. I kinda doubt that a function failing in that way would pass any testing. What is this “testing” thing of which you speak? Ha ha ha :-( I wonder if there's a way to overload the string representation of the array so that it throws an error. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Alexey Klyukin al...@commandprompt.com writes: Since almost everyone votes for making the new behavior a default option I'm inclined to do that change, although I'm against throwing out the compatibility option. There are many other reasons except for PL/Perl for people to upgrade to 9.1, let's not force them to rewrite their Perl code if they were not planning to do that. IMO a GUC for this completely sucks, because if you do need to convert your Perl functions, the only way to do it is to have a flag day wherein they all change at once. And what about people writing Perl functions that they'd like to give to other people? If you have to have a flag, the only useful sort of flag is one that can be attached to individual functions. Compare what we did for plpgsql.variable_conflict in 9.0. I don't know how practical that will be in plperl, though. I thought the idea of overloading the string representation to look like the old style was a cute solution. If we don't have anyone at hand who knows how to do that, let's find someone who does. Not break our users' code because we're too lazy to find out how to do it properly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 01/12/2011 04:22 PM, Tom Lane wrote: Alexey Klyukinal...@commandprompt.com writes: Since almost everyone votes for making the new behavior a default option I'm inclined to do that change, although I'm against throwing out the compatibility option. There are many other reasons except for PL/Perl for people to upgrade to 9.1, let's not force them to rewrite their Perl code if they were not planning to do that. IMO a GUC for this completely sucks, because if you do need to convert your Perl functions, the only way to do it is to have a flag day wherein they all change at once. And what about people writing Perl functions that they'd like to give to other people? If you have to have a flag, the only useful sort of flag is one that can be attached to individual functions. Compare what we did for plpgsql.variable_conflict in 9.0. I don't know how practical that will be in plperl, though. I don't see why it should be terribly difficult. We have the source code and we have a couple of powerful regex engines. Determining it it has a string in some position like # pragma: plperl.arrays_as_strings doesn't seem onerous. It's just a SMOC. It's not too hard to imagine other things that might be useful for. I thought the idea of overloading the string representation to look like the old style was a cute solution. If we don't have anyone at hand who knows how to do that, let's find someone who does. Not break our users' code because we're too lazy to find out how to do it properly. What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 4:45 PM, Andrew Dunstan and...@dunslane.net wrote: I thought the idea of overloading the string representation to look like the old style was a cute solution. If we don't have anyone at hand who knows how to do that, let's find someone who does. Not break our users' code because we're too lazy to find out how to do it properly. What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. I agree with your gut feeling. I suspect the Perl magic solution will make no one very happy. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 3:29 PM, Robert Haas wrote: What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. I agree with your gut feeling. I suspect the Perl magic solution will make no one very happy. Could do both, I suppose… David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
David E. Wheeler da...@kineticode.com writes: On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote: I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. I think that'd be pretty rare. I'm not seeing how an unsupported fear that there *might* be some incompatibilities is a good argument for instead adopting an approach that absolutely, positively, guaranteed *WILL* break everybody's code. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
David E. Wheeler da...@kineticode.com writes: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 22:12, Tom Lane t...@sss.pgh.pa.us wrote: David E. Wheeler da...@kineticode.com writes: On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: You mean packing both a string representation and a reference to a single SV * value? Dunno, I'm not a guts guy. I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. Yep, In-fact if we wanted we could even die (or throw an exception in other language speak :) ) when the string value is demanded. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 14:45, Andrew Dunstan and...@dunslane.net wrote: What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. I can't see how it would break, unless we did it wrong... If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. There might be a more gutsy way to do this so that 'ref' gives you back what you expected (would that be 'ARRAY' or 'SCALAR' ?), but there is a simple pure perl solution using overload: package PLPerl::ArgArray; use overload '' = \to_str; sub new { # note we bless an arrayref here instead of the usual hashref so # you can use this 'object' as a normal array return bless([@_], ... ); } sub to_str { my $self = shift; # yeah this is not right not correct :P # we could also die here with something like You are trying to use an Array as a string or whatever return join(',', map { '{'. $_ .'}' } @{$self}); } - -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] arrays as pl/perl input arguments [PATCH]
Hello, Here's the patch that improves handling of arrays as pl/perl function input arguments, converting postgres arrays of arbitrary dimensions into perl array references. It includes regression tests and a documentation changes, and it builds and runs successfully on my mac os x and linux boxes. To maintain compatibility with existing pl/perl code a new variable, plperl.convert_array_arguments (better name?), is introduced. Its default value is false, when set to true it triggers the new behavior, i.e. SET plperl.convert_array_arguments = true; CREATE OR REPLACE FUNCTION test_array(text[]) RETURNS TEXT AS $$ my $arg = shift; if (ref $arg eq 'ARRAY') { return array conversion is enabled; } else { return array conversion is disabled; } $$ LANGUAGE plperl; test=# select test_array('{1,2,3}'); test_array - array conversion is enabled (1 row) You can find other, less trivial examples, by examining plperl_array regression test. The implementation detects whether the input argument of a perl function is an array, and calls plperl_ref_from_pg_array if it is. The latter obtains a flat array of Datums from deconstruct_array and, using information about array dimensions, recursively creates perl array references in split_array. To pass array information between recursive calls a new 'plperl_array_info' struct was added. Arrays as members of composite types are also handled in plperl_hash_from_tuple. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. pg_to_perl_arrays.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 11, 2011, at 2:25 PM, Alexey Klyukin wrote: Hello, Here's the patch that improves handling of arrays as pl/perl function input arguments, converting postgres arrays of arbitrary dimensions into perl array references. Awesome! I've wanted this for *years*. It includes regression tests and a documentation changes, and it builds and runs successfully on my mac os x and linux boxes. To maintain compatibility with existing pl/perl code a new variable, plperl.convert_array_arguments (better name?), is introduced. Its default value is false, when set to true it triggers the new behavior, i.e. Have you considered instead passing an array-based object with is string overloading designed to return the pg array string format? That would make for nice, transparent compatibility without the need for a GUC. Not my idea, BTW, but suggested by Tim Bunce. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 01/11/2011 06:07 PM, David E. Wheeler wrote: To maintain compatibility with existing pl/perl code a new variable, plperl.convert_array_arguments (better name?), is introduced. Its default value is false, when set to true it triggers the new behavior, i.e. Have you considered instead passing an array-based object with is string overloading designed to return the pg array string format? That would make for nice, transparent compatibility without the need for a GUC. Not my idea, BTW, but suggested by Tim Bunce. I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote: I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. I think that'd be pretty rare. David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 01/11/2011 07:17 PM, David E. Wheeler wrote: On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote: I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. I think that'd be pretty rare. Possibly it would. But we usually try pretty hard to avoid that sort of breakage. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan and...@dunslane.net wrote: On 01/11/2011 07:17 PM, David E. Wheeler wrote: On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote: I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. I think that'd be pretty rare. Possibly it would. But we usually try pretty hard to avoid that sort of breakage. By the same token, I'm not convinced it's a good idea for this behavior to be off by default. Surely many people will altogether fail to notice that it's an option? If we're going to have a backward-compatibility GUC at all, ISTM that you ought to get the good stuff unless you ask for the old way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On 01/11/2011 09:06 PM, Robert Haas wrote: On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstanand...@dunslane.net wrote: On 01/11/2011 07:17 PM, David E. Wheeler wrote: On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote: I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed. I think that'd be pretty rare. Possibly it would. But we usually try pretty hard to avoid that sort of breakage. By the same token, I'm not convinced it's a good idea for this behavior to be off by default. Surely many people will altogether fail to notice that it's an option? If we're going to have a backward-compatibility GUC at all, ISTM that you ought to get the good stuff unless you ask for the old way. Sure, that seems reasonable. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] arrays as input parameters in plperl
Hi, I'd like to improve the way PL/Perl handles arrays as function input parameters. In PL/Perl arrays are passed as plain text strings, and getting a value of an array element requires additional perl code to parse that string. I'd like to make this easier by converting each postgresq array passed as an input parameter to the reference to the corresponding perl array. The patch in attachment illustrates this. it's limited to one-dimensional array output. The list of upcoming improvements is: - convert n-dimensional array to the perl equivalent (a reference to a list of references) - proper support for arrays of composite types - compatibility with existing plperl functions by introducing a new custom variable, i.e. plperl.pass_array_refs that triggers the new behavior. I think it should be disabled by default. - documentation and additional tests The patch adds a new attribute to the plperl_proc_desc struct, that records whether Nth argument is an array. The function plperl_ref_from_pg_array does the actual job of converting array input parameter to the perl array reference. I considered writing a perl routine instead of a C function, although I though it would be less readable, more complex and slower due to double conversion of input. The disadvantage of a C function is a code duplication with array_out, on which my function is based, although it can be avoided by putting a relevant part of array_out into a separate function. The patch is attached. Anybody interested in this feature ? Ideas, improvements, suggestions ? Regards, -- Alexey Klyukin http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc plperl_array.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Awhile back I wrote: I did some tests just now to determine the total number of catalog entries associated with a simple table definition. Assuming it has N user columns of built-in types (hence not requiring pg_depend entries for the datatypes), I count 1 pg_class entry for the table itself 1 pg_type entry for the rowtype N + 6 pg_attribute entries for the user and system columns 2 pg_depend entries (type - table and table - namespace) 2 pg_shdepend entries (ownership of table and type) Of course this goes up *fast* if you need a toast table, indexes, constraints, etc, but that's the irreducible minimum. Generating an array rowtype would add three more catalog entries to this (the array pg_type entry, a pg_depend arraytype-rowtype link, and another pg_shdepend entry), which isn't a huge percentage overhead. Obviously if we wanted to trim some fat here, getting rid of the redundant pg_attribute entries for system columns would be the first place to look. BTW, in the array patch as just committed, I was able to get rid of the pg_shdepend entries for a table rowtype (when it's not a free-standing composite type) and for an array type; instead they indirectly depend on the owner of the parent table or element type respectively. So the net increase from 8.2 is only one catalog entry (we save one existing pg_shdepend entry for the rowtype, and then add a pg_type entry for the array type and a pg_depend entry to link it to the rowtype). regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On 5/11/07, Tom Lane [EMAIL PROTECTED] wrote: BTW, in the array patch as just committed, I was able to get rid of the I am testing this feature, no problem so far. It's fast, and works exactly as advertised! Great work! (aiui, no domain arrays for 8.3?) merlin ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Arrays of Complex Types
I wrote: OK, summarising what looks to me like a consensus position, ISTM the plan could be: . fix makeArrayTypeName() or similar to make it try harder to generate a unique non-clashing name . remove the existing 62 instead of 63 name length restrictions . autogenerate array types for all explicitly or implicitly created composite types other than for system catalog objects. . defer for the present any consideration of a CREATE TYPE foo AS ARRAY ... command. Regarding catalog objects, we might have to try a little harder than just not generating in bootstrap mode - IIRC we generate system views (including pg_stats) in non-bootstrap mode. Maybe we just need to exempt anything in the pg_catalog namespace. What would happen if a user created a view over pg_statistic? Should the test be to avoid arrays for things that depend on the catalogs? Or maybe we should go to the heart of the problem and simply check for pseudo-types directly. I've been working on David's patch and done the following: . inhibit creation of array types for composites during initdb . some bug fixes . have CheckAttributeType recurse into composite types, so you can no longer create a table/type with a composite field which contains a pseudo-type column (like pg_statistic) However, there are still some oddities. For example, a change to or removal of the base type affects the array type, but the array type can be directly operated on (e.g. alter type _aa set schema foo ). I'm inclined to say we should prevent direct operations on array types, and they should live or die by their parent types. Thoughts? cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On Sun, May 06, 2007 at 01:33:47PM -0400, Andrew Dunstan wrote: However, there are still some oddities. For example, a change to or removal of the base type affects the array type, but the array type can be directly operated on (e.g. alter type _aa set schema foo ). I'm inclined to say we should prevent direct operations on array types, and they should live or die by their parent types. Thoughts? +1 on binding the array types tightly to the parent types. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: How would we do that? Not create the array types in bootstrap mode? Or just special-case pg_statistic? Not generate them in bootstrap mode works for me. IIRC, there's code somewhere in there that allows anyarray to pass as a column type in bootstrap mode, so that seems to fit ... OK, summarising what looks to me like a consensus position, ISTM the plan could be: . fix makeArrayTypeName() or similar to make it try harder to generate a unique non-clashing name . remove the existing 62 instead of 63 name length restrictions . autogenerate array types for all explicitly or implicitly created composite types other than for system catalog objects. . defer for the present any consideration of a CREATE TYPE foo AS ARRAY ... command. Regarding catalog objects, we might have to try a little harder than just not generating in bootstrap mode - IIRC we generate system views (including pg_stats) in non-bootstrap mode. Maybe we just need to exempt anything in the pg_catalog namespace. What would happen if a user created a view over pg_statistic? Should the test be to avoid arrays for things that depend on the catalogs? Or maybe we should go to the heart of the problem and simply check for pseudo-types directly. cheers andrew ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly