[PATCH] Generic type subscripting

On 2/28/17 13:02, Dmitry Dolgov wrote:

+<programlisting>
+-- Extract value by key
+SELECT ('{"a": 1}'::jsonb)['a'];
+
+-- Extract nested value by key path
+SELECT ('{"a": {"b": {"c": 1}}}'::jsonb)['a']['b']['c'];
+
+-- Extract element by index
+SELECT ('[1, "2", null]'::jsonb)['1'];
+
+-- Update value by key
+UPDATE table_name set jsonb_field['key'] = 1;
+
+-- Select records using where clause with subscripting
+SELECT * from table_name where jsonb_field['key'] = '"value"';
+</programlisting>

I see a possible problem here: This design only allows one subscripting
function. But what you'd really want in this case is at least two: one
taking an integer type for selecting by array index, and one taking text
for selecting by field name.

I suppose that since a given value can only be either an array or an
object, there is no ambiguity, but I think this might also lose some
error checking. It might also not work the same way for other types.

It looks like your jsonb subscripting function just returns null if it
can't find a field, which is also a bit dubious.

--
Peter Eisentraut http://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

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

I see a possible problem here: This design only allows one subscripting
function. But what you'd really want in this case is at least two: one
taking an integer type for selecting by array index, and one taking text
for selecting by field name.

No, I think you're missing the point: there is just one function per
datatype for *parse analysis* of a subscripting operation applied to
that datatype. What it chooses to allow as subscript type, and what
function it determines will be used at execution, is up to it.

I agree that the given jsonb_subscript is failing to handle the
subscript-an-array-with-an-integer case, but that's a datatype-specific
shortcoming not a failure of the overall design.

I would guess that what we really want for jsonb is the ability to
intermix integer and text subscripts, so that
jsonbcol['foo'][42]['bar']
would extract the "bar" field of an object in position 42 of an
array in field "foo" of the given jsonb value. So you probably
end up still having one jsonb execution function, not two, and
it would have different code paths depending on whether it sees
the type of the next subscript expression to be integer or text.

It looks like your jsonb subscripting function just returns null if it
can't find a field, which is also a bit dubious.

Nah, that seems fine. Our precedent for standard array subscripting is to
return NULL for out-of-range subscripts, and the jsonb -> operator also
returns NULL if there's no such field. It would be rather surprising if
jsonb subscripting threw an error instead; and I do not think it would be
more useful.

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

Dmitry Dolgov <9erthalion6@gmail.com> writes:

[ generic_type_subscription_v7.patch ]

I looked through this a bit.

I think that the basic design of having a type-specific parse analysis
function that returns a constructed SubscriptingRef node is fine.

I'm not totally excited about the naming you've chosen though,
particularly the function names "array_subscripting()" and
"jsonb_subscripting()" --- those are too generic, and a person coming to
them for the first time would probably expect that they actually execute
subscripting, when they do no such thing. Names like
"array_subscript_parse()" might be better. Likewise the name of the
new pg_type column doesn't really convey what it does, though I suppose
"typsubscriptparse" is too much of a mouthful. "typsubparse" seems short
enough but might be confusing too.

I wonder also if we should try to provide some helper functions rather
than expecting every data type to know all there is to know about parsing
and execution of subscripting. Not sure what would be helpful, however.

One thing that needs more thought for sure is the nested-assignment case
(the logic around isAssignmentIndirectionExpr) --- the design you've got
here implies that *every* container-like datatype would need to duplicate
that logic, and I don't think we want to go there.

The documentation needs a lot of work of course, and I do not think
you're doing either yourself or anybody else any favors with the proposed
additions to src/tutorial/. You'll never be sure that that stuff even
compiles let alone accurately represents what people need to do. Actual
running code is much better. It may be that jsonb_subscript is enough
of an extension example, but perhaps adding a subscripting feature to some
contrib module would be better.

Aren't SBS_VALIDATION and SBS_EXEC just hangovers from the previous
design? They're still in parse_node.h, and they're still mentioned in
the docs, but I don't see them used in actual code anywhere.
get_slice_arguments seems to be a hangover as well, which is good
because it's mighty ugly and undocumented.

It seems rather silly for ExecEvalSubscriptingRef to be palloc'ing some
per-subscript arrays each time through when it's got other arrays that are
still of fixed size MAXDIM. I can believe that it might be a good idea
to increase or remove the MAXDIM limit, but this doesn't do it. In any
case, you don't want to add the overhead of a couple of pallocs per
execution. Using OidFunctionCall2 is even worse: that's adding a system
catalog lookup per execution. You need to be caching the function address
as is done for regular function and operator calls. (I take it you've not
done performance testing yet.)

I'm not really finding this to be an improvement:
-				  errmsg("array subscript in assignment must not be null")));
+				  errmsg("container subscript in assignment must not be null")));
"Container" is going to seem like jargon to users.  Maybe it'd be okay to
drop the word altogether and just say "subscript in assignment must not be
null".  (Another question here is whether every datatype will be on board
with the current rules about null subscripts, or whether we need to
delegate null-handling to the datatype-specific execution function.
I'm not sure; it would complicate the API significantly for what might be
useless flexibility.)

I'm tempted to propose that it'd be a good idea to separate the regular
(varlena) array code paths from the fixed-length-array code paths
altogether, which you could do in this implementation by having separate
execution functions for them. That would buy back some fraction of
whatever overhead we're adding with the additional level of function call.
Maybe even separate execution functions for the slice and not-slice
cases, though that might be overkill.

I'm not on board with these APPLY_OPERATOR_TO_TYPE macros. If you
think you have a cute idea for improving the notation in the node support
files, great; submit a patch that changes all of the support functions
at once. Patches that introduce one or two support functions that look
radically different from all the rest are not acceptable.

Likewise, what you did in places like JumbleExpr is too cute by half.
Just make two separate cases for the two new node types. You're not
saving enough code to justify the additional intellectual complexity
and maintenance burden of doing it like that.

I do not think that the extra SubscriptingBase data structure is paying
for itself either; you're taking a hit in readability from the extra level
of struct, and as far as I can find it's not buying you one single thing,
because there's no code that operates on a SubscriptingBase argument.
I'd just drop that idea and make two independent struct types, or else
stick with the original ArrayRef design that made one struct serve both
purposes. (IOW, my feeling that a separate assign struct would be a
readability improvement isn't exactly getting borne out by what you've
done here. But maybe there's a better way to do that.)

I wouldn't suggest putting a lot of work into the execQual.c part of the
patch right now, as execQual.c is going to look completely different if
Andres' patch gets in. Better to concentrate on cleaning up the parsenode
struct types and thinking about a less messy answer for nested assignment.

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

Hi Dmitry,

On 3/14/17 7:10 PM, Tom Lane wrote:

Dmitry Dolgov <9erthalion6@gmail.com> writes:

[ generic_type_subscription_v7.patch ]

I looked through this a bit.

This thread has been idle for over a week. Please respond and/or post a
new patch by 2017-03-24 00:00 AoE (UTC-12) or this submission will be
marked "Returned with Feedback".

Thanks,
--
-David
david@pgmasters.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 21 March 2017 at 18:16, David Steele <david@pgmasters.net> wrote:

This thread has been idle for over a week.

Yes, sorry for the late reply. I'm still trying to find a better solution
for
some of the problems, that arose in this patch.

On 15 March 2017 at 00:10, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Dmitry Dolgov <9erthalion6@gmail.com> writes:

I looked through this a bit.

Thank you for the feedback.

On 10 March 2017 at 06:20, Peter Eisentraut <

peter.eisentraut@2ndquadrant.com> wrote:

I see a possible problem here: This design only allows one subscripting
function. But what you'd really want in this case is at least two: one
taking an integer type for selecting by array index, and one taking text
for selecting by field name.

I would guess that what we really want for jsonb is the ability to
intermix integer and text subscripts, so that
jsonbcol['foo'][42]['bar']
would extract the "bar" field of an object in position 42 of an
array in field "foo" of the given jsonb value.

Maybe I misunderstood you, but isn't it already possible?

```
=# select ('{"a": [{"b": 1}, {"c": 2}]}'::jsonb)['a'][0]['b'];
jsonb
-------
1
(1 row)

=# select * from test;
data
-----------------------------
{"a": [{"b": 1}, {"c": 2}]}
(1 row)

=# update test set data['a'][0]['b'] = 42;
UPDATE 1

=# select * from test;
data
-----------------------------
{"a": [{"b": 42}, {"c": 2}]}
(1 row)
```

I'm not totally excited about the naming you've chosen though,
particularly the function names "array_subscripting()" and
"jsonb_subscripting()" --- those are too generic, and a person coming to
them for the first time would probably expect that they actually execute
subscripting, when they do no such thing. Names like
"array_subscript_parse()" might be better. Likewise the name of the
new pg_type column doesn't really convey what it does, though I suppose
"typsubscriptparse" is too much of a mouthful. "typsubparse" seems short
enough but might be confusing too.

It looks quite tempting for me to replace the word "subscript" by an
abbreviation all over the patch. Then it will become something like
"typsbsparse" which is not that mouthful, but I'm not sure if it will be
easily
recognizable.

I wonder also if we should try to provide some helper functions rather
than expecting every data type to know all there is to know about parsing
and execution of subscripting. Not sure what would be helpful, however.

I don't really see what details we can hide behind this helper, because
almost
all code there is type specific (e.g. to check if subscript indexes are
correct), can you elaborate on that?

The documentation needs a lot of work of course, and I do not think
you're doing either yourself or anybody else any favors with the proposed
additions to src/tutorial/.

Yes, unfortunately, I forget to update documentation from the previous
version
of the patch. I'll fix it soon in the next version.

Aren't SBS_VALIDATION and SBS_EXEC just hangovers from the previous
design? They're still in parse_node.h, and they're still mentioned in
the docs, but I don't see them used in actual code anywhere.

Yes, these are from the previous version too, I'll remove them.

Another question here is whether every datatype will be on board
with the current rules about null subscripts, or whether we need to
delegate null-handling to the datatype-specific execution function.
I'm not sure; it would complicate the API significantly for what might be
useless flexibility.

It looks for me that it's a great idea to perform null-handling inside
datatype
specific code and I'm not sure, what would be complicated? All necessary
information for that is already in `SubscriptingExecData` (I just have to
use
`eisnull` in a different way).

I do not think that the extra SubscriptingBase data structure is paying
for itself either; you're taking a hit in readability from the extra level
of struct, and as far as I can find it's not buying you one single thing,
because there's no code that operates on a SubscriptingBase argument.
I'd just drop that idea and make two independent struct types, or else
stick with the original ArrayRef design that made one struct serve both
purposes. (IOW, my feeling that a separate assign struct would be a
readability improvement isn't exactly getting borne out by what you've
done here. But maybe there's a better way to do that.)

I'm thinking to replace these structures by more meaningful ones, something
like:

```
typedef struct SubscriptContainerRef
{
Expr xpr;
Oid refcontainertype;
Oid refelemtype;
int32 reftypmod;
Oid refcollid;
} SubscriptBase;

typedef struct SubscriptExtractRef
{
Expr xpr;
SubscriptContainerRef *containerExpr;
List *refupperindexpr;
List *reflowerindexpr;
Oid refevalfunc;
} SubscriptExtractRef;

typedef struct SubscriptAssignRef
{
Expr xpr;
SubscriptContainerRef *containerExpr;
Expr *refassgnexpr;
List *refupperindexpr;
List *reflowerindexpr;
Oid refevalfunc;
} SubscriptAssignRef;

```

It's close to the previous version, but here will be a bit less duplicated
code
in comparison with two independent structures, we still can use different
nodes
for data assignment and data extraction, and node processing for these nodes
can be little bit more separated, e.g.:

```
case T_SubscriptExtractRef:
{
// extract specific logic
sbsref->containerExpr = ExecInitExpr((Expr *)
sbsref->containerExpr, parent);
}
case T_SubscriptAssignRef:
{
// assign specific logic
sbsref->containerExpr = ExecInitExpr((Expr *)
sbsref->containerExpr, parent);
}
case T_SubscriptContainerRef:
{
// subscript container logic
}
```

Hi Dmitry,

On 3/21/17 4:42 PM, Dmitry Dolgov wrote:

On 21 March 2017 at 18:16, David Steele <david@pgmasters.net

<mailto:david@pgmasters.net>> wrote:

This thread has been idle for over a week.

Yes, sorry for the late reply. I'm still trying to find a better
solution for
some of the problems, that arose in this patch.

On 15 March 2017 at 00:10, Tom Lane <tgl@sss.pgh.pa.us

<mailto:tgl@sss.pgh.pa.us>> wrote:

Dmitry Dolgov <9erthalion6@gmail.com <mailto:9erthalion6@gmail.com>>

writes:

I looked through this a bit.

Do you have an idea when you will have a patch ready? We are now into
the last week of the commitfest. I see one question for Tom, but it's
not clear that this would prevent you from producing a new patch.

Please post a new patch by 2017-03-28 00:00 AoE (UTC-12) or this
submission will be marked "Returned with Feedback".

Thanks,
--
-David
david@pgmasters.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 24 March 2017 at 15:39, David Steele <david@pgmasters.net> wrote:

Do you have an idea when you will have a patch ready?

Yes, I'll prepare a new version with most important changes in two days.

David Steele <david@pgmasters.net> writes:

Do you have an idea when you will have a patch ready? We are now into
the last week of the commitfest. I see one question for Tom, but it's
not clear that this would prevent you from producing a new patch.

FWIW, I'm up to my eyeballs in Andres' faster-expressions patch, and
won't have time to think about this one for at least a couple more
days.

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

On 24.03.2017 18:29, Tom Lane wrote:

David Steele <david@pgmasters.net> writes:

Do you have an idea when you will have a patch ready? We are now into
the last week of the commitfest. I see one question for Tom, but it's
not clear that this would prevent you from producing a new patch.

FWIW, I'm up to my eyeballs in Andres' faster-expressions patch, and
won't have time to think about this one for at least a couple more
days.

regards, tom lane

I can try to review a new version of the patch, when Dmitry will send
it. If no one objects. Besides, I've seen the previous versions of the
patch from previous commitfest.

--
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

Here is a new version of this patch. What was changed:

* I rebased code against the latest version of master and adapted recent
changes about the expression execution

* Several names (functions and related pg_type column) were changed

* A new oid function field was introduced to handle nested assignment
situation

* I updated the documentation for patch

* `MAXDIM` was replaced by `MAX_SUBSCRIPT_DEPTH`

* I returned one `SubscriptingRef` for both fetch & assign operations, since
there is no real readability improvements at this point (they're already
separated at the time of evaluation, and besides the evaluation code
fetch &
assign are handled almost identically).

Hello,

On 27.03.2017 23:28, Dmitry Dolgov wrote:

Here is a new version of this patch. What was changed:

* I rebased code against the latest version of master and adapted recent
changes about the expression execution

* Several names (functions and related pg_type column) were changed

* A new oid function field was introduced to handle nested assignment
situation

* I updated the documentation for patch

* `MAXDIM` was replaced by `MAX_SUBSCRIPT_DEPTH`

* I returned one `SubscriptingRef` for both fetch & assign operations, since
there is no real readability improvements at this point (they're already
separated at the time of evaluation, and besides the evaluation code
fetch &
assign are handled almost identically).

Your patch reverts commits from 25-26 march. And therefore contains
15000 lines.

I think the patch needs rebasing.

--
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

On 28 March 2017 at 11:58, Arthur Zakirov <a.zakirov@postgrespro.ru> wrote:

Your patch reverts commits from 25-26 march. And therefore contains 15000

lines.

Wow, I didn't notice that, sorry - will fix it shortly.

On 28 March 2017 at 12:08, Dmitry Dolgov <9erthalion6@gmail.com> wrote:

Wow, I didn't notice that, sorry - will fix it shortly.

So, here is the corrected version of the patch.

On 28.03.2017 19:31, Dmitry Dolgov wrote:

On 28 March 2017 at 12:08, Dmitry Dolgov <9erthalion6@gmail.com
<mailto:9erthalion6@gmail.com>> wrote:

Wow, I didn't notice that, sorry - will fix it shortly.

So, here is the corrected version of the patch.

I have some picky comments.

I'm not sure that "typsbsparse" is better than "typsubscripting" or
"typsubparse". Maybe "typsubsparse"?

<row>
+      <entry><structfield>typsubscripting</structfield></entry>
+      <entry><type>regproc</type></entry>

Here you didn't fix "typsubscripting" to new name.

+  <title>JSON subscripting</title>
+  <para>
+   JSONB data type support array-style subscripting expressions to extract or update particular element. An example of subscripting syntax:

Should be "JSONB data type supports".

+  to handle subscripting expressions. It should contains logic for verification
+  and decide which function must be used for evaluation of this expression.
+  For instance:

Should be "It should contain".

+ <sect2 id="json-subscripting">
+  <title>JSON subscripting</title>
+  <para>
+   JSONB data type support array-style subscripting expressions to extract or update particular element. An example of subscripting syntax:

You have implemented jsonb subscripting. The documentation should be
fixed to:

+ <sect2 id="jsonb-subscripting">
+  <title><type>jsonb</> Subscripting</title>
+  <para>
+   <type>jsonb</> data type support array-style subscripting 
expressions to extract or update particular element. An example of 
subscripting syntax:

I think IsOneOf() macros should be removed. Since it is not used anywhere.

+		Assert(subexpr != NULL);
+
+		if (subexpr == NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_DATATYPE_MISMATCH),
+					 errmsg("jsonb subscript does not support slices"),
+					 parser_errposition(pstate, exprLocation(
+						((Node *) lfirst(sbsref->refupperindexpr->head))))));

Here one of the conditions is excess. Better to delete assert condition
I think.

I've tried tests from message [1]. They looks good. Performance looks
similar for subscripting without patch and with patch.

I wanted to implement subscripting for ltree or hstore extensions.
Subscripting for ltree looks more interesting. Especially with slicing.
But I haven't done it yet. I hope that I will implement it tomorrow.

1.
/messages/by-id/CAKNkYnz_WWkzzxyFx934N=Ep47CAFju-Rk-sGeZo0ui8QdrGmw@mail.gmail.com

--
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

On 29 March 2017 at 19:14, Arthur Zakirov <a.zakirov@postgrespro.ru> wrote:

I'm not sure that "typsbsparse" is better than "typsubscripting" or

"typsubparse". Maybe "typsubsparse"?

`typsubparse` is more confusing as for me, but I like `typsubsparse`.

I've tried tests from message [1]. They looks good. Performance looks

similar for subscripting without patch and with patch.

Great, thank you for confirmation.

I've attached a new version of the patch with related improvements.

On 29.03.2017 20:14, Arthur Zakirov wrote:

I wanted to implement subscripting for ltree or hstore extensions.
Subscripting for ltree looks more interesting. Especially with slicing.
But I haven't done it yet. I hope that I will implement it tomorrow.

ltree
-----

I've implemented fetching ltree elements using subscripting. But haven't
implemented assigning ltree elements yet. I'll send second patch after
implementing assigning.

Now you can execute the following query:

SELECT ('Top.Science.Astronomy.Astrophysics'::ltree)[1:2];
ltree
-------------
Top.Science

Comments
--------

But I've noticed about some points.

In array_subscript_parse() passed uninitialized values of "typesource"
and "typeneeded" variables for coerce_to_target_type().

+			typesource = exprType(assignExpr);
+			typesource = is_slice ? sbsref->refcontainertype : sbsref->refelemtype;

Here is the bug. Second variable should be "typeneeded". Moreover these
assignments should be moved up to first coerce_to_target_type() execution.

+	foreach(l, sbsref->reflowerindexpr)
+	{
+		List *expr_ai = (List *) lfirst(l);
+		A_Indices *ai = (A_Indices *) lfirst(list_tail(expr_ai));
+
+		subexpr = (Node *) lfirst(list_head(expr_ai));

This code looks like a magic. This happens because of appending
A_Indeces to lowerIndexpr:

-			subexpr = NULL;
+			lowerIndexpr = lappend(lowerIndexpr, list_make2(subexpr, ai));
}

And this A_Indeces used only when slicing is not used to make a constant
1. Maybe there are another way?

Also it would be better if "refevalfunc" and "refnestedfunc" had
pointers to functions not Oid type. Now you need to create "..._fetch"
and "..._assign" functions in catalog and in "..._parse" function you
need get their Oid using to_regproc() function.

Can we use IndexAmRoutine structure method, when you use only pointers
to necessary functions? You can see an example in blhandler() function
in blutils.c.

The last point is about the tutorial. As Tom pointed it is not useful
when the tutorial doesn't execute. It happens because there is not
"custom" type in subscripting.sql. Also it contradicts the README of
tutorials.

--
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

On 30 March 2017 at 19:36, Arthur Zakirov <a.zakirov@postgrespro.ru> wrote:

The last point is about the tutorial. As Tom pointed it is not useful

when the tutorial doesn't execute. It happens because there is not "custom"
type in subscripting.sql.

I'm confused. Maybe I'm missing something, but there is "custom" type in
this file:

```
-- subscripting.sql

CREATE TYPE custom (
internallength = 8,
input = custom_in,
output = custom_out,
subscripting = custom_subscript_parse
);
```

```

\i subscripting.sql

psql:subscripting.sql:39: NOTICE: 42704: type "custom" is not yet defined
DETAIL: Creating a shell type definition.
LOCATION: compute_return_type, functioncmds.c:141
CREATE FUNCTION
Time: 4.257 ms
psql:subscripting.sql:47: NOTICE: 42809: argument type custom is only a
shell
LOCATION: interpret_function_parameter_list, functioncmds.c:245
CREATE FUNCTION
Time: 37.038 ms
CREATE FUNCTION
Time: 13.891 ms
CREATE FUNCTION
Time: 0.946 ms
CREATE FUNCTION
Time: 1.161 ms
CREATE TYPE
Time: 1.336 ms
CREATE TABLE
Time: 2.129 ms
INSERT 0 1
Time: 2.501 ms
data
------
2
(1 row)

Time: 0.960 ms
UPDATE 1
Time: 0.887 ms
```

So the only problem I see is notification about "type 'custom' is not yet
defined", but it's the same for "complex" tutorial

```

\i complex.sql

psql:complex.sql:39: NOTICE: 42704: type "complex" is not yet defined
DETAIL: Creating a shell type definition.
LOCATION: compute_return_type, functioncmds.c:141
CREATE FUNCTION
Time: 1.741 ms
psql:complex.sql:47: NOTICE: 42809: argument type complex is only a shell
LOCATION: interpret_function_parameter_list, functioncmds.c:245
CREATE FUNCTION
Time: 0.977 ms
psql:complex.sql:55: NOTICE: 42809: return type complex is only a shell
LOCATION: compute_return_type, functioncmds.c:105
CREATE FUNCTION
Time: 0.975 ms
psql:complex.sql:63: NOTICE: 42809: argument type complex is only a shell
LOCATION: interpret_function_parameter_list, functioncmds.c:245
CREATE FUNCTION
Time: 0.893 ms
CREATE TYPE
Time: 0.992 ms
...
```

Can you clarify this point?

2017-03-31 5:32 GMT+03:00 Dmitry Dolgov <9erthalion6@gmail.com>:

On 30 March 2017 at 19:36, Arthur Zakirov <a.zakirov@postgrespro.ru> wrote:

The last point is about the tutorial. As Tom pointed it is not useful when
the tutorial doesn't execute. It happens because there is not "custom" type
in subscripting.sql.

I'm confused. Maybe I'm missing something, but there is "custom" type in
this file:

Sorry for confusing. I should have been more careful. I've mixed up
NOTICE message with error message and I haven't noticed CREATE TYPE
command.

--
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

2017-03-28 19:31 GMT+03:00 Dmitry Dolgov <9erthalion6@gmail.com>:

On 28 March 2017 at 12:08, Dmitry Dolgov <9erthalion6@gmail.com> wrote:

Wow, I didn't notice that, sorry - will fix it shortly.

So, here is the corrected version of the patch.

Thank you!

The patch looks good to me. But it is not applied :) I think it is
because of new FTS functions for jsonb. The patch need rebasing.

But, from my point of view, it would be nice if the code mentioned
earlier was improved:

+ foreach(l, sbsref->reflowerindexpr)
+ {
+ List *expr_ai = (List *) lfirst(l);
+ A_Indices *ai = (A_Indices *) lfirst(list_tail(expr_ai));
+
+ subexpr = (Node *) lfirst(list_head(expr_ai));

It is necessary for arrays of course because of logic mentioned in the
documentation.

If any dimension is written as a slice, i.e., contains a colon, then all dimensions are treated as slices. Any dimension that has only a single number (no colon) is treated as being from 1 to the number specified.

But it would be better if SubscriptingRef structure had a new field of
List type. This field can store list of booleans, which means is there
slices or not. I think it can improve readability of the code.

--
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