array_in: '{}}'::text[]

Started by Markus Bertheauabout 22 years ago8 messagespatches
Jump to latest
#1Markus Bertheau
twanger@bluetwanger.de

Is there a reason the array_in parser accepts additional closing braces
at the end?

oocms=# SELECT '{}}'::text[];
text
------
{}
(1 запись)

Thanks

--
Markus Bertheau <twanger@bluetwanger.de>

#2Markus Bertheau
twanger@bluetwanger.de
In reply to: Markus Bertheau (#1)
Re: array_in: '{}}'::text[]

В Пнд, 23.08.2004, в 13:45, Markus Bertheau пишет:

Is there a reason the array_in parser accepts additional closing braces
at the end?

In fact it seems to accept everything after the closing brace matching
the first opening brace.

Thanks

--
Markus Bertheau <twanger@bluetwanger.de>

#3Joe Conway
mail@joeconway.com
In reply to: Markus Bertheau (#1)
Re: array_in: '{}}'::text[]

Markus Bertheau wrote:

Is there a reason the array_in parser accepts additional closing braces
at the end?

oocms=# SELECT '{}}'::text[];
text
------
{}
(1 запись)

Hmmm, I was *about* to say that this is fixed in cvs (and indeed, the
array_in parser is significantly tightened up compared to previous
releases), but unfortunately, there is still work to be done :(

regression=# SELECT '{}}'::text[];
text
------
{}
(1 row)

regression=# select version();
version
---------------------------------------------------------------------------------------------------------------------
PostgreSQL 8.0.0beta1 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 3.3.3 20040412 (Red Hat Linux 3.3.3-7)
(1 row)

Look for a fix soon, at a cvs repository near you....

Joe

#4Joe Conway
mail@joeconway.com
In reply to: Joe Conway (#3)
Re: [SQL] array_in: '{}}'::text[]

Joe Conway wrote:

Markus Bertheau wrote:

Is there a reason the array_in parser accepts additional closing braces
at the end?

oocms=# SELECT '{}}'::text[];
text
------
{}
(1 запись)

Hmmm, I was *about* to say that this is fixed in cvs (and indeed, the
array_in parser is significantly tightened up compared to previous
releases), but unfortunately, there is still work to be done :(

The attached patch takes care of the above issue:

regression=# SELECT '{}}'::text[];
ERROR: malformed array literal: "{}}"

If there are no objections, I'll apply in about 24 hours.

Joe

Attachments:

current.80.difftext/x-patch; name=current.80.diffDownload+110-8
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#4)
Re: [SQL] array_in: '{}}'::text[]

Joe Conway <mail@joeconway.com> writes:

/* Make a modifiable copy of the input */
! string_save = (char *) palloc0(strlen(string) + 1);
strcpy(string_save, string);

palloc0, instead of palloc, is clearly a waste of cycles here ...

actually, why isn't this just a pstrdup?

/* special case for an empty array */
! if (strlen(str) == 2 && strncmp(str, "{}", 2) == 0)
return 0;

Why not just if (strcmp(str, "{}") == 0)

Looks reasonable otherwise.

regards, tom lane

#6Joe Conway
mail@joeconway.com
In reply to: Tom Lane (#5)
Re: [SQL] array_in: '{}}'::text[]

Tom Lane wrote:

actually, why isn't this just a pstrdup?

Why not just if (strcmp(str, "{}") == 0)

Good points. Changes made, and attached committed.

Joe

Attachments:

current.80.difftext/x-patch; name=current.80.diffDownload+111-9
#7Markus Bertheau
twanger@bluetwanger.de
In reply to: Joe Conway (#6)
Re: [SQL] array_in: '{}}'::text[]

В Сбт, 28.08.2004, в 21:33, Joe Conway пишет:

/* special case for an empty array */
! if (strcmp(str, "{}") == 0)
return 0;

Without looking at the code in a whole, you accept '{} ' as an empty
array literal, so why is the special case for '{}' needed here? I should
be catched by the code that accepts additional whitespace after the last
closing brace, just that there is no white space:

+ 	/* only whitespace is allowed after the closing brace */
+ 	while (*ptr)
+ 	{
+ 		if (!isspace(*ptr++))
+ 			ereport(ERROR,
+ 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ 				errmsg("malformed array literal: \"%s\"", str)));
+ 	}
+ 	
+ 	/* special case for an empty array */
+ 	if (empty_array)
+ 		return 0;

Here's the second special case for empty arrays.

--
Markus Bertheau <twanger@bluetwanger.de>

#8Joe Conway
mail@joeconway.com
In reply to: Markus Bertheau (#7)
Re: [SQL] array_in: '{}}'::text[]

Markus Bertheau wrote:

Without looking at the code in a whole, you accept '{} ' as an empty
array literal, so why is the special case for '{}' needed here?

It's a fast path for a common special case. Why spend any cycles parsing
if we can immediately recognize it? However, anything other than a
simple '{}' does require parsing.

Joe