From: Arthur Zakirov <a.zakirov@postgrespro.ru>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Steele <david@pgmasters.net>
Cc: peter.eisentraut@2ndquadrant.com, PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Generic type subscripting
Date: Wed, 5 Apr 2017 16:06:04 +0300
Message-ID: <f65bee8c-34b8-178c-2ff2-28b2cfb7de4f@postgrespro.ru> (raw)
In-Reply-To: <CA+q6zcWVDF5Y8+FNQoV1vOmA9zCt4TiCqpHmDP3grP6v2mE30A@mail.gmail.com>
References: <CA+q6zcVovR+XY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA@mail.gmail.com>
<20423.1489533013@sss.pgh.pa.us>
<393cd0ae-5b84-849e-fc73-7077470a009e@pgmasters.net>
<CA+q6zcUX=Om75kgZXkpYPhnwUCo4osFpzd83dKPzjSGfCmxEgA@mail.gmail.com>
<65df4a65-a9d8-3531-5baf-edb237771f0e@pgmasters.net>
<22355.1490369372@sss.pgh.pa.us>
<449f42b9-f338-9e45-cb33-3e15c7b547ad@postgrespro.ru>
<CA+q6zcWFLpEX6H1LOOKFJMAN+LB+mmEx=6Syd4WVioXkbXq32A@mail.gmail.com>
<c7cc0592-4c9b-699d-3b61-810804759de6@postgrespro.ru>
<CA+q6zcXQ9UO0_qGqoFgd_4ymKYxwr-3he+abEbnWaPk2nbmPVQ@mail.gmail.com>
<CA+q6zcXsonX+qLhf2crQgj14NzJmcibABhyVch1kjNtSL1+V1A@mail.gmail.com>
<CAKNkYnzpMwx0KtLCaOoFVGQZoJHJ5E+mTrZGEcary0eKu2LdLg@mail.gmail.com>
<CA+q6zcWVDF5Y8+FNQoV1vOmA9zCt4TiCqpHmDP3grP6v2mE30A@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 04.04.2017 15:41, Dmitry Dolgov wrote:
> Sorry for late reply. Here is a new version of the patch, I rebased it and
> fixed those issues you've mentioned (pretty nasty problems, thank you for
> noticing).
Thank you!
I've looked at the patch again.
I'd like to focus on "refevalfunc" and "refnestedfunc" fields as I did
earlier. I think using Oid type for them is a bad approach. "..._fetch"
and "..._assign" functions in catalog is unnecessary movement to me.
User of subscript of his type may think the same. But he won't see the
code and won't know why he needs these functions.
And so "..._fetch" and "..._assign" functions in catalog is a bad design
to me. But, of course, it is just my opinion. This approach is the main
think which we should resolve first, because after commiting the patch
it will be hard to fix it.
> static int ArrayCount(const char *str, int *dim, char typdelim);
> +bool isAssignmentIndirectionExpr(ExprState *exprstate);
> static void ReadArrayStr(char *arrayStr, const char *origStr,
I think isAssignmentIndirectionExpr() here was forgoten to delete,
because isAssignmentIndirectionExpr() is in execExpr.c now.
> + if (subexpr == NULL)
> + {
> + lowerIndexpr = lappend(lowerIndexpr, subexpr);
> + continue;
> + }
> +
> +
There is the extra line here after the brace.
> if (array_type != sbsref->refcontainertype)
> {
>
> node = coerce_to_target_type(pstate,
> node, array_type,
> sbsref->refcontainertype, sbsref->reftypmod,
> COERCION_ASSIGNMENT,
> COERCE_IMPLICIT_CAST,
> -1);
>
> /* can fail if we had int2vector/oidvector, but not for true domains */
> if (node == NULL && node->type != 0)
> ereport(ERROR,
> (errcode(ERRCODE_CANNOT_COERCE),
> errmsg("cannot cast type %s to %s",
> format_type_be(array_type),
> format_type_be(sbsref->refcontainertype)),
> parser_errposition(pstate, 0)));
>
> PG_RETURN_POINTER(node);
> }
Also I was wondering do we need this code in array_subscript_parse()? I
haven't understood the purpose of it. If it is necessary then would be
good to add explain comment.
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.zakirov@postgrespro.ru, 9erthalion6@gmail.com, tgl@sss.pgh.pa.us, david@pgmasters.net
Subject: Re: [PATCH] Generic type subscripting
In-Reply-To: <f65bee8c-34b8-178c-2ff2-28b2cfb7de4f@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox